Re: [PATCH 05/14] hw/i2c/Kconfig: Add an entry for the SMBus

[Corey Minyard]

On Wed, Jan 01, 2020 at 11:25:42AM +0100, Philippe Mathieu-Daudé wrote:
> On 12/31/19 8:16 PM, Corey Minyard wrote:
> > On Tue, Dec 31, 2019 at 07:32:07PM +0100, Philippe Mathieu-Daudé wrote:
> > > The System Management Bus is more or less a derivative of the I2C
> > > bus, thus the Kconfig entry depends of I2C.
> > > Not all boards providing an I2C bus support SMBus.
> > > Use two different Kconfig entries to be able to select I2C without
> > > selecting SMBus.
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > > ---
> > > Cc: Corey Minyard <address@hidden>
> > > ---
> > >   default-configs/mips-softmmu-common.mak | 1 +
> > >   hw/i2c/Kconfig                          | 8 ++++++--
> > >   hw/i2c/Makefile.objs                    | 3 ++-
> > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/default-configs/mips-softmmu-common.mak 
> > > b/default-configs/mips-softmmu-common.mak
> > > index da29c6c0b2..ac76d944b8 100644
> > > --- a/default-configs/mips-softmmu-common.mak
> > > +++ b/default-configs/mips-softmmu-common.mak
> > > @@ -37,6 +37,7 @@ CONFIG_R4K=y
> > >   CONFIG_MALTA=y
> > >   CONFIG_PCNET_PCI=y
> > >   CONFIG_MIPSSIM=y
> > > +CONFIG_SMBUS=y
> > 
> > Why is the above necessary?  Wouldn't CONFIG_ACPI_SMBUS=y below cause
> > this to be done?
> 
> Yes you are correct!
> 
> > 
> > >   CONFIG_ACPI_SMBUS=y
> > >   CONFIG_SMBUS_EEPROM=y
> > >   CONFIG_TEST_DEVICES=y
> > > diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> > > index 2bbd395813..09642a6dcb 100644
> > > --- a/hw/i2c/Kconfig
> > > +++ b/hw/i2c/Kconfig
> > > @@ -1,9 +1,13 @@
> > >   config I2C
> > >       bool
> > > +config SMBUS
> > > +    bool
> > > +    select I2C
> > > +
> > >   config SMBUS_EEPROM
> > >       bool
> > > -    depends on I2C
> > > +    select SMBUS
> > >   config VERSATILE_I2C
> > >       bool
> > > @@ -11,7 +15,7 @@ config VERSATILE_I2C
> > >   config ACPI_SMBUS
> > >       bool
> > > -    select I2C
> > > +    select SMBUS
> > >   config BITBANG_I2C
> > >       bool
> > > diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
> > > index d7073a401f..cbbc8507a3 100644
> > > --- a/hw/i2c/Makefile.objs
> > > +++ b/hw/i2c/Makefile.objs
> > > @@ -1,4 +1,5 @@
> > > -common-obj-$(CONFIG_I2C) += core.o smbus_slave.o smbus_master.o
> > > +common-obj-$(CONFIG_I2C) += core.o
> > > +common-obj-$(CONFIG_SMBUS) += smbus_slave.o smbus_master.o
> > >   common-obj-$(CONFIG_SMBUS_EEPROM) += smbus_eeprom.o
> > >   common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o
> > >   common-obj-$(CONFIG_ACPI_X86_ICH) += smbus_ich9.o
> > 
> > There is some messiness with ICH, but it appears that everything will
> > work correctly there and this patch is not the right place to fix it.
> 
> I did some cleaning with the ICH9 chipset last year, I need to find some
> time to refresh it.
> 
> > This looks fine and avoids including smbus code when it is not
> > necessary.  With the MIPS config item removed (assuming it is not
> > necessary):
> > 
> > Reviewed-by: Corey Minyard <address@hidden>
> 
> Thanks!
> 
> > 
> > I can take it into my tree if you like.
> 
> Sure, do you mind cleaning default-configs/mips-softmmu-common.mak or you
> prefer a respin of this single patch?

I removed that piece and have it queued.

Thanks,

-corey

> 
> > 
> > Thanks,
> > 
> > -corey
> > 
> > > -- 
> > > 2.21.0
> > > 
> > 
>