diff mbox series

usb: typec: tcpm: Fix altmode re-registration causes sysfs create fail

Message ID 1671096096-20307-1-git-send-email-u0084500@gmail.com
State New
Headers show
Series usb: typec: tcpm: Fix altmode re-registration causes sysfs create fail | expand

Commit Message

cy_huang Dec. 15, 2022, 9:21 a.m. UTC
From: ChiYuan Huang <cy_huang@richtek.com>

There's the altmode re-registeration issue after data role
swap (DR_SWAP).

Comparing to USBPD 2.0, in USBPD 3.0, it loose the limit that only DFP
can initiate the VDM command to get partner identity information.

For a USBPD 3.0 UFP device, it may already get the identity information
from its port partner before DR_SWAP. If DR_SWAP send or receive at the
mean time, 'send_discover' flag will be raised again. It causes discover
identify action restart while entering ready state. And after all
discover actions are done, the 'tcpm_register_altmodes' will be called.
If old altmode is not unregistered, this sysfs create fail can be found.

In 'DR_SWAP_CHANGE_DR' state case, only DFP will unregister altmodes.
For UFP, the original altmodes keep registered.

This patch fix the logic that after DR_SWAP, 'tcpm_unregister_altmodes'
must be called whatever the current data role is.

Fixes: ae8a2ca8a221 ("usb: typec: Group all TCPCI/TCPM code together)
Reported-by: TommyYl Chen <tommyyl.chen@mediatek.com>
Cc: stable@vger.kernel.org
Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
Hi,

Below's the issue log for the reference.

*TCPM
[    3.856679] AMS DISCOVER_MODES start
[    3.856687] PD TX, header: 0x188f
[    3.858827] PD TX complete, status: 0
[    3.865330] PD RX, header: 0x2daf [1]
[    3.865340] Rx VDM cmd 0xff01a043 type 1 cmd 3 len 2
[    3.865348] AMS DISCOVER_MODES finished
[    3.865352]  Alternate mode 0: SVID 0xff01, VDO 1: 0x001c0045
[    3.865362] AMS DISCOVER_MODES start
[    3.865367] PD TX, header: 0x1a8f
[    3.867802] PD TX complete, status: 0
[    3.875208] PD RX, header: 0x2faf [1]
[    3.875216] Rx VDM cmd 0x413ca043 type 1 cmd 3 len 2
[    3.875222] AMS DISCOVER_MODES finished
[    3.875225]  Alternate mode 1: SVID 0x413c, VDO 1: 0x00000001
[    3.938243] AMS GET_SINK_CAPABILITIES start
[    3.938255] state change SNK_READY -> AMS_START [rev3 GET_SINK_CAPABILITIES]
[    3.938266] state change AMS_START -> GET_SINK_CAP [rev3 GET_SINK_CAPABILITIES]
[    3.938274] PD TX, header: 0xe88
[    3.940268] PD TX complete, status: 0
[    3.940310] pending state change GET_SINK_CAP -> GET_SINK_CAP_TIMEOUT @ 60 ms
[rev3 GET_SINK_CAPABILITIES]
[    3.946291] PD RX, header: 0x13a4 [1]
[    3.946295] Port partner FRS capable partner_frs_current:0 port_frs_current:0 enable:n
[    3.946298] state change GET_SINK_CAP -> SNK_READY [rev3 GET_SINK_CAPABILITIES]
[    3.946304] AMS GET_SINK_CAPABILITIES finished
[    4.239342] CC1: 5 -> 4, CC2: 0 -> 0 [state SNK_READY, polarity 0, connected]
[    4.256594] PD RX, header: 0x5a9 [1]
[    4.256603] state change SNK_READY -> DR_SWAP_ACCEPT [rev3 DATA_ROLE_SWAP]
[    4.256609] PD TX, header: 0x83
[    4.258528] PD TX complete, status: 0
[    4.258584] state change DR_SWAP_ACCEPT -> DR_SWAP_CHANGE_DR [rev3 DATA_ROLE_SWAP]
[    4.258591] Requesting mux state 1, usb-role 1, orientation 1
[    4.259588] AMS DATA_ROLE_SWAP finished
[    4.259592] state change DR_SWAP_CHANGE_DR -> SNK_READY [rev3 NONE_AMS]
[    4.259605] AMS DISCOVER_IDENTITY start
[    4.259609] Sink TX No Go
[    4.260874] CC1: 4 -> 5, CC2: 0 -> 0 [state SNK_READY, polarity 0, connected]
[    4.359636] AMS DISCOVER_IDENTITY start
[    4.359642] PD TX, header: 0x12af
[    4.361884] PD TX complete, status: 0
[    4.369433] PD RX, header: 0x578f [1]
[    4.369439] Rx VDM cmd 0xff00a041 type 1 cmd 1 len 5
[    4.369448] AMS DISCOVER_IDENTITY finished
[    4.369515] Identity: 413c:c013.0712
[    4.369521] AMS DISCOVER_SVIDS start
[    4.369524] PD TX, header: 0x14af
[    4.371696] PD TX complete, status: 0
[    4.378564] PD RX, header: 0x398f [1]
[    4.378573] Rx VDM cmd 0xff00a042 type 1 cmd 2 len 3
[    4.378579] AMS DISCOVER_SVIDS finished
[    4.378582] SVID 1: 0xff01
[    4.378584] SVID 2: 0x413c
[    4.378594] AMS DISCOVER_MODES start
[    4.378597] PD TX, header: 0x16af
[    4.380696] PD TX complete, status: 0
[    4.387008] PD RX, header: 0x2b8f [1]
[    4.387014] Rx VDM cmd 0xff01a043 type 1 cmd 3 len 2
[    4.387021] AMS DISCOVER_MODES finished
[    4.387023]  Alternate mode 0: SVID 0xff01, VDO 1: 0x001c0045
[    4.387029] AMS DISCOVER_MODES start
[    4.387031] PD TX, header: 0x18af
[    4.389134] PD TX complete, status: 0
[    4.395528] PD RX, header: 0x2d8f [1]
[    4.395538] Rx VDM cmd 0x413ca043 type 1 cmd 3 len 2
[    4.395546] AMS DISCOVER_MODES finished
[    4.395548]  Alternate mode 1: SVID 0x413c, VDO 1: 0x00000001

*Kernel TRACE
sysfs: cannot create duplicate filename
'/devices/platform/soc/11d01000.i2c/i2c-0/0-0034/mt6360-tcpc.6.auto/typec/port0/port0.0/partner'
CPU: 2 PID: 299 Comm: mt6360-tcpc.6.a Tainted: GO      5.15.37-mtk+g880abc5122e7 #1
Hardware name: MediaTek MT8195 demo board (DT)
Call trace:
 dump_backtrace+0x0/0x1ac
 show_stack+0x24/0x30
 dump_stack_lvl+0x68/0x84
 dump_stack+0x1c/0x38
 sysfs_warn_dup+0x70/0x90
 typec_probe+0xa4/0x134 [typec]
 really_probe.part.0+0xa4/0x310
 __device_attach_driver+0x100/0x16c
 bus_for_each_drv+0x84/0xe0
 __device_attach+0xe0/0x1ac
 device_add+0x39c/0x8b0
 device_register+0x2c/0x40
 typec_register_altmode+0x1f4/0x360 [typec]
 typec_partner_register_altmode+0x1c/0x30 [typec]
 tcpm_pd_rx_handler+0x19d4/0x1c0c [tcpm]
 kthread_worker_fn+0xb8/0x290
 kthread+0x15c/0x170
 ret_from_fork+0x10/0x20
[    4.395962] typec_displayport port0-partner.2: failed to create symlinks
[    4.395967] typec_displayport: probe of port0-partner.2 failed with error -17

It seems it's a common issue if typec port supports the modal operation.

---
 drivers/usb/typec/tcpm/tcpm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

ChiYuan Huang Jan. 9, 2023, 1:41 a.m. UTC | #1
On Thu, Dec 15, 2022 at 12:43:35PM +0100, Greg KH wrote:
> On Thu, Dec 15, 2022 at 05:53:44PM +0800, ChiYuan Huang wrote:
> > Greg KH <gregkh@linuxfoundation.org> 於 2022年12月15日 週四 下午5:44寫道:
> > >
> > > On Thu, Dec 15, 2022 at 05:21:36PM +0800, cy_huang wrote:
> > > > From: ChiYuan Huang <cy_huang@richtek.com>
> > >
> > > Why not send directly from this address so we can validate that this is
> > > the correct email address of yours?
> > >
> > It's  the company mailbox policy. To send the external mail, there's
> > the security text block at the bottom.
> > Except this, some mail address are also blocked. To avoid this, I use
> > my personal mail to send the patch
> > and leave the SoB for the Richtek mailbox.
> > It's lazy to fight for this.
>
> Please fix it, otherwise your company's email address will be spoofed
> and people can claim to be sending changes from their domain.
>
> Please fix that up, abusing random gmail addresses like this is not ok,
> sorry.

Thanks for your comment.
After the work with MIS for several weeks, we finnaly got one way to do it.

But I'm not sure all mail account can receive the mail.
If anyone cannot receive the mail, please inform me.

ChiYuan Huang.

>
> greg k-h
************* Email Confidentiality Notice ********************

The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
Greg Kroah-Hartman Jan. 9, 2023, 6:38 a.m. UTC | #2
On Mon, Jan 09, 2023 at 09:41:23AM +0800, ChiYuan Huang wrote:
> ************* Email Confidentiality Notice ********************
> 
> The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!

Now deleted.

For obvious reasons, this wording is not compatible with kernel
development :(
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 904c7b4..59b366b 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4594,14 +4594,13 @@  static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
 	case DR_SWAP_CHANGE_DR:
-		if (port->data_role == TYPEC_HOST) {
-			tcpm_unregister_altmodes(port);
+		tcpm_unregister_altmodes(port);
+		if (port->data_role == TYPEC_HOST)
 			tcpm_set_roles(port, true, port->pwr_role,
 				       TYPEC_DEVICE);
-		} else {
+		else
 			tcpm_set_roles(port, true, port->pwr_role,
 				       TYPEC_HOST);
-		}
 		tcpm_ams_finish(port);
 		tcpm_set_state(port, ready_state(port), 0);
 		break;