| Message ID | 20230215054951.238394-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a826492fc9dfe32afd70fff93955ae8174bbf14b |
| Headers | show |
| Series | [v4] usb: typec: tcpm: fix create duplicate source-capabilities file | expand |
On Wed, Feb 15, 2023 at 01:49:51PM +0800, Xu Yang wrote: > The kernel will dump in the below cases: > sysfs: cannot create duplicate filename > '/devices/virtual/usb_power_delivery/pd1/source-capabilities' > > 1. 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. > 2. Power swap twice on a device that is initailly sink role. > > This will unregister existing capabilities when above cases occurs. > > 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> > > --- > changelogs: > v2: unregister existing capabilities on specific cases > v3: add changelog and modify commit message > v4: reset port->partner_source_caps to NULL > --- Given the churn on this, and the seemingly lack-of-testing, I'll wait until after -rc1 is out before applying this, is that ok? thanks, greg k-h
Hi Greg, > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Wednesday, February 15, 2023 4:10 PM > To: Xu Yang <xu.yang_2@nxp.com> > Cc: linux@roeck-us.net; heikki.krogerus@linux.intel.com; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > Jun Li <jun.li@nxp.com> > Subject: [EXT] Re: [PATCH v4] usb: typec: tcpm: fix create duplicate source-capabilities file > > Caution: EXT Email > > On Wed, Feb 15, 2023 at 01:49:51PM +0800, Xu Yang wrote: > > The kernel will dump in the below cases: > > sysfs: cannot create duplicate filename > > '/devices/virtual/usb_power_delivery/pd1/source-capabilities' > > > > 1. 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. > > 2. Power swap twice on a device that is initailly sink role. > > > > This will unregister existing capabilities when above cases occurs. > > > > 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> > > > > --- > > changelogs: > > v2: unregister existing capabilities on specific cases > > v3: add changelog and modify commit message > > v4: reset port->partner_source_caps to NULL > > --- > > Given the churn on this, and the seemingly lack-of-testing, I'll wait > until after -rc1 is out before applying this, is that ok? Sure. This patch works fine in my previous testing. But it failed at a specific case today. I'll be carful in the future. Thanks, Xu Yang > > thanks, > > greg k-h
On 2/14/23 21:49, Xu Yang wrote: > The kernel will dump in the below cases: > sysfs: cannot create duplicate filename > '/devices/virtual/usb_power_delivery/pd1/source-capabilities' > > 1. 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. > 2. Power swap twice on a device that is initailly sink role. > > This will unregister existing capabilities when above cases occurs. > > 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> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > --- > changelogs: > v2: unregister existing capabilities on specific cases > v3: add changelog and modify commit message > v4: reset port->partner_source_caps to NULL > --- > drivers/usb/typec/tcpm/tcpm.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index a0d943d78580..7f39cb9b3429 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -4570,6 +4570,9 @@ static void run_state_machine(struct tcpm_port *port) > case SOFT_RESET: > port->message_id = 0; > port->rx_msgid = -1; > + /* remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > tcpm_pd_send_control(port, PD_CTRL_ACCEPT); > tcpm_ams_finish(port); > if (port->pwr_role == TYPEC_SOURCE) { > @@ -4589,6 +4592,9 @@ static void run_state_machine(struct tcpm_port *port) > case SOFT_RESET_SEND: > port->message_id = 0; > port->rx_msgid = -1; > + /* remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > if (tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET)) > tcpm_set_state_cond(port, hard_reset_state(port), 0); > else > @@ -4718,6 +4724,9 @@ static void run_state_machine(struct tcpm_port *port) > tcpm_set_state(port, SNK_STARTUP, 0); > break; > case PR_SWAP_SNK_SRC_SINK_OFF: > + /* will be source, remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > /* > * Prevent vbus discharge circuit from turning on during PR_SWAP > * as this is not a disconnect.
On Wed, Feb 15, 2023 at 01:49:51PM +0800, Xu Yang wrote: > The kernel will dump in the below cases: > sysfs: cannot create duplicate filename > '/devices/virtual/usb_power_delivery/pd1/source-capabilities' > > 1. 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. > 2. Power swap twice on a device that is initailly sink role. > > This will unregister existing capabilities when above cases occurs. > > 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> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > changelogs: > v2: unregister existing capabilities on specific cases > v3: add changelog and modify commit message > v4: reset port->partner_source_caps to NULL > --- > drivers/usb/typec/tcpm/tcpm.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index a0d943d78580..7f39cb9b3429 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -4570,6 +4570,9 @@ static void run_state_machine(struct tcpm_port *port) > case SOFT_RESET: > port->message_id = 0; > port->rx_msgid = -1; > + /* remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > tcpm_pd_send_control(port, PD_CTRL_ACCEPT); > tcpm_ams_finish(port); > if (port->pwr_role == TYPEC_SOURCE) { > @@ -4589,6 +4592,9 @@ static void run_state_machine(struct tcpm_port *port) > case SOFT_RESET_SEND: > port->message_id = 0; > port->rx_msgid = -1; > + /* remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > if (tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET)) > tcpm_set_state_cond(port, hard_reset_state(port), 0); > else > @@ -4718,6 +4724,9 @@ static void run_state_machine(struct tcpm_port *port) > tcpm_set_state(port, SNK_STARTUP, 0); > break; > case PR_SWAP_SNK_SRC_SINK_OFF: > + /* will be source, remove existing capabilities */ > + usb_power_delivery_unregister_capabilities(port->partner_source_caps); > + port->partner_source_caps = NULL; > /* > * Prevent vbus discharge circuit from turning on during PR_SWAP > * as this is not a disconnect. > -- > 2.34.1
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index a0d943d78580..7f39cb9b3429 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -4570,6 +4570,9 @@ static void run_state_machine(struct tcpm_port *port) case SOFT_RESET: port->message_id = 0; port->rx_msgid = -1; + /* remove existing capabilities */ + usb_power_delivery_unregister_capabilities(port->partner_source_caps); + port->partner_source_caps = NULL; tcpm_pd_send_control(port, PD_CTRL_ACCEPT); tcpm_ams_finish(port); if (port->pwr_role == TYPEC_SOURCE) { @@ -4589,6 +4592,9 @@ static void run_state_machine(struct tcpm_port *port) case SOFT_RESET_SEND: port->message_id = 0; port->rx_msgid = -1; + /* remove existing capabilities */ + usb_power_delivery_unregister_capabilities(port->partner_source_caps); + port->partner_source_caps = NULL; if (tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET)) tcpm_set_state_cond(port, hard_reset_state(port), 0); else @@ -4718,6 +4724,9 @@ static void run_state_machine(struct tcpm_port *port) tcpm_set_state(port, SNK_STARTUP, 0); break; case PR_SWAP_SNK_SRC_SINK_OFF: + /* will be source, remove existing capabilities */ + usb_power_delivery_unregister_capabilities(port->partner_source_caps); + port->partner_source_caps = NULL; /* * Prevent vbus discharge circuit from turning on during PR_SWAP * as this is not a disconnect.
The kernel will dump in the below cases: sysfs: cannot create duplicate filename '/devices/virtual/usb_power_delivery/pd1/source-capabilities' 1. 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. 2. Power swap twice on a device that is initailly sink role. This will unregister existing capabilities when above cases occurs. 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> --- changelogs: v2: unregister existing capabilities on specific cases v3: add changelog and modify commit message v4: reset port->partner_source_caps to NULL --- drivers/usb/typec/tcpm/tcpm.c | 9 +++++++++ 1 file changed, 9 insertions(+)