diff mbox series

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

Message ID 20191231183216.6781-6-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw: Fix various --without-default-devices issues | expand

Commit Message

Philippe Mathieu-Daudé Dec. 31, 2019, 6:32 p.m. UTC
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(-)

Comments

Corey Minyard Dec. 31, 2019, 7:16 p.m. UTC | #1
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?

>  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.

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>

I can take it into my tree if you like.

Thanks,

-corey

> -- 
> 2.21.0
>
Philippe Mathieu-Daudé Jan. 1, 2020, 10:25 a.m. UTC | #2
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
>>
>
Corey Minyard Jan. 1, 2020, 4:15 p.m. UTC | #3
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,

-corey

> 
> > 
> > Thanks,
> > 
> > -corey
> > 
> > > -- 
> > > 2.21.0
> > > 
> > 
>
Philippe Mathieu-Daudé Jan. 1, 2020, 4:21 p.m. UTC | #4
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.
diff mbox series

Patch

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
 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