On Mon, Feb 27, 2023 at 12:53:23PM +0100, Philippe Mathieu-Daudé wrote: > On 19/2/23 15:21, Corey Minyard wrote: > > On Sun, Feb 19, 2023 at 02:45:44PM +0100, Philippe Mathieu-Daudé wrote: > > > On 18/2/23 21:25, Corey Minyard wrote: > > > > On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote: > > > > > ich9_smb_init() is a legacy init function, so modernize the code. > > > > > > > > > > Note that the smb_io_base parameter was unused. > > > > > > > > Acked-by: Corey Minyard <cminy...@mvista.com> > > > > > > > > > > > > > > Signed-off-by: Bernhard Beschow <shen...@gmail.com> > > > > > --- > > > > > include/hw/i386/ich9.h | 1 - > > > > > hw/i2c/smbus_ich9.c | 13 +++---------- > > > > > hw/i386/pc_q35.c | 11 ++++++++--- > > > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > > > > > > > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h > > > > > index 05464f6965..52ea116f44 100644 > > > > > --- a/include/hw/i386/ich9.h > > > > > +++ b/include/hw/i386/ich9.h > > > > > @@ -9,7 +9,6 @@ > > > > > #include "qom/object.h" > > > > > void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled); > > > > > -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base); > > > > > void ich9_generate_smi(void); > > > > > diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c > > > > > index d29c0f6ffa..f0dd3cb147 100644 > > > > > --- a/hw/i2c/smbus_ich9.c > > > > > +++ b/hw/i2c/smbus_ich9.c > > > > > @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, > > > > > Error **errp) > > > > > pm_smbus_init(&d->qdev, &s->smb, false); > > > > > pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, > > > > > PCI_BASE_ADDRESS_SPACE_IO, > > > > > &s->smb.io); > > > > > + > > > > > + s->smb.set_irq = ich9_smb_set_irq; > > > > > + s->smb.opaque = s; > > > > > > Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque > > > arguments? > > > > That would be nice, but the other two user of this function, > > hw/isa/vt82c686.c and hw/acpi/piix4.c, don't use these fields. > > I doubt we are getting any new users. I'm fine either way, but the > > value is not large. > > The value is in enforcing stricter APIs (providing good examples).
I agree, and the change would be good, but IMHO it's beyond the scope of this change and would slow things down. I'll prepare a patch that does this. Thanks, -corey > > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > >
