Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY property state

From: Ramuthevar, Vadivel MuruganX
Date: Thu Aug 27 2020 - 02:11:36 EST


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 <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx>

Set the VBUS and POLARITY property state.

ditto. Need to change the work from 'state' and 'capability'.
Noted.


Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx>
---
  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)