[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 11/11] pflash: Clean up after co
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v2 11/11] pflash: Clean up after commit 368a354f02b part 2 |
Date: |
Tue, 5 Mar 2019 18:54:17 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 |
On 3/5/19 5:42 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
>
>> Hi Markus,
>>
>> On 2/26/19 8:34 PM, Markus Armbruster wrote:
>>> QOMification left parameter @size unused in pflash_cfi01_register()
>>> and pflash_cfi02_register(). Obviously, @size should match
>>> @sector_len and @nb_blocs, i.e. size == sector_len * nb_blocs. All
>>> callers satisfy this.
>>>
>>> Remove @nb_blocs and compute it from @size and @sector_len.
>>
>> The pflash devices are simplistically modelled has having a single
>> sector size. This is not true for real hw (pflash have different sector
>> sizes). It is however true for the unofficial VirtPFlash device, which
>> doesn't have any spec.
>>
>> The only valid parameter is @size. The @sector_len and @nb_blocs don't
>> match the hardwares. QEMU @sector_len is currently "The biggest
>> sector_size the device has" and QEMU @nb_blocs is really "@size /
>> @sector_len", although it doesn't match the nb_blocs the hardware has.
>>
>> I don't disagree with the patch content, but the description is not
>> accurate. We can indeed drop the @nb_blocs argument because we model an
>> unique sector size, thus we can calculate "@nb_blocs = @size /
>> @sector_len" in the model.
>>
>> With the patch description fixed:
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>
> Let me try to fix it.
>
> Our pflash devices are simplistically modelled has having
> "num-blocks" sectors of equal size "sector-length". Real hardware
> commonly has sectors of different sizes. How our "sector-length"
> property is related to the physical device's multiple sector sizes
> is unclear.
>
> Helper functions pflash_cfi01_register() and pflash_cfi02_register()
> create a pflash device, set properties including "sector-length" and
> "num-blocks", and realize. They take parameters @size, @sector_len
> and @nb_blocs.
>
> QOMification left parameter @size unused. Obviously, @size should
> match @sector_len and @nb_blocs, i.e. size == sector_len * nb_blocs.
> All callers satisfy this.
>
> Remove @nb_blocs and compute it from @size and @sector_len.
>
> D'accord?
Oui, parfait!