diff mbox

[4.18,REGRESSION,fix,1/2] typec: tcpm: Correctly report power_supply current and voltage for non pd supply

Message ID 20180701094835.8775-2-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 so that we have a psy reporting the
supply voltage and current for all tcpm devices.

This broke the reporting of current and voltage through the psy interface
when supplied by a a non pd supply (5V, current as reported by
get_current_limit). The cause of this breakage is port->supply_voltage
and port->current_limit not being set in that case.

This commit fixes this by setting port->supply_voltage and
port->current_limit from tcpm_set_current_limit().

This commit also removes setting supply_voltage and current_limit
from tcpm_reset_port() as that calls tcpm_set_current_limit(0, 0)
which now already sets these to 0.

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/usb/typec/tcpm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Guenter Roeck July 1, 2018, 2:01 p.m. UTC | #1
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 so that we have a psy reporting the
> supply voltage and current for all tcpm devices.
> 
> This broke the reporting of current and voltage through the psy interface
> when supplied by a a non pd supply (5V, current as reported by
> get_current_limit). The cause of this breakage is port->supply_voltage
> and port->current_limit not being set in that case.
> 
> This commit fixes this by setting port->supply_voltage and
> port->current_limit from tcpm_set_current_limit().
> 
> This commit also removes setting supply_voltage and current_limit
> from tcpm_reset_port() as that calls tcpm_set_current_limit(0, 0)
> which now already sets these to 0.
> 
> 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/usb/typec/tcpm.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 8a201dd53d36..316959c26db9 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -724,6 +724,9 @@ static int tcpm_set_current_limit(struct tcpm_port *port, u32 max_ma, u32 mv)
>   
>   	tcpm_log(port, "Setting voltage/current limit %u mV %u mA", mv, max_ma);
>   
> +	port->supply_voltage = mv;
> +	port->current_limit = max_ma;
> +
>   	if (port->tcpc->set_current_limit)
>   		ret = port->tcpc->set_current_limit(port->tcpc, max_ma, mv);
>   
> @@ -2594,8 +2597,6 @@ static void tcpm_reset_port(struct tcpm_port *port)
>   	tcpm_set_attached_state(port, false);
>   	port->try_src_count = 0;
>   	port->try_snk_count = 0;
> -	port->supply_voltage = 0;
> -	port->current_limit = 0;
>   	port->usb_type = POWER_SUPPLY_USB_TYPE_C;
>   
>   	power_supply_changed(port->psy);
> 

--
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:28 a.m. UTC | #2
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 so that we have a psy reporting the
> supply voltage and current for all tcpm devices.
> 
> This broke the reporting of current and voltage through the psy interface
> when supplied by a a non pd supply (5V, current as reported by
> get_current_limit). The cause of this breakage is port->supply_voltage
> and port->current_limit not being set in that case.
> 
> This commit fixes this by setting port->supply_voltage and
> port->current_limit from tcpm_set_current_limit().
> 
> This commit also removes setting supply_voltage and current_limit
> from tcpm_reset_port() as that calls tcpm_set_current_limit(0, 0)
> which now already sets these to 0.
> 
> 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>

Apologies for this, and thanks for the fix.

Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
>  drivers/usb/typec/tcpm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 8a201dd53d36..316959c26db9 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -724,6 +724,9 @@ static int tcpm_set_current_limit(struct tcpm_port *port,
> u32 max_ma, u32 mv)
> 
>  	tcpm_log(port, "Setting voltage/current limit %u mV %u mA", mv, max_ma);
> 
> +	port->supply_voltage = mv;
> +	port->current_limit = max_ma;
> +
>  	if (port->tcpc->set_current_limit)
>  		ret = port->tcpc->set_current_limit(port->tcpc, max_ma, mv);
> 
> @@ -2594,8 +2597,6 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  	tcpm_set_attached_state(port, false);
>  	port->try_src_count = 0;
>  	port->try_snk_count = 0;
> -	port->supply_voltage = 0;
> -	port->current_limit = 0;
>  	port->usb_type = POWER_SUPPLY_USB_TYPE_C;
> 
>  	power_supply_changed(port->psy);
> --
> 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 | #3
Hi,

On 02-07-18 12:28, 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 so that we have a psy reporting the
>> supply voltage and current for all tcpm devices.
>>
>> This broke the reporting of current and voltage through the psy interface
>> when supplied by a a non pd supply (5V, current as reported by
>> get_current_limit). The cause of this breakage is port->supply_voltage
>> and port->current_limit not being set in that case.
>>
>> This commit fixes this by setting port->supply_voltage and
>> port->current_limit from tcpm_set_current_limit().
>>
>> This commit also removes setting supply_voltage and current_limit
>> from tcpm_reset_port() as that calls tcpm_set_current_limit(0, 0)
>> which now already sets these to 0.
>>
>> 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>
> 
> Apologies for this, and thanks for the fix.

No problem, you're welcome and thank you for the review.

Regards,

Hans


> 
> Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> 
>> ---
>>   drivers/usb/typec/tcpm.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
>> index 8a201dd53d36..316959c26db9 100644
>> --- a/drivers/usb/typec/tcpm.c
>> +++ b/drivers/usb/typec/tcpm.c
>> @@ -724,6 +724,9 @@ static int tcpm_set_current_limit(struct tcpm_port *port,
>> u32 max_ma, u32 mv)
>>
>>   	tcpm_log(port, "Setting voltage/current limit %u mV %u mA", mv, max_ma);
>>
>> +	port->supply_voltage = mv;
>> +	port->current_limit = max_ma;
>> +
>>   	if (port->tcpc->set_current_limit)
>>   		ret = port->tcpc->set_current_limit(port->tcpc, max_ma, mv);
>>
>> @@ -2594,8 +2597,6 @@ static void tcpm_reset_port(struct tcpm_port *port)
>>   	tcpm_set_attached_state(port, false);
>>   	port->try_src_count = 0;
>>   	port->try_snk_count = 0;
>> -	port->supply_voltage = 0;
>> -	port->current_limit = 0;
>>   	port->usb_type = POWER_SUPPLY_USB_TYPE_C;
>>
>>   	power_supply_changed(port->psy);
>> --
>> 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
diff mbox

Patch

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 8a201dd53d36..316959c26db9 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -724,6 +724,9 @@  static int tcpm_set_current_limit(struct tcpm_port *port, u32 max_ma, u32 mv)
 
 	tcpm_log(port, "Setting voltage/current limit %u mV %u mA", mv, max_ma);
 
+	port->supply_voltage = mv;
+	port->current_limit = max_ma;
+
 	if (port->tcpc->set_current_limit)
 		ret = port->tcpc->set_current_limit(port->tcpc, max_ma, mv);
 
@@ -2594,8 +2597,6 @@  static void tcpm_reset_port(struct tcpm_port *port)
 	tcpm_set_attached_state(port, false);
 	port->try_src_count = 0;
 	port->try_snk_count = 0;
-	port->supply_voltage = 0;
-	port->current_limit = 0;
 	port->usb_type = POWER_SUPPLY_USB_TYPE_C;
 
 	power_supply_changed(port->psy);