Re: [PATCH 09/12] m25p80: Introduce an helper to retrieve the BlockBackend of a device

["Cédric Le Goater" <clg@kaod.org>]

On 5/30/23 23:14, Philippe Mathieu-Daudé wrote:
> On 8/5/23 09:58, Cédric Le Goater wrote:
>> It will help in getting rid of some drive_get(IF_MTD) calls by
>> retrieving the BlockBackend directly from the m25p80 device.
>>
>> Cc: Alistair Francis <alistair@alistair23.me>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   include/hw/block/flash.h | 4 ++++
>>   hw/block/m25p80.c        | 6 ++++++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
>> index 7198953702..de93756cbe 100644
>> --- a/include/hw/block/flash.h
>> +++ b/include/hw/block/flash.h
>> @@ -76,4 +76,8 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
>>   void ecc_reset(ECCState *s);
>>   extern const VMStateDescription vmstate_ecc_state;
>> +/* m25p80.c */
>> +
>> +BlockBackend *m25p80_get_blk(DeviceState *dev);
> 
> - Option 1, declare QOM typedef and use proper type:
> 
>    #define TYPE_M25P80 "m25p80-generic"
>    OBJECT_DECLARE_TYPE(Flash, M25P80Class, M25P80)
> 
>    BlockBackend *m25p80_get_blk(Flash *dev);
> 
> - Option 2, preliminary patch renaming 'Flash' type to
> 'M25P80' then option 1 again

That's a large change and we would need to introduce a m25p80.h with
these definitions. Given that the caller needs a DeviceState in the
end, may be not worth the extra hassle.

How would you rename 'Flash' ? 'SpiFlash' ?

Thanks,

C.

> 
> - Option 3: no change.
> 
> With the QOM style we try to enforce, I'd go for #2.
> 
> Still,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>>   #endif
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index dc5ffbc4ff..afc3fdf4d6 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -25,6 +25,7 @@
>>   #include "qemu/units.h"
>>   #include "sysemu/block-backend.h"
>>   #include "hw/block/block.h"
>> +#include "hw/block/flash.h"
>>   #include "hw/qdev-properties.h"
>>   #include "hw/qdev-properties-system.h"
>>   #include "hw/ssi/ssi.h"
>> @@ -1830,3 +1831,8 @@ static void m25p80_register_types(void)
>>   }
>>   type_init(m25p80_register_types)
>> +
>> +BlockBackend *m25p80_get_blk(DeviceState *dev)
>> +{
>> +    return M25P80(dev)->blk;
>> +}
>