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

["Ramuthevar, Vadivel MuruganX" <vadivel.muruganx.ramuthevar@linux.intel.com>]

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@linux.intel.com>
>>
>> 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@linux.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.
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)
>>
> 
>