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

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

On 1/1/20 5:15 PM, Corey Minyard wrote:
> 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é <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?
> 
> I removed that piece and have it queued.

Thanks!

Phil.