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

[Cédric Le Goater]

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;
> > +}