Add usb-typec detection support for the Intel LGM SoC based boards.
Original driver is not supporting usb detection on Intel LGM SoC based boards
then we debugged and fixed the issue, but before sending our patches Mr.Krzyszto
has sent the same kind of patches, so I have rebased over his latest patches
which is present in maintainer tree.
Built and tested it's working fine, overthat created the new patch.
Thanks for Krzyszto and Chanwoo Choi
---
v2:
- Krzyszto review comments update
- squash my previous patches 1 to 5 as single patch
- add extcon_set_property_capability for EXTCON_USB and EXTCON_PROP_USB_TYPEC_POLARITY
Reference to mail discussion:
https://www.mail-archive.com/[email protected]/msg2281723.html
Ramuthevar Vadivel Murugan (2):
extcon: ptn5150: Switch to GENMASK() and BIT() macros
extcon: ptn5150: Set the VBUS and POLARITY property state
drivers/extcon/extcon-ptn5150.c | 49 ++++++++++++++---------------------------
1 file changed, 17 insertions(+), 32 deletions(-)
--
2.11.0
From: Ramuthevar Vadivel Murugan <[email protected]>
Set the VBUS and POLARITY property state.
Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
---
drivers/extcon/extcon-ptn5150.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
index 8b930050a3f1..b5217a61615c 100644
--- a/drivers/extcon/extcon-ptn5150.c
+++ b/drivers/extcon/extcon-ptn5150.c
@@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c)
return ret;
}
+ extcon_set_property_capability(info->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS);
+ extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS);
+ extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_TYPEC_POLARITY);
/* Initialize PTN5150 device and print vendor id and version id */
ret = ptn5150_init_dev_type(info);
if (ret)
--
2.11.0
From: Ramuthevar Vadivel Murugan <[email protected]>
Switch to GENMASK() and BIT() macros.
Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
drivers/extcon/extcon-ptn5150.c | 43 +++++++++++------------------------------
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
index 8ba706fad887..8b930050a3f1 100644
--- a/drivers/extcon/extcon-ptn5150.c
+++ b/drivers/extcon/extcon-ptn5150.c
@@ -7,6 +7,7 @@
// Author: Vijai Kumar K <[email protected]>
// Copyright (c) 2020 Krzysztof Kozlowski <[email protected]>
+#include <linux/bitfield.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
@@ -35,29 +36,13 @@ enum ptn5150_reg {
#define PTN5150_UFP_ATTACHED 0x2
/* Define PTN5150 MASK/SHIFT constant */
-#define PTN5150_REG_DEVICE_ID_VENDOR_SHIFT 0
-#define PTN5150_REG_DEVICE_ID_VENDOR_MASK \
- (0x3 << PTN5150_REG_DEVICE_ID_VENDOR_SHIFT)
+#define PTN5150_REG_DEVICE_ID_VERSION GENMASK(7, 3)
+#define PTN5150_REG_DEVICE_ID_VENDOR GENMASK(2, 0)
-#define PTN5150_REG_DEVICE_ID_VERSION_SHIFT 3
-#define PTN5150_REG_DEVICE_ID_VERSION_MASK \
- (0x1f << PTN5150_REG_DEVICE_ID_VERSION_SHIFT)
-
-#define PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT 2
-#define PTN5150_REG_CC_PORT_ATTACHMENT_MASK \
- (0x7 << PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT)
-
-#define PTN5150_REG_CC_VBUS_DETECTION_SHIFT 7
-#define PTN5150_REG_CC_VBUS_DETECTION_MASK \
- (0x1 << PTN5150_REG_CC_VBUS_DETECTION_SHIFT)
-
-#define PTN5150_REG_INT_CABLE_ATTACH_SHIFT 0
-#define PTN5150_REG_INT_CABLE_ATTACH_MASK \
- (0x1 << PTN5150_REG_INT_CABLE_ATTACH_SHIFT)
-
-#define PTN5150_REG_INT_CABLE_DETACH_SHIFT 1
-#define PTN5150_REG_INT_CABLE_DETACH_MASK \
- (0x1 << PTN5150_REG_CC_CABLE_DETACH_SHIFT)
+#define PTN5150_REG_CC_PORT_ATTACHMENT GENMASK(4, 2)
+#define PTN5150_REG_CC_VBUS_DETECTION BIT(7)
+#define PTN5150_REG_INT_CABLE_ATTACH_MASK BIT(0)
+#define PTN5150_REG_INT_CABLE_DETACH_MASK BIT(1)
struct ptn5150_info {
struct device *dev;
@@ -95,9 +80,7 @@ static void ptn5150_check_state(struct ptn5150_info *info)
return;
}
- port_status = ((reg_data &
- PTN5150_REG_CC_PORT_ATTACHMENT_MASK) >>
- PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT);
+ port_status = FIELD_GET(PTN5150_REG_CC_PORT_ATTACHMENT, reg_data);
switch (port_status) {
case PTN5150_DFP_ATTACHED:
@@ -107,8 +90,7 @@ static void ptn5150_check_state(struct ptn5150_info *info)
break;
case PTN5150_UFP_ATTACHED:
extcon_set_state_sync(info->edev, EXTCON_USB, false);
- vbus = ((reg_data & PTN5150_REG_CC_VBUS_DETECTION_MASK) >>
- PTN5150_REG_CC_VBUS_DETECTION_SHIFT);
+ vbus = FIELD_GET(PTN5150_REG_CC_VBUS_DETECTION, reg_data);
if (vbus)
gpiod_set_value_cansleep(info->vbus_gpiod, 0);
else
@@ -191,11 +173,8 @@ static int ptn5150_init_dev_type(struct ptn5150_info *info)
return -EINVAL;
}
- vendor_id = ((reg_data & PTN5150_REG_DEVICE_ID_VENDOR_MASK) >>
- PTN5150_REG_DEVICE_ID_VENDOR_SHIFT);
- version_id = ((reg_data & PTN5150_REG_DEVICE_ID_VERSION_MASK) >>
- PTN5150_REG_DEVICE_ID_VERSION_SHIFT);
-
+ vendor_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VENDOR, reg_data);
+ version_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VERSION, reg_data);
dev_dbg(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n",
version_id, vendor_id);
--
2.11.0
Hi,
You better to change the 'state' word to 'capability'.
Actually, this patch doesn't change the value of property.
It set the capability value of property.
"Set the VBUS and POLARITY property capability"
On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote:
> From: Ramuthevar Vadivel Murugan <[email protected]>
>
> Set the VBUS and POLARITY property state.
ditto. Need to change the work from 'state' and 'capability'.
>
> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
> ---
> drivers/extcon/extcon-ptn5150.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
> index 8b930050a3f1..b5217a61615c 100644
> --- a/drivers/extcon/extcon-ptn5150.c
> +++ b/drivers/extcon/extcon-ptn5150.c
> @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c)
> return ret;
> }
>
> + extcon_set_property_capability(info->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_TYPEC_POLARITY);
Need to add blank line.
I understood that you set the property capability
because of get_flipped() function of your patch[1].
But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY
when changing the state of EXTCON_USB_HOST. The polarity property value is always
zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns
always the same *flipped value.
EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0
EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0
If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior,
you don't need to get the property value from extcon consumer driver
like drivers/phy/phy-lgm-usb.c.
Actually, I don't understand why you don't handle the value
of EXTCON_PROP_USB_TYPEC_POLARITY.
Or, are there any case of what drivers/phy/phy-lgm-usb.c
uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property
in the future?
So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability
for the extensibility in order to use other extcon device on later?
[1] https://www.spinics.net/lists/devicetree/msg371828.html
+static int get_flipped(struct tca_apb *ta, bool *flipped)
+{
+ union extcon_property_value property;
+ int ret;
+
+ ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_TYPEC_POLARITY, &property);
+ if (ret) {
+ dev_err(ta->phy.dev, "no polarity property from extcon\n");
+ return ret;
+ }
+
+ *flipped = property.intval;
+
+ return ret;
+}
> /* Initialize PTN5150 device and print vendor id and version id */
> ret = ptn5150_init_dev_type(info);
> if (ret)
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote:
> From: Ramuthevar Vadivel Murugan <[email protected]>
>
> Switch to GENMASK() and BIT() macros.
>
> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/extcon/extcon-ptn5150.c | 43 +++++++++++------------------------------
> 1 file changed, 11 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
> index 8ba706fad887..8b930050a3f1 100644
> --- a/drivers/extcon/extcon-ptn5150.c
> +++ b/drivers/extcon/extcon-ptn5150.c
> @@ -7,6 +7,7 @@
> // Author: Vijai Kumar K <[email protected]>
> // Copyright (c) 2020 Krzysztof Kozlowski <[email protected]>
>
> +#include <linux/bitfield.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> @@ -35,29 +36,13 @@ enum ptn5150_reg {
> #define PTN5150_UFP_ATTACHED 0x2
>
> /* Define PTN5150 MASK/SHIFT constant */
> -#define PTN5150_REG_DEVICE_ID_VENDOR_SHIFT 0
> -#define PTN5150_REG_DEVICE_ID_VENDOR_MASK \
> - (0x3 << PTN5150_REG_DEVICE_ID_VENDOR_SHIFT)
> +#define PTN5150_REG_DEVICE_ID_VERSION GENMASK(7, 3)
> +#define PTN5150_REG_DEVICE_ID_VENDOR GENMASK(2, 0)
>
> -#define PTN5150_REG_DEVICE_ID_VERSION_SHIFT 3
> -#define PTN5150_REG_DEVICE_ID_VERSION_MASK \
> - (0x1f << PTN5150_REG_DEVICE_ID_VERSION_SHIFT)
> -
> -#define PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT 2
> -#define PTN5150_REG_CC_PORT_ATTACHMENT_MASK \
> - (0x7 << PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT)
> -
> -#define PTN5150_REG_CC_VBUS_DETECTION_SHIFT 7
> -#define PTN5150_REG_CC_VBUS_DETECTION_MASK \
> - (0x1 << PTN5150_REG_CC_VBUS_DETECTION_SHIFT)
> -
> -#define PTN5150_REG_INT_CABLE_ATTACH_SHIFT 0
> -#define PTN5150_REG_INT_CABLE_ATTACH_MASK \
> - (0x1 << PTN5150_REG_INT_CABLE_ATTACH_SHIFT)
> -
> -#define PTN5150_REG_INT_CABLE_DETACH_SHIFT 1
> -#define PTN5150_REG_INT_CABLE_DETACH_MASK \
> - (0x1 << PTN5150_REG_CC_CABLE_DETACH_SHIFT)
> +#define PTN5150_REG_CC_PORT_ATTACHMENT GENMASK(4, 2)
> +#define PTN5150_REG_CC_VBUS_DETECTION BIT(7)
> +#define PTN5150_REG_INT_CABLE_ATTACH_MASK BIT(0)
> +#define PTN5150_REG_INT_CABLE_DETACH_MASK BIT(1)
>
> struct ptn5150_info {
> struct device *dev;
> @@ -95,9 +80,7 @@ static void ptn5150_check_state(struct ptn5150_info *info)
> return;
> }
>
> - port_status = ((reg_data &
> - PTN5150_REG_CC_PORT_ATTACHMENT_MASK) >>
> - PTN5150_REG_CC_PORT_ATTACHMENT_SHIFT);
> + port_status = FIELD_GET(PTN5150_REG_CC_PORT_ATTACHMENT, reg_data);
>
> switch (port_status) {
> case PTN5150_DFP_ATTACHED:
> @@ -107,8 +90,7 @@ static void ptn5150_check_state(struct ptn5150_info *info)
> break;
> case PTN5150_UFP_ATTACHED:
> extcon_set_state_sync(info->edev, EXTCON_USB, false);
> - vbus = ((reg_data & PTN5150_REG_CC_VBUS_DETECTION_MASK) >>
> - PTN5150_REG_CC_VBUS_DETECTION_SHIFT);
> + vbus = FIELD_GET(PTN5150_REG_CC_VBUS_DETECTION, reg_data);
> if (vbus)
> gpiod_set_value_cansleep(info->vbus_gpiod, 0);
> else
> @@ -191,11 +173,8 @@ static int ptn5150_init_dev_type(struct ptn5150_info *info)
> return -EINVAL;
> }
>
> - vendor_id = ((reg_data & PTN5150_REG_DEVICE_ID_VENDOR_MASK) >>
> - PTN5150_REG_DEVICE_ID_VENDOR_SHIFT);
> - version_id = ((reg_data & PTN5150_REG_DEVICE_ID_VERSION_MASK) >>
> - PTN5150_REG_DEVICE_ID_VERSION_SHIFT);
> -
> + vendor_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VENDOR, reg_data);
> + version_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VERSION, reg_data);
> dev_dbg(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n",
> version_id, vendor_id);
>
>
Applied it. Thanks.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Hi,
On 27/8/2020 12:51 pm, Chanwoo Choi wrote:
> Hi,
>
> You better to change the 'state' word to 'capability'.
> Actually, this patch doesn't change the value of property.
> It set the capability value of property.
>
> "Set the VBUS and POLARITY property capability"
Thank you for the review comments, sure will update.
>
> On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote:
>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>
>> Set the VBUS and POLARITY property state.
>
> ditto. Need to change the work from 'state' and 'capability'.
Noted.
>
>>
>> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
>> ---
>> drivers/extcon/extcon-ptn5150.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
>> index 8b930050a3f1..b5217a61615c 100644
>> --- a/drivers/extcon/extcon-ptn5150.c
>> +++ b/drivers/extcon/extcon-ptn5150.c
>> @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c)
>> return ret;
>> }
>>
>> + extcon_set_property_capability(info->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS);
>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS);
>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_TYPEC_POLARITY);
>
> Need to add blank line.
Noted.
>
> I understood that you set the property capability
> because of get_flipped() function of your patch[1].
>
> But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY
> when changing the state of EXTCON_USB_HOST. The polarity property value is always
> zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns
> always the same *flipped value.
>
> EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0
> EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0
by default EXTCON_PROP_USB_TYPEC_POLARITY is 1
>
> If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior,
> you don't need to get the property value from extcon consumer driver
> like drivers/phy/phy-lgm-usb.c.
>
> Actually, I don't understand why you don't handle the value
> of EXTCON_PROP_USB_TYPEC_POLARITY.
>
> Or, are there any case of what drivers/phy/phy-lgm-usb.c
> uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property
> in the future?
Yes, you're right, user connect the different USB cable then we check
polarity, accordingly driver proceeds, thanks!
>
> So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability
> for the extensibility in order to use other extcon device on later?
yes, that might be the case as well.
>
>
> [1] https://www.spinics.net/lists/devicetree/msg371828.html
> +static int get_flipped(struct tca_apb *ta, bool *flipped)
> +{
> + union extcon_property_value property;
> + int ret;
> +
> + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_TYPEC_POLARITY, &property);
> + if (ret) {
> + dev_err(ta->phy.dev, "no polarity property from extcon\n");
> + return ret;
> + }
> +
> + *flipped = property.intval;
> +
> + return ret;
> +}
Thank you for the gone through my usb-phy patch as well
Regards
Vadivel
>
>
>> /* Initialize PTN5150 device and print vendor id and version id */
>> ret = ptn5150_init_dev_type(info);
>> if (ret)
>>
>
>
Hi,
On 27/8/2020 1:26 pm, Chanwoo Choi wrote:
>> -
>> + vendor_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VENDOR, reg_data);
>> + version_id = FIELD_GET(PTN5150_REG_DEVICE_ID_VERSION, reg_data);
>> dev_dbg(info->dev, "Device type: version: 0x%x, vendor: 0x%x\n",
>> version_id, vendor_id);
>>
>>
> Applied it. Thanks.
Thank you!
Regards
Vadivel
Hi,
On 8/27/20 2:17 PM, Ramuthevar, Vadivel MuruganX wrote:
> Hi,
>
> On 27/8/2020 12:51 pm, Chanwoo Choi wrote:
>> Hi,
>>
>> You better to change the 'state' word to 'capability'.
>> Actually, this patch doesn't change the value of property.
>> It set the capability value of property.
>>
>> "Set the VBUS and POLARITY property capability"
> Thank you for the review comments, sure will update.
>>
>> On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote:
>>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>>
>>> Set the VBUS and POLARITY property state.
>>
>> ditto. Need to change the work from 'state' and 'capability'.
> Noted.
>>
>>>
>>> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
>>> ---
>>> drivers/extcon/extcon-ptn5150.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
>>> index 8b930050a3f1..b5217a61615c 100644
>>> --- a/drivers/extcon/extcon-ptn5150.c
>>> +++ b/drivers/extcon/extcon-ptn5150.c
>>> @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c)
>>> return ret;
>>> }
>>> + extcon_set_property_capability(info->edev, EXTCON_USB,
>>> + EXTCON_PROP_USB_VBUS);
>>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>>> + EXTCON_PROP_USB_VBUS);
>>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>>> + EXTCON_PROP_USB_TYPEC_POLARITY);
>>
>> Need to add blank line.
> Noted.
>>
>> I understood that you set the property capability
>> because of get_flipped() function of your patch[1].
>>
>> But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY
>> when changing the state of EXTCON_USB_HOST. The polarity property value is always
>> zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns
>> always the same *flipped value.
>>
>> EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0
>> EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0
> by default EXTCON_PROP_USB_TYPEC_POLARITY is 1
If you don't touch the value of EXTCON_PROP_USB_TYPEC_POLARITY property,
EXTCON_PROP_USB_TYPEC_POLARITY has default value (0).
>>
>> If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior,
>> you don't need to get the property value from extcon consumer driver
>> like drivers/phy/phy-lgm-usb.c.
>>
>> Actually, I don't understand why you don't handle the value
>> of EXTCON_PROP_USB_TYPEC_POLARITY.
>>
>> Or, are there any case of what drivers/phy/phy-lgm-usb.c
>> uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property
>> in the future?
> Yes, you're right, user connect the different USB cable then we check polarity, accordingly driver proceeds, thanks!
OK.
>>
>> So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability
>> for the extensibility in order to use other extcon device on later?
> yes, that might be the case as well.
OK.
>>
>>
>> [1] https://protect2.fireeye.com/v1/url?k=1fb29698-422c0d72-1fb31dd7-0cc47a6cba04-3009aa7184024984&q=1&e=566e4565-e7db-4a90-b036-fc28dbdb742f&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdevicetree%2Fmsg371828.html
>> +static int get_flipped(struct tca_apb *ta, bool *flipped)
>> +{
>> + union extcon_property_value property;
>> + int ret;
>> +
>> + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_TYPEC_POLARITY, &property);
>> + if (ret) {
>> + dev_err(ta->phy.dev, "no polarity property from extcon\n");
>> + return ret;
>> + }
>> +
>> + *flipped = property.intval;
>> +
>> + return ret;
>> +}
> Thank you for the gone through my usb-phy patch as well
>
> Regards
> Vadivel
>>
>>
>>> /* Initialize PTN5150 device and print vendor id and version id */
>>> ret = ptn5150_init_dev_type(info);
>>> if (ret)
>>>
>>
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Hi,
On 27/8/2020 1:35 pm, Chanwoo Choi wrote:
> Hi,
>
> On 8/27/20 2:17 PM, Ramuthevar, Vadivel MuruganX wrote:
>> Hi,
>>
>> On 27/8/2020 12:51 pm, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> You better to change the 'state' word to 'capability'.
>>> Actually, this patch doesn't change the value of property.
>>> It set the capability value of property.
>>>
>>> "Set the VBUS and POLARITY property capability"
>> Thank you for the review comments, sure will update.
>>>
>>> On 8/27/20 12:56 PM, Ramuthevar,Vadivel MuruganX wrote:
>>>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>>>
>>>> Set the VBUS and POLARITY property state.
>>>
>>> ditto. Need to change the work from 'state' and 'capability'.
>> Noted.
>>>
>>>>
>>>> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
>>>> ---
>>>> drivers/extcon/extcon-ptn5150.c | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
>>>> index 8b930050a3f1..b5217a61615c 100644
>>>> --- a/drivers/extcon/extcon-ptn5150.c
>>>> +++ b/drivers/extcon/extcon-ptn5150.c
>>>> @@ -279,6 +279,12 @@ static int ptn5150_i2c_probe(struct i2c_client *i2c)
>>>> return ret;
>>>> }
>>>> + extcon_set_property_capability(info->edev, EXTCON_USB,
>>>> + EXTCON_PROP_USB_VBUS);
>>>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>>>> + EXTCON_PROP_USB_VBUS);
>>>> + extcon_set_property_capability(info->edev, EXTCON_USB_HOST,
>>>> + EXTCON_PROP_USB_TYPEC_POLARITY);
>>>
>>> Need to add blank line.
>> Noted.
>>>
>>> I understood that you set the property capability
>>> because of get_flipped() function of your patch[1].
>>>
>>> But, I think that you need to change the value of EXTCON_PROP_USB_TYPEC_POLARITY
>>> when changing the state of EXTCON_USB_HOST. The polarity property value is always
>>> zero regardless of EXTCON_USB_HOST state as following: The get_flipped()[1] returns
>>> always the same *flipped value.
>>>
>>> EXTCON_USB_HOST is 1, EXTCON_PROP_USB_TYPEC_POLARITY is 0
>>> EXTCON_USB_HOST is 0, EXTCON_PROP_USB_TYPEC_POLARITY is 0
>> by default EXTCON_PROP_USB_TYPEC_POLARITY is 1
>
> If you don't touch the value of EXTCON_PROP_USB_TYPEC_POLARITY property,
> EXTCON_PROP_USB_TYPEC_POLARITY has default value (0).
Ok, thanks!
will update the patch, send it.
Regards
Vadivel
>
>>>
>>> If EXTCON_PROP_USB_TYPEC_POLARITY value is not related to any behavior,
>>> you don't need to get the property value from extcon consumer driver
>>> like drivers/phy/phy-lgm-usb.c.
>>>
>>> Actually, I don't understand why you don't handle the value
>>> of EXTCON_PROP_USB_TYPEC_POLARITY.
>>>
>>> Or, are there any case of what drivers/phy/phy-lgm-usb.c
>>> uses the different extcon device with EXTCON_PROP_USB_TYPEC_POLARITY property
>>> in the future?
>> Yes, you're right, user connect the different USB cable then we check polarity, accordingly driver proceeds, thanks!
>
> OK.
>
>>>
>>> So, do you set the EXTCON_PROP_USB_TYPEC_POLARITY capability
>>> for the extensibility in order to use other extcon device on later?
>> yes, that might be the case as well.
>
> OK.
>
>>>
>>>
>>> [1] https://protect2.fireeye.com/v1/url?k=1fb29698-422c0d72-1fb31dd7-0cc47a6cba04-3009aa7184024984&q=1&e=566e4565-e7db-4a90-b036-fc28dbdb742f&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdevicetree%2Fmsg371828.html
>>> +static int get_flipped(struct tca_apb *ta, bool *flipped)
>>> +{
>>> + union extcon_property_value property;
>>> + int ret;
>>> +
>>> + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST,
>>> + EXTCON_PROP_USB_TYPEC_POLARITY, &property);
>>> + if (ret) {
>>> + dev_err(ta->phy.dev, "no polarity property from extcon\n");
>>> + return ret;
>>> + }
>>> +
>>> + *flipped = property.intval;
>>> +
>>> + return ret;
>>> +}
>> Thank you for the gone through my usb-phy patch as well
>>
>> Regards
>> Vadivel
>>>
>>>
>>>> /* Initialize PTN5150 device and print vendor id and version id */
>>>> ret = ptn5150_init_dev_type(info);
>>>> if (ret)
>>>>
>>>
>>>
>>
>>
>
>