Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 7IbRRDJwcffUzf4JGbPho2bB/kv/zJn/L0WQ5IifCmlODhEl0wrT1WXvGzNjokv4bOHuQt9kbl
 NYqUHijTj4rw==
IronPort-SDR: LfemeSRC1ysU9CyeHxx5JB+z9uvnt1Pj+foQrsc/kyoyo8kohTGfJZFOb6OcXBttl+Jnj956KY
 agFTvs3y5r+A==
Reply-To: vadivel.muruganx.ramuthevar@linux.intel.com
Subject: Re: [PATCH v2 2/2] extcon: ptn5150: Set the VBUS and POLARITY
 property state
To:     Chanwoo Choi <cw00.choi@samsung.com>, linux-kernel@vger.kernel.org
Cc:     vijaikumar.kanagarajan@gmail.com, krzk@kernel.org,
        myungjoo.ham@samsung.com, heikki.krogerus@linux.intel.com,
        cheol.yong.kim@intel.com, qi-ming.wu@intel.com, yin1.li@intel.com
References: <20200827035633.37348-1-vadivel.muruganx.ramuthevar@linux.intel.com>
 <CGME20200827035651epcas1p33a045925293860a361a3be0cf21a2e2a@epcas1p3.samsung.com>
 <20200827035633.37348-3-vadivel.muruganx.ramuthevar@linux.intel.com>
 <b44d90d2-e91d-afd4-22c0-f64400ba9e11@samsung.com>
From:   "Ramuthevar, Vadivel MuruganX" 
        <vadivel.muruganx.ramuthevar@linux.intel.com>
Message-ID: <1e9360ee-22f6-2135-26c7-a5b25fe7776d@linux.intel.com>
Date:   Thu, 27 Aug 2020 13:17:16 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <b44d90d2-e91d-afd4-22c0-f64400ba9e11@samsung.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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)
>>
> 
>