linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
@ 2022-12-16  3:31 Xu Yang
  2022-12-16  5:13 ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Xu Yang @ 2022-12-16  3:31 UTC (permalink / raw)
  To: linux, heikki.krogerus; +Cc: gregkh, linux-usb, linux-imx, jun.li, xu.yang_2

After soft reset has completed, an Explicit Contract negotiation occurs.
The sink device will receive source capabilitys again. This will cause
a duplicate source-capabilities file be created.

And the kernel will dump:
sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'

This will unregister existing capabilities before register new one.

Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
cc: <stable@vger.kernel.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 904c7b4ce2f0..02d8a704db95 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
 	if (IS_ERR(port->partner_pd))
 		return PTR_ERR(port->partner_pd);
 
+	/* remove existing capabilities since got new one */
+	usb_power_delivery_unregister_capabilities(port->partner_source_caps);
+
 	memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
 	caps.role = TYPEC_SOURCE;
 
@@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
 	if (IS_ERR(port->partner_pd))
 		return PTR_ERR(port->partner_pd);
 
+	/* remove existing capabilities since got new one */
+	usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
+
 	memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
 	caps.role = TYPEC_SINK;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
  2022-12-16  3:31 [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file Xu Yang
@ 2022-12-16  5:13 ` Guenter Roeck
  2022-12-16  6:40   ` [EXT] " Xu Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2022-12-16  5:13 UTC (permalink / raw)
  To: Xu Yang, heikki.krogerus; +Cc: gregkh, linux-usb, linux-imx, jun.li

On 12/15/22 19:31, Xu Yang wrote:
> After soft reset has completed, an Explicit Contract negotiation occurs.
> The sink device will receive source capabilitys again. This will cause
> a duplicate source-capabilities file be created.
> 
> And the kernel will dump:
> sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'
> 
> This will unregister existing capabilities before register new one.
> 
> Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>   drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 904c7b4ce2f0..02d8a704db95 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
>   	if (IS_ERR(port->partner_pd))
>   		return PTR_ERR(port->partner_pd);
>   
> +	/* remove existing capabilities since got new one */
> +	usb_power_delivery_unregister_capabilities(port->partner_source_caps);
> +
>   	memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
>   	caps.role = TYPEC_SOURCE;
>   
> @@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
>   	if (IS_ERR(port->partner_pd))
>   		return PTR_ERR(port->partner_pd);
>   
> +	/* remove existing capabilities since got new one */
> +	usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
> +
>   	memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
>   	caps.role = TYPEC_SINK;
>   

Shouldn't usb_power_delivery_unregister_capabilities() be called from
the SOFT_RESET state handler ?

Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
  2022-12-16  5:13 ` Guenter Roeck
@ 2022-12-16  6:40   ` Xu Yang
  2023-01-09  0:35     ` Xu Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Xu Yang @ 2022-12-16  6:40 UTC (permalink / raw)
  To: Guenter Roeck, heikki.krogerus; +Cc: gregkh, linux-usb, dl-linux-imx, Jun Li

Hi Guenter,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, December 16, 2022 1:13 PM
> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> Subject: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> 
> Caution: EXT Email
> 
> On 12/15/22 19:31, Xu Yang wrote:
> > After soft reset has completed, an Explicit Contract negotiation occurs.
> > The sink device will receive source capabilitys again. This will cause
> > a duplicate source-capabilities file be created.
> >
> > And the kernel will dump:
> > sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'
> >
> > This will unregister existing capabilities before register new one.
> >
> > Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
> > cc: <stable@vger.kernel.org>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >   drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 904c7b4ce2f0..02d8a704db95 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
> >       if (IS_ERR(port->partner_pd))
> >               return PTR_ERR(port->partner_pd);
> >
> > +     /* remove existing capabilities since got new one */
> > +     usb_power_delivery_unregister_capabilities(port->partner_source_caps);
> > +
> >       memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
> >       caps.role = TYPEC_SOURCE;
> >
> > @@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
> >       if (IS_ERR(port->partner_pd))
> >               return PTR_ERR(port->partner_pd);
> >
> > +     /* remove existing capabilities since got new one */
> > +     usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
> > +
> >       memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
> >       caps.role = TYPEC_SINK;
> >
> 
> Shouldn't usb_power_delivery_unregister_capabilities() be called from
> the SOFT_RESET state handler ?

Although this issue is triggered by soft reset event, there is also
other possibilities which may produce this behavior. Such as received
2rd source capability or use Get_Source_Cap message. It's a dynamic
process even after source/sink is ready. So I think it's better to handle
it in tcpm_register_source/sink_caps(). Not sure if this is reasonable.

Thanks,
Xu Yang

> 
> Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
  2022-12-16  6:40   ` [EXT] " Xu Yang
@ 2023-01-09  0:35     ` Xu Yang
  2023-01-10 15:01       ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Xu Yang @ 2023-01-09  0:35 UTC (permalink / raw)
  To: Guenter Roeck, heikki.krogerus; +Cc: gregkh, linux-usb, dl-linux-imx, Jun Li

Hi,

> -----Original Message-----
> From: Xu Yang
> Sent: Friday, December 16, 2022 2:41 PM
> To: Guenter Roeck <linux@roeck-us.net>; heikki.krogerus@linux.intel.com
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> Subject: RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> 
> Hi Guenter,
> 
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > Sent: Friday, December 16, 2022 1:13 PM
> > To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
> > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> > Subject: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> >
> > Caution: EXT Email
> >
> > On 12/15/22 19:31, Xu Yang wrote:
> > > After soft reset has completed, an Explicit Contract negotiation occurs.
> > > The sink device will receive source capabilitys again. This will cause
> > > a duplicate source-capabilities file be created.
> > >
> > > And the kernel will dump:
> > > sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'
> > >
> > > This will unregister existing capabilities before register new one.
> > >
> > > Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
> > > cc: <stable@vger.kernel.org>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > >   drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > > index 904c7b4ce2f0..02d8a704db95 100644
> > > --- a/drivers/usb/typec/tcpm/tcpm.c
> > > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > > @@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
> > >       if (IS_ERR(port->partner_pd))
> > >               return PTR_ERR(port->partner_pd);
> > >
> > > +     /* remove existing capabilities since got new one */
> > > +     usb_power_delivery_unregister_capabilities(port->partner_source_caps);
> > > +
> > >       memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
> > >       caps.role = TYPEC_SOURCE;
> > >
> > > @@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
> > >       if (IS_ERR(port->partner_pd))
> > >               return PTR_ERR(port->partner_pd);
> > >
> > > +     /* remove existing capabilities since got new one */
> > > +     usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
> > > +
> > >       memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
> > >       caps.role = TYPEC_SINK;
> > >
> >
> > Shouldn't usb_power_delivery_unregister_capabilities() be called from
> > the SOFT_RESET state handler ?
> 
> Although this issue is triggered by soft reset event, there is also
> other possibilities which may produce this behavior. Such as received
> 2rd source capability or use Get_Source_Cap message. It's a dynamic
> process even after source/sink is ready. So I think it's better to handle
> it in tcpm_register_source/sink_caps(). Not sure if this is reasonable.
> 
> Thanks,
> Xu Yang

Do you have any other comments or suggestions with this patch?

Thanks,
Xu Yang

> 
> >
> > Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
  2023-01-09  0:35     ` Xu Yang
@ 2023-01-10 15:01       ` Guenter Roeck
  2023-01-11 13:39         ` Xu Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2023-01-10 15:01 UTC (permalink / raw)
  To: Xu Yang, heikki.krogerus; +Cc: gregkh, linux-usb, dl-linux-imx, Jun Li

On 1/8/23 16:35, Xu Yang wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Xu Yang
>> Sent: Friday, December 16, 2022 2:41 PM
>> To: Guenter Roeck <linux@roeck-us.net>; heikki.krogerus@linux.intel.com
>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
>> Subject: RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
>>
>> Hi Guenter,
>>
>>> -----Original Message-----
>>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>>> Sent: Friday, December 16, 2022 1:13 PM
>>> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
>>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
>>> Subject: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
>>>
>>> Caution: EXT Email
>>>
>>> On 12/15/22 19:31, Xu Yang wrote:
>>>> After soft reset has completed, an Explicit Contract negotiation occurs.
>>>> The sink device will receive source capabilitys again. This will cause
>>>> a duplicate source-capabilities file be created.
>>>>
>>>> And the kernel will dump:
>>>> sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'
>>>>
>>>> This will unregister existing capabilities before register new one.
>>>>
>>>> Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
>>>> cc: <stable@vger.kernel.org>
>>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>>> ---
>>>>    drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>>>> index 904c7b4ce2f0..02d8a704db95 100644
>>>> --- a/drivers/usb/typec/tcpm/tcpm.c
>>>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>>>> @@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
>>>>        if (IS_ERR(port->partner_pd))
>>>>                return PTR_ERR(port->partner_pd);
>>>>
>>>> +     /* remove existing capabilities since got new one */
>>>> +     usb_power_delivery_unregister_capabilities(port->partner_source_caps);
>>>> +
>>>>        memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
>>>>        caps.role = TYPEC_SOURCE;
>>>>
>>>> @@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
>>>>        if (IS_ERR(port->partner_pd))
>>>>                return PTR_ERR(port->partner_pd);
>>>>
>>>> +     /* remove existing capabilities since got new one */
>>>> +     usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
>>>> +
>>>>        memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
>>>>        caps.role = TYPEC_SINK;
>>>>
>>>
>>> Shouldn't usb_power_delivery_unregister_capabilities() be called from
>>> the SOFT_RESET state handler ?
>>
>> Although this issue is triggered by soft reset event, there is also
>> other possibilities which may produce this behavior. Such as received
>> 2rd source capability or use Get_Source_Cap message. It's a dynamic
>> process even after source/sink is ready. So I think it's better to handle
>> it in tcpm_register_source/sink_caps(). Not sure if this is reasonable.
>>
>> Thanks,
>> Xu Yang
> 
> Do you have any other comments or suggestions with this patch?
> 

First of all, the current code is obviously wrong. If soft reset results in
pd capabilities to be invalid, that should be handled in soft reset,
just like it is handled in hard reset. Otherwise there would be stale pd
devices around which are no longer valid.

Second, if it can indeed happen that multiple source capabilities
messages are received, this should be handled as defined by the protocol.
Either the second set of messages is expected to be ignored, or it is
expected to replace existing capabilities. Either case, that situation
should be handled consciously: unregistering and re-registering
capabilities results in removal and re-creation of devices. Just doing
that unconditionally even if unnecessary (if capabilities are the same)
needs more discussion, and should not be hidden in another patch which
is supposed to address a different problem.

Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
  2023-01-10 15:01       ` Guenter Roeck
@ 2023-01-11 13:39         ` Xu Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Xu Yang @ 2023-01-11 13:39 UTC (permalink / raw)
  To: Guenter Roeck, heikki.krogerus; +Cc: gregkh, linux-usb, dl-linux-imx, Jun Li

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Tuesday, January 10, 2023 11:02 PM
> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> Subject: Re: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> 
> Caution: EXT Email
> 
> On 1/8/23 16:35, Xu Yang wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Xu Yang
> >> Sent: Friday, December 16, 2022 2:41 PM
> >> To: Guenter Roeck <linux@roeck-us.net>; heikki.krogerus@linux.intel.com
> >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> >> Subject: RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> >>
> >> Hi Guenter,
> >>
> >>> -----Original Message-----
> >>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> >>> Sent: Friday, December 16, 2022 1:13 PM
> >>> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
> >>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> >>> Subject: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file
> >>>
> >>> Caution: EXT Email
> >>>
> >>> On 12/15/22 19:31, Xu Yang wrote:
> >>>> After soft reset has completed, an Explicit Contract negotiation occurs.
> >>>> The sink device will receive source capabilitys again. This will cause
> >>>> a duplicate source-capabilities file be created.
> >>>>
> >>>> And the kernel will dump:
> >>>> sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities'
> >>>>
> >>>> This will unregister existing capabilities before register new one.
> >>>>
> >>>> Fixes: 8203d26905ee ("usb: typec: tcpm: Register USB Power Delivery Capabilities")
> >>>> cc: <stable@vger.kernel.org>
> >>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >>>> ---
> >>>>    drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
> >>>>    1 file changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> >>>> index 904c7b4ce2f0..02d8a704db95 100644
> >>>> --- a/drivers/usb/typec/tcpm/tcpm.c
> >>>> +++ b/drivers/usb/typec/tcpm/tcpm.c
> >>>> @@ -2371,6 +2371,9 @@ static int tcpm_register_source_caps(struct tcpm_port *port)
> >>>>        if (IS_ERR(port->partner_pd))
> >>>>                return PTR_ERR(port->partner_pd);
> >>>>
> >>>> +     /* remove existing capabilities since got new one */
> >>>> +     usb_power_delivery_unregister_capabilities(port->partner_source_caps);
> >>>> +
> >>>>        memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
> >>>>        caps.role = TYPEC_SOURCE;
> >>>>
> >>>> @@ -2394,6 +2397,9 @@ static int tcpm_register_sink_caps(struct tcpm_port *port)
> >>>>        if (IS_ERR(port->partner_pd))
> >>>>                return PTR_ERR(port->partner_pd);
> >>>>
> >>>> +     /* remove existing capabilities since got new one */
> >>>> +     usb_power_delivery_unregister_capabilities(port->partner_sink_caps);
> >>>> +
> >>>>        memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
> >>>>        caps.role = TYPEC_SINK;
> >>>>
> >>>
> >>> Shouldn't usb_power_delivery_unregister_capabilities() be called from
> >>> the SOFT_RESET state handler ?
> >>
> >> Although this issue is triggered by soft reset event, there is also
> >> other possibilities which may produce this behavior. Such as received
> >> 2rd source capability or use Get_Source_Cap message. It's a dynamic
> >> process even after source/sink is ready. So I think it's better to handle
> >> it in tcpm_register_source/sink_caps(). Not sure if this is reasonable.
> >>
> >> Thanks,
> >> Xu Yang
> >
> > Do you have any other comments or suggestions with this patch?
> >
> 
> First of all, the current code is obviously wrong. If soft reset results in
> pd capabilities to be invalid, that should be handled in soft reset,
> just like it is handled in hard reset. Otherwise there would be stale pd
> devices around which are no longer valid.
> 
> Second, if it can indeed happen that multiple source capabilities
> messages are received, this should be handled as defined by the protocol.
> Either the second set of messages is expected to be ignored, or it is
> expected to replace existing capabilities. Either case, that situation
> should be handled consciously: unregistering and re-registering
> capabilities results in removal and re-creation of devices. Just doing
> that unconditionally even if unnecessary (if capabilities are the same)
> needs more discussion, and should not be hidden in another patch which
> is supposed to address a different problem.

Thanks for your comments. According to the protocol , it's not possible
for the source to send multiple capabilities. Unless the source doesn't
follow the rule. That is indeed another problem. I agree with you that
it should be handled in soft reset handler if soft reset results in pd
capabilities to be invaild. I will prepare a v2 for this case.

Thanks,
Xu Yang

> 
> Guenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-11 13:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16  3:31 [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file Xu Yang
2022-12-16  5:13 ` Guenter Roeck
2022-12-16  6:40   ` [EXT] " Xu Yang
2023-01-09  0:35     ` Xu Yang
2023-01-10 15:01       ` Guenter Roeck
2023-01-11 13:39         ` Xu Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).