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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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é <philmd@redhat.com>
>> ---
>> Cc: Corey Minyard <cminyard@mvista.com>
>> ---
>>   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 <cminyard@mvista.com>

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?

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