Sometimes supports_usb_power_delivery reports incorrect value.

Sometimes supports_usb_power_delivery reports incorrect value.

[Badhri Jagan Sridharan]

Hi Heikki,

While testing with different type-c phones available in the market,
With some phones, I noticed that supports_usb_power_delivery
reports "no" eventhough an explicit pd contract has been
established. After spending sometime debugging, I noticed that
the root cause of this is that the partner device(acting as source)
takes too long to send the SRC_CAP message. This makes the
underlying TCPM code to report usb_pd set to 0 while initially
calling typec_register_partner. However,since  there is no
provision in the type-c sysfs interface to update
supports_usb_power_delivery once the contract is established,
supports_usb_power_delivery is left to report "no" even if the partner
source device is at present performing Type-C PD.
Is it OK to add a api to enable updating supports_usb_power_delivery
node in the typec sysfs code after typec_register_partner has been
called ? Or do you have other suggestions ? Please advice.

Thanks & Regards,
Badhri.

Re: Sometimes supports_usb_power_delivery reports incorrect value.

[Heikki Krogerus]

Hi,

On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
> Hi Heikki,
> 
> While testing with different type-c phones available in the market,
> With some phones, I noticed that supports_usb_power_delivery
> reports "no" eventhough an explicit pd contract has been
> established. After spending sometime debugging, I noticed that
> the root cause of this is that the partner device(acting as source)
> takes too long to send the SRC_CAP message. This makes the
> underlying TCPM code to report usb_pd set to 0 while initially
> calling typec_register_partner. However,since  there is no
> provision in the type-c sysfs interface to update
> supports_usb_power_delivery once the contract is established,
> supports_usb_power_delivery is left to report "no" even if the partner
> source device is at present performing Type-C PD.
> Is it OK to add a api to enable updating supports_usb_power_delivery
> node in the typec sysfs code after typec_register_partner has been
> called ? Or do you have other suggestions ? Please advice.

supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
is called with value TYPEC_PWR_MODE_PD, and it should be called, also
in tcpm.c, always when USB PD contract has been established. I did not
check the latest tcpm.c code, but I assume it does that. If it
doesn't, it needs to be fixed of course.

Are you sure you really have the contract established?


Thanks,

-- 
heikki

Re: Sometimes supports_usb_power_delivery reports incorrect value.

[Badhri Jagan Sridharan]

On Tue, Aug 15, 2017 at 6:36 AM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> Hi,
>
> On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
>> Hi Heikki,
>>
>> While testing with different type-c phones available in the market,
>> With some phones, I noticed that supports_usb_power_delivery
>> reports "no" eventhough an explicit pd contract has been
>> established. After spending sometime debugging, I noticed that
>> the root cause of this is that the partner device(acting as source)
>> takes too long to send the SRC_CAP message. This makes the
>> underlying TCPM code to report usb_pd set to 0 while initially
>> calling typec_register_partner. However,since  there is no
>> provision in the type-c sysfs interface to update
>> supports_usb_power_delivery once the contract is established,
>> supports_usb_power_delivery is left to report "no" even if the partner
>> source device is at present performing Type-C PD.
>> Is it OK to add a api to enable updating supports_usb_power_delivery
>> node in the typec sysfs code after typec_register_partner has been
>> called ? Or do you have other suggestions ? Please advice.
>
> supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
> is called with value TYPEC_PWR_MODE_PD, and it should be called, also

Oops my bad !! I somehow did not notice the presence of your following
commit:

usb: typec: update partner power delivery support with opmode

which has not been picked-up in our codebase yet.

> in tcpm.c, always when USB PD contract has been established. I did not
> check the latest tcpm.c code, but I assume it does that. If it

TCPM does do this.

> doesn't, it needs to be fixed of course.
>
> Are you sure you really have the contract established?

Yes I did verify using the pd-analyzer. I will your change and see how it goes.
Thanks !

>
>
> Thanks,
>
> --
> heikki

Re: Sometimes supports_usb_power_delivery reports incorrect value.

[Badhri Jagan Sridharan]

Submitted couple of patches for the missing pieces in TCPM.
Those patches along with "usb: typec: update partner power delivery
support with opmode"
seems to address the issue of reporting the right value for
supports_usb_power_delivery.

Thanks,
Badhri

On Tue, Aug 15, 2017 at 7:13 AM, Badhri Jagan Sridharan
<badhri@google.com> wrote:
> On Tue, Aug 15, 2017 at 6:36 AM, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
>> Hi,
>>
>> On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
>>> Hi Heikki,
>>>
>>> While testing with different type-c phones available in the market,
>>> With some phones, I noticed that supports_usb_power_delivery
>>> reports "no" eventhough an explicit pd contract has been
>>> established. After spending sometime debugging, I noticed that
>>> the root cause of this is that the partner device(acting as source)
>>> takes too long to send the SRC_CAP message. This makes the
>>> underlying TCPM code to report usb_pd set to 0 while initially
>>> calling typec_register_partner. However,since  there is no
>>> provision in the type-c sysfs interface to update
>>> supports_usb_power_delivery once the contract is established,
>>> supports_usb_power_delivery is left to report "no" even if the partner
>>> source device is at present performing Type-C PD.
>>> Is it OK to add a api to enable updating supports_usb_power_delivery
>>> node in the typec sysfs code after typec_register_partner has been
>>> called ? Or do you have other suggestions ? Please advice.
>>
>> supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
>> is called with value TYPEC_PWR_MODE_PD, and it should be called, also
>
> Oops my bad !! I somehow did not notice the presence of your following
> commit:
>
> usb: typec: update partner power delivery support with opmode
>
> which has not been picked-up in our codebase yet.
>
>> in tcpm.c, always when USB PD contract has been established. I did not
>> check the latest tcpm.c code, but I assume it does that. If it
>
> TCPM does do this.
>
>> doesn't, it needs to be fixed of course.
>>
>> Are you sure you really have the contract established?
>
> Yes I did verify using the pd-analyzer. I will your change and see how it goes.
> Thanks !
>
>>
>>
>> Thanks,
>>
>> --
>> heikki

Re: Sometimes supports_usb_power_delivery reports incorrect value.

[Heikki Krogerus]

On Tue, Aug 15, 2017 at 04:26:07PM -0700, Badhri Jagan Sridharan wrote:
> Submitted couple of patches for the missing pieces in TCPM.
> Those patches along with "usb: typec: update partner power delivery
> support with opmode"
> seems to address the issue of reporting the right value for
> supports_usb_power_delivery.

Cool! Thanks Badhri!

-- 
heikki