Re: [PATCH 27/50] pckbd: alter i8042_mm_init() to return a I8042_MMIO device

[Mark Cave-Ayland]

On 10/06/2022 08:04, Mark Cave-Ayland wrote:

> On 09/06/2022 11:58, Peter Maydell wrote:
>
> > On Sun, 22 May 2022 at 19:19, Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> > > This exposes the I8042_MMIO device to the caller to allow the register memory
> > > region to be mapped outside of i8042_mm_init().
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > I'm not sure where best to put this review comment in the patchseries,
> > so I'll just note it here: I see that at the end of the series we end
> > up with an i8042_mm_init() which is just "create a device with qdev_new,
> > set properties, realize it, and connect its gpio lines". Since that
> > function has exactly one callsite (in hw/mips/jazz.c) I think we should
> > add a patch on the end that gets rid of i8042_mm_init() entirely in
> > favour of the board doing all the "create and wire up device" code inline.
>
> I actually have a patch to do this in the follow-up part 2 series locally. The aim 
> was to get part 1 merged first (which is removing the update callback and the 
> opaque), and then finish tidying up the QOM modelling of the devices in part 2.
>
> However since part 1 has been posted, both Hervé and Helge have mentioned that they 
> wouldn't mind if there was a migration break for the hppa and MIPS magnum machines. 
> As this does save a bit of work for the I8042_MMIO device, I'll see if it is easy 
> enough to bring forward into part 1.

After a bit of experimentation I think deferring this to part 2 is the easiest 
solution here: even though there is a patch to remove the "base" property again for 
the I8042_MMIO_DEVICE, that solution still feels cleaner than scattering the 
migration compatibility across multiple series.


ATB,

Mark.