diff mbox

[4.18,REGRESSION,fix,2/2] i2c-cht-wc: Fix bq24190 supplier

Message ID 20180701094835.8775-3-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede July 1, 2018, 9:48 a.m. UTC
Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through
power_supply") moved the code to register a power_supply representing
the device supplying power to the type-C connector, from the fusb302
code to the generic tcpm code.

This has caused the power-supply registered by the fusb302 driver,
which determines how much current the bq24190 can draw, to change name
from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".

Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/i2c/busses/i2c-cht-wc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Wolfram Sang July 1, 2018, 10:29 a.m. UTC | #1
On Sun, Jul 01, 2018 at 11:48:35AM +0200, Hans de Goede wrote:
> Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through
> power_supply") moved the code to register a power_supply representing
> the device supplying power to the type-C connector, from the fusb302
> code to the generic tcpm code.
> 
> This has caused the power-supply registered by the fusb302 driver,
> which determines how much current the bq24190 can draw, to change name
> from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".
> 
> Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Wolfram Sang <wsa@the-dreams.de>
Guenter Roeck July 1, 2018, 2:02 p.m. UTC | #2
On 07/01/2018 02:48 AM, Hans de Goede wrote:
> Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through
> power_supply") moved the code to register a power_supply representing
> the device supplying power to the type-C connector, from the fusb302
> code to the generic tcpm code.
> 
> This has caused the power-supply registered by the fusb302 driver,
> which determines how much current the bq24190 can draw, to change name
> from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".
> 
> Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/i2c/busses/i2c-cht-wc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
> index 44cffad43701..c4d176f5ed79 100644
> --- a/drivers/i2c/busses/i2c-cht-wc.c
> +++ b/drivers/i2c/busses/i2c-cht-wc.c
> @@ -234,7 +234,8 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
>   	.name			= "cht_wc_ext_chrg_irq_chip",
>   };
>   
> -static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };
> +static const char * const bq24190_suppliers[] = {
> +	"tcpm-source-psy-i2c-fusb302" };
>   
>   static const struct property_entry bq24190_props[] = {
>   	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Thomson July 2, 2018, 10:34 a.m. UTC | #3
On 01 July 2018 10:49, Hans de Goede wrote:

> Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through
> power_supply") moved the code to register a power_supply representing
> the device supplying power to the type-C connector, from the fusb302
> code to the generic tcpm code.
> 
> This has caused the power-supply registered by the fusb302 driver,
> which determines how much current the bq24190 can draw, to change name
> from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".
> 
> Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/i2c/busses/i2c-cht-wc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
> index 44cffad43701..c4d176f5ed79 100644
> --- a/drivers/i2c/busses/i2c-cht-wc.c
> +++ b/drivers/i2c/busses/i2c-cht-wc.c
> @@ -234,7 +234,8 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
>  	.name			= "cht_wc_ext_chrg_irq_chip",
>  };
> 
> -static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };
> +static const char * const bq24190_suppliers[] = {
> +	"tcpm-source-psy-i2c-fusb302" };

In the TCPM code, the psy name is a combination of a fixed prefix
'tcpm-source-psy-' and the dev_name of the port controller to ensure we can
handle multiple fusb302 devices in one system. Is this hard coded name here
going to work for all uses of this driver?

> 
>  static const struct property_entry bq24190_props[] = {
>  	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),
> --
> 2.17.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede July 2, 2018, 10:50 a.m. UTC | #4
Hi,

On 02-07-18 12:34, Adam Thomson wrote:
> On 01 July 2018 10:49, Hans de Goede wrote:
> 
>> Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through
>> power_supply") moved the code to register a power_supply representing
>> the device supplying power to the type-C connector, from the fusb302
>> code to the generic tcpm code.
>>
>> This has caused the power-supply registered by the fusb302 driver,
>> which determines how much current the bq24190 can draw, to change name
>> from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".
>>
>> Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")
>> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/i2c/busses/i2c-cht-wc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
>> index 44cffad43701..c4d176f5ed79 100644
>> --- a/drivers/i2c/busses/i2c-cht-wc.c
>> +++ b/drivers/i2c/busses/i2c-cht-wc.c
>> @@ -234,7 +234,8 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
>>   	.name			= "cht_wc_ext_chrg_irq_chip",
>>   };
>>
>> -static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };
>> +static const char * const bq24190_suppliers[] = {
>> +	"tcpm-source-psy-i2c-fusb302" };
> 
> In the TCPM code, the psy name is a combination of a fixed prefix
> 'tcpm-source-psy-' and the dev_name of the port controller to ensure we can
> handle multiple fusb302 devices in one system. Is this hard coded name here
> going to work for all uses of this driver?

The i2c-cht-wc.c driver only instantiates a client for the bq24190 if
the ACPI INT33FE device is present:

         if (acpi_dev_present("INT33FE", NULL, -1)) {
                 board_info.irq = adap->client_irq;
                 adap->client = i2c_new_device(&adap->adapter, &board_info);

And the fusb302 boards with the INT33FE device is instantiated
by drivers/platform/x86/intel_cht_int33fe.c which sets i2c_board_info.dev_name
to "fusb302" when instantiating it, these boards are kinda special and we
need to manually instantiate multiple i2c devices on them, so hardcoding things
here is pretty much the only way to deal with this (and note that the name
was hardcoded before too).

Regards,

Hans



> 
>>
>>   static const struct property_entry bq24190_props[] = {
>>   	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),
>> --
>> 2.17.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Thomson July 2, 2018, 11:25 a.m. UTC | #5
On 02 July 2018 11:51, Hans de Goede wrote:

> Hi,

> 

> On 02-07-18 12:34, Adam Thomson wrote:

> > On 01 July 2018 10:49, Hans de Goede wrote:

> >

> >> Commit f2a8aa053c17 ("typec: tcpm: Represent source supply through

> >> power_supply") moved the code to register a power_supply representing

> >> the device supplying power to the type-C connector, from the fusb302

> >> code to the generic tcpm code.

> >>

> >> This has caused the power-supply registered by the fusb302 driver,

> >> which determines how much current the bq24190 can draw, to change name

> >> from "fusb302-typec-source" to "tcpm-source-psy-i2c-fusb302".

> >>

> >> Fixes: f2a8aa053c17 ("typec: tcpm: Represent source supply through...")

> >> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

> >> ---

> >>   drivers/i2c/busses/i2c-cht-wc.c | 3 ++-

> >>   1 file changed, 2 insertions(+), 1 deletion(-)

> >>

> >> diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c

> >> index 44cffad43701..c4d176f5ed79 100644

> >> --- a/drivers/i2c/busses/i2c-cht-wc.c

> >> +++ b/drivers/i2c/busses/i2c-cht-wc.c

> >> @@ -234,7 +234,8 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {

> >>   	.name			= "cht_wc_ext_chrg_irq_chip",

> >>   };

> >>

> >> -static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };

> >> +static const char * const bq24190_suppliers[] = {

> >> +	"tcpm-source-psy-i2c-fusb302" };

> >

> > In the TCPM code, the psy name is a combination of a fixed prefix

> > 'tcpm-source-psy-' and the dev_name of the port controller to ensure we can

> > handle multiple fusb302 devices in one system. Is this hard coded name here

> > going to work for all uses of this driver?

> 

> The i2c-cht-wc.c driver only instantiates a client for the bq24190 if

> the ACPI INT33FE device is present:

> 

>          if (acpi_dev_present("INT33FE", NULL, -1)) {

>                  board_info.irq = adap->client_irq;

>                  adap->client = i2c_new_device(&adap->adapter, &board_info);

> 

> And the fusb302 boards with the INT33FE device is instantiated

> by drivers/platform/x86/intel_cht_int33fe.c which sets i2c_board_info.dev_name

> to "fusb302" when instantiating it, these boards are kinda special and we

> need to manually instantiate multiple i2c devices on them, so hardcoding things

> here is pretty much the only way to deal with this (and note that the name

> was hardcoded before too).


Ok, that's fine with me, and it's clear this has been considered. Was just an
initial concern due to the new TCPM psy usage and naming.

FWIW:

Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
index 44cffad43701..c4d176f5ed79 100644
--- a/drivers/i2c/busses/i2c-cht-wc.c
+++ b/drivers/i2c/busses/i2c-cht-wc.c
@@ -234,7 +234,8 @@  static const struct irq_chip cht_wc_i2c_irq_chip = {
 	.name			= "cht_wc_ext_chrg_irq_chip",
 };
 
-static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };
+static const char * const bq24190_suppliers[] = {
+	"tcpm-source-psy-i2c-fusb302" };
 
 static const struct property_entry bq24190_props[] = {
 	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),