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

[Philippe Mathieu-Daudé]

On 31/5/23 08:48, Cédric Le Goater wrote:
> 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

Yes, it can be done later if you rather.

> and we would need to introduce a m25p80.h with
> these definitions.

FlashPartInfo is used as pointer so can be forward-declared.
Then we only need to move M25P80_INTERNAL_DATA_BUFFER_SZ.
For 5 lines, "hw/block/flash.h" is good enough IMO, keeping
all the rest to m25p80.c.

> Given that the caller needs a DeviceState in the
> end, may be not worth the extra hassle.
>
> How would you rename 'Flash' ? 'SpiFlash' ?

Since you ask, my preference is SpiNorFlash :)

But again, this can be done later on top.

Thanks,

Phil.