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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b44d90d2-e91d-afd4-22c0-f64400ba9e11@samsung.com>
Date:   Thu, 27 Aug 2020 13:51:03 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     "Ramuthevar,Vadivel MuruganX" 
        <vadivel.muruganx.ramuthevar@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Cc:     vijaikumar.kanagarajan@...il.com, krzk@...nel.org,
        myungjoo.ham@...sung.com, heikki.krogerus@...ux.intel.com,
        cheol.yong.kim@...el.com, qi-ming.wu@...el.com, yin1.li@...el.com
Subject: Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY
 property state

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 <vadivel.muruganx.ramuthevar@...ux.intel.com>
> 
> Set the VBUS and POLARITY property state.

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

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