Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1704826 > unrolled thread

[PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon Hans de Goede <hdegoede@redhat.com> - 2017-08-06 14:40 +0200
    Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2  charger detection through extcon Guenter Roeck <linux@roeck-us.net> - 2017-08-06 16:30 +0200
      Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2  charger detection through extcon Hans de Goede <hdegoede@redhat.com> - 2017-08-06 16:40 +0200
        Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2  charger detection through extcon Guenter Roeck <linux@roeck-us.net> - 2017-08-06 17:00 +0200

#1704826 — [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
USB2 charger detection on these systems. Note that the "fcs,extcon-name"
property name is only for kernel internal use by X86/ACPI platform code
and as such is NOT documented in the devicetree bindings.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index be454b5ff6c7..1d8c9b66df2f 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 {
 	fusb302_tcpc_dev->init = tcpm_init;
 	fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+	fusb302_tcpc_dev->get_usb2_current_limit =
+		tcpm_get_usb2_current_limit_extcon;
 	fusb302_tcpc_dev->set_cc = tcpm_set_cc;
 	fusb302_tcpc_dev->get_cc = tcpm_get_cc;
 	fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
 	struct device *dev = &client->dev;
+	const char *name;
 	int ret = 0;
 	u32 val;
 
@@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
 	if (device_property_read_u32(dev, "fcs,operating-snk-mw", &val) == 0)
 		chip->tcpc_config.operating_snk_mw = val;
 
+	/*
+	 * Devicetree platforms should get extcon via phandle (not yet
+	 * supported). On ACPI platforms, we get the name from a device prop.
+	 * This device prop is for kernel internal use only and is expected
+	 * to be set by the platform code which also registers the i2c client
+	 * for the fusb302.
+	 */
+	if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
+		chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
+		if (!chip->tcpc_dev.usb2_extcon)
+			return -EPROBE_DEFER;
+	}
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.13.3

[toc] | [next] | [standalone]

#1704858 — Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

On 08/06/2017 05:35 AM, Hans de Goede wrote:
> The fusb302 port-controller relies on an external device doing USB2
> charger-type detection.
> 
> The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
> X86/ACPI platforms already has a charger-type detection driver which
> uses extcon to communicate the detected charger-type.
> 
> This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
> USB2 charger detection on these systems. Note that the "fcs,extcon-name"
> property name is only for kernel internal use by X86/ACPI platform code
> and as such is NOT documented in the devicetree bindings.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/staging/typec/fusb302/fusb302.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
> index be454b5ff6c7..1d8c9b66df2f 100644
> --- a/drivers/staging/typec/fusb302/fusb302.c
> +++ b/drivers/staging/typec/fusb302/fusb302.c
> @@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>   {
>   	fusb302_tcpc_dev->init = tcpm_init;
>   	fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
> +	fusb302_tcpc_dev->get_usb2_current_limit =
> +		tcpm_get_usb2_current_limit_extcon;
>   	fusb302_tcpc_dev->set_cc = tcpm_set_cc;
>   	fusb302_tcpc_dev->get_cc = tcpm_get_cc;
>   	fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
> @@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
>   	struct fusb302_chip *chip;
>   	struct i2c_adapter *adapter;
>   	struct device *dev = &client->dev;
> +	const char *name;
>   	int ret = 0;
>   	u32 val;
>   
> @@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
>   	if (device_property_read_u32(dev, "fcs,operating-snk-mw", &val) == 0)
>   		chip->tcpc_config.operating_snk_mw = val;
>   
> +	/*
> +	 * Devicetree platforms should get extcon via phandle (not yet
> +	 * supported). On ACPI platforms, we get the name from a device prop.
> +	 * This device prop is for kernel internal use only and is expected
> +	 * to be set by the platform code which also registers the i2c client
> +	 * for the fusb302.
> +	 */
> +	if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {

Those new devicetree properties need to be documented and approved by dt maintainers.

> +		chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
> +		if (!chip->tcpc_dev.usb2_extcon)

extcon_get_extcon_dev() can also return an ERR_PTR.

As before, I am not really happy typing this into tcpm via tcpc_dev.
Until we have a better solution, I would prefer for that code to stay with the fusb302
code.

> +			return -EPROBE_DEFER;
> +	}
> +
>   	ret = fusb302_debugfs_init(chip);
>   	if (ret < 0)
>   		return ret;
> 

[toc] | [prev] | [next] | [standalone]

#1704862 — Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

Hi,

On 06-08-17 16:22, Guenter Roeck wrote:
> On 08/06/2017 05:35 AM, Hans de Goede wrote:
>> The fusb302 port-controller relies on an external device doing USB2
>> charger-type detection.
>>
>> The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
>> X86/ACPI platforms already has a charger-type detection driver which
>> uses extcon to communicate the detected charger-type.
>>
>> This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
>> USB2 charger detection on these systems. Note that the "fcs,extcon-name"
>> property name is only for kernel internal use by X86/ACPI platform code
>> and as such is NOT documented in the devicetree bindings.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/staging/typec/fusb302/fusb302.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
>> index be454b5ff6c7..1d8c9b66df2f 100644
>> --- a/drivers/staging/typec/fusb302/fusb302.c
>> +++ b/drivers/staging/typec/fusb302/fusb302.c
>> @@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>>   {
>>       fusb302_tcpc_dev->init = tcpm_init;
>>       fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
>> +    fusb302_tcpc_dev->get_usb2_current_limit =
>> +        tcpm_get_usb2_current_limit_extcon;
>>       fusb302_tcpc_dev->set_cc = tcpm_set_cc;
>>       fusb302_tcpc_dev->get_cc = tcpm_get_cc;
>>       fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
>> @@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
>>       struct fusb302_chip *chip;
>>       struct i2c_adapter *adapter;
>>       struct device *dev = &client->dev;
>> +    const char *name;
>>       int ret = 0;
>>       u32 val;
>> @@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
>>       if (device_property_read_u32(dev, "fcs,operating-snk-mw", &val) == 0)
>>           chip->tcpc_config.operating_snk_mw = val;
>> +    /*
>> +     * Devicetree platforms should get extcon via phandle (not yet
>> +     * supported). On ACPI platforms, we get the name from a device prop.
>> +     * This device prop is for kernel internal use only and is expected
>> +     * to be set by the platform code which also registers the i2c client
>> +     * for the fusb302.
>> +     */
>> +    if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
> 
> Those new devicetree properties need to be documented and approved by dt maintainers.

As indicated by the comment, this property should not be used in the devicetree
case, notice this is a device-property and not an of property and since it is
not intended to be used with devicetree at all (in devicetree a phandle rather
then a name would be used), it has no place under Documentation/devicetree at all.

Also there is no current binding documentation for the "fcs,fusb302" compatible
and the weird "fcs,int_n" gpio which really is an irq which it already uses.


> 
>> +        chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
>> +        if (!chip->tcpc_dev.usb2_extcon)
> 
> extcon_get_extcon_dev() can also return an ERR_PTR.
> 
> As before, I am not really happy typing this into tcpm via tcpc_dev.
> Until we have a better solution, I would prefer for that code to stay with the fusb302
> code.

Ok, I tried to make this all re-usable since I think other port-controller drivers
will need something similar, but I can kill the entire tcpm-helpers.c file in v2
and then put everything in fusb302.c. I take it that that is what you want ?

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1704868 — Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

On 08/06/2017 07:36 AM, Hans de Goede wrote:
> Hi,
> 
> On 06-08-17 16:22, Guenter Roeck wrote:
>> On 08/06/2017 05:35 AM, Hans de Goede wrote:
>>> The fusb302 port-controller relies on an external device doing USB2
>>> charger-type detection.
>>>
>>> The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
>>> X86/ACPI platforms already has a charger-type detection driver which
>>> uses extcon to communicate the detected charger-type.
>>>
>>> This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
>>> USB2 charger detection on these systems. Note that the "fcs,extcon-name"
>>> property name is only for kernel internal use by X86/ACPI platform code
>>> and as such is NOT documented in the devicetree bindings.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/staging/typec/fusb302/fusb302.c | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
>>> index be454b5ff6c7..1d8c9b66df2f 100644
>>> --- a/drivers/staging/typec/fusb302/fusb302.c
>>> +++ b/drivers/staging/typec/fusb302/fusb302.c
>>> @@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>>>   {
>>>       fusb302_tcpc_dev->init = tcpm_init;
>>>       fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
>>> +    fusb302_tcpc_dev->get_usb2_current_limit =
>>> +        tcpm_get_usb2_current_limit_extcon;
>>>       fusb302_tcpc_dev->set_cc = tcpm_set_cc;
>>>       fusb302_tcpc_dev->get_cc = tcpm_get_cc;
>>>       fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
>>> @@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
>>>       struct fusb302_chip *chip;
>>>       struct i2c_adapter *adapter;
>>>       struct device *dev = &client->dev;
>>> +    const char *name;
>>>       int ret = 0;
>>>       u32 val;
>>> @@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
>>>       if (device_property_read_u32(dev, "fcs,operating-snk-mw", &val) == 0)
>>>           chip->tcpc_config.operating_snk_mw = val;
>>> +    /*
>>> +     * Devicetree platforms should get extcon via phandle (not yet
>>> +     * supported). On ACPI platforms, we get the name from a device prop.
>>> +     * This device prop is for kernel internal use only and is expected
>>> +     * to be set by the platform code which also registers the i2c client
>>> +     * for the fusb302.
>>> +     */
>>> +    if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
>>
>> Those new devicetree properties need to be documented and approved by dt maintainers.
> 
> As indicated by the comment, this property should not be used in the devicetree
> case, notice this is a device-property and not an of property and since it is
> not intended to be used with devicetree at all (in devicetree a phandle rather
> then a name would be used), it has no place under Documentation/devicetree at all.
> 

Ok. I thought device properties were supposed to unify dt and non-dt situations,
but apparently not. Oh well :-(.

> Also there is no current binding documentation for the "fcs,fusb302" compatible
> and the weird "fcs,int_n" gpio which really is an irq which it already uses.
> 

Yes, one of those TODO items for moving the code out of staging. "fcs," should
possibly be "fusb302,", and int_n _is_ weird.

> 
>>
>>> +        chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
>>> +        if (!chip->tcpc_dev.usb2_extcon)
>>
>> extcon_get_extcon_dev() can also return an ERR_PTR.
>>
>> As before, I am not really happy typing this into tcpm via tcpc_dev.
>> Until we have a better solution, I would prefer for that code to stay with the fusb302
>> code.
> 
> Ok, I tried to make this all re-usable since I think other port-controller drivers
> will need something similar, but I can kill the entire tcpm-helpers.c file in v2
> and then put everything in fusb302.c. I take it that that is what you want ?
> 
Yes, please.

Also, please copy Yueyao Zhu for the fusb302 changes in v2.

Thanks,
Guenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web