RE: [EXT] Re: [PATCH] usb: typec: tcpm: fix create duplicate source/sink-capabilities file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Guenter,

> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Friday, December 16, 2022 1:13 PM
> To: Xu Yang <xu.yang_2@xxxxxxx>; heikki.krogerus@xxxxxxxxxxxxxxx
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Jun Li <jun.li@xxxxxxx>
> 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@xxxxxxxxxxxxxxx>
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > ---
> >   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





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux