Hello Biju, Thanks for your response and sorry for the delay. > -----Original Message----- > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > Sent: Wednesday, November 27, 2024 9:48 AM > Subject: RE: [PATCH v2 3/4] usb: typec: hd3ss3220: support configuring port > type > > Hi Facklam, Olivér, > > > -----Original Message----- > > From: Facklam, Olivér <oliver.facklam@xxxxxxxxxxx> > > Sent: 27 November 2024 08:03 > > Subject: RE: [PATCH v2 3/4] usb: typec: hd3ss3220: support configuring > > port type > > > > Hi Heikki, > > > > > -----Original Message----- > > > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > > Sent: Monday, November 25, 2024 11:28 AM > > > > > > Hi Olivér, > > > > > > Sorry to keep you waiting. > > > > > > On Mon, Nov 18, 2024 at 02:00:41PM +0000, Facklam, Olivér wrote: > > > > Hello Heikki, > > > > > > > > Thanks for reviewing. > > > > > > > > > -----Original Message----- > > > > > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > > > > Sent: Monday, November 18, 2024 12:50 PM > > > > > > > > > > Hi Oliver, > > > > > > > > > > I'm sorry, I noticed a problem with this... > > > > > > > > > > On Thu, Nov 14, 2024 at 09:02:08AM +0100, Oliver Facklam wrote: > > > > > > The TI HD3SS3220 Type-C controller supports configuring the > > > > > > port type it will operate as through the MODE_SELECT field of > > > > > > the General Control Register. > > > > > > > > > > > > Configure the port type based on the fwnode property "power-role" > > > > > > during probe, and through the port_type_set typec_operation. > > > > > > > > > > > > The MODE_SELECT field can only be changed when the controller > > > > > > is in unattached state, so follow the sequence recommended by > > > > > > the datasheet > > > > > to: > > > > > > 1. disable termination on CC pins to disable the controller 2. > > > > > > change the mode 3. re-enable termination > > > > > > > > > > > > This will effectively cause a connected device to disconnect > > > > > > for the duration of the mode change. > > > > > > > > > > > Signed-off-by: Oliver Facklam <oliver.facklam@xxxxxxxxxxx> > > > > > > --- > > > > > > drivers/usb/typec/hd3ss3220.c | 66 > > > > > ++++++++++++++++++++++++++++++++++++++++++- > > > > > > 1 file changed, 65 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/usb/typec/hd3ss3220.c > > > > > b/drivers/usb/typec/hd3ss3220.c > > > > > > index > > > > > > > > > e581272bb47de95dee8363a5491f543354fcbbf8..e3e9b1597e3b09b82f0726a > > > > > 01f311fb60b4284da 100644 > > > > > > --- a/drivers/usb/typec/hd3ss3220.c > > > > > > +++ b/drivers/usb/typec/hd3ss3220.c > > > > [...] > > > > > > @@ -131,8 +183,16 @@ static int hd3ss3220_dr_set(struct > > > > > > typec_port > > > > > *port, enum typec_data_role role) > > > > > > return ret; > > > > > > } > > > > > > > > > > > > +static int hd3ss3220_port_type_set(struct typec_port *port, > > > > > > +enum > > > > > typec_port_type type) > > > > > > +{ > > > > > > + struct hd3ss3220 *hd3ss3220 = typec_get_drvdata(port); > > > > > > + > > > > > > + return hd3ss3220_set_port_type(hd3ss3220, type); } > > > > > > > > > static const struct typec_operations hd3ss3220_ops = { > > > > > > - .dr_set = hd3ss3220_dr_set > > > > > > + .dr_set = hd3ss3220_dr_set, > > > > > > + .port_type_set = hd3ss3220_port_type_set, > > > > > > }; > > > > > > > > > > So here I think you should implement the pr_set callback instead. > > > > > > > > I can do that, but based on the MODE_SELECT register description, > > > > it seems to me that this setting is fundamentally changing the > > > > operation mode of the chip, i.e. the state machine that is being > > > > run for initial > > > connection. > > > > So there would have to be a way of "resetting" it to be a > > > > dual-role port again, which the "pr_set" callback doesn't seem to have? > > > > This register can be written to set the HD3SS3220 mode > > > > operation. The ADDR pin must be set to I2C mode. If the default > > > > is maintained, HD3SS3220 shall operate according to the PORT > > > > pin levels and modes. The MODE_SELECT can only be > > > > changed when in the unattached state. > > > > 00 - DRP mode (start from unattached.SNK) (default) > > > > 01 - UFP mode (unattached.SNK) > > > > 10 - DFP mode (unattached.SRC) > > > > 11 - DRP mode (start from unattached.SNK) > > > > > > Okay, I see. This is not a case for pr_set. > > > > > > I'm still confused about the use case here. It seems you are not > > > interested in role swapping after all, so why would you need this > > > functionality to be exposed to the user space? > > > > > > I'm sorry if I've missed something. > > > > > > About the port_type attribute file itself. I would feel more > > > comfortable with it if it was allowed to be written only when there > > > is nothing connected to the port. At the very least, I think it > > > should be documented better so what it's really meant for would be more > clear to everybody. > > > > After researching some more about this operation, I came across the > > driver for TUSB320 [1] which seems to have a very similar behavior (also > from TI). > > [1] - https://lore.kernel.org/all/20220730180500.152004-1- > > marex@xxxxxxx/T/#ma7a322bc207414e4185c29d257ff30f5769f5d70 > > > > For one variant of the chip, the implementation relies on the CC > > disabling like in this patch. A different variant tests the current connection > status before proceeding. > > > > > Can you please provide your test logs? Adding them below. > > Previously I tested 2 devices with > Switching roles host->device->host using > > echo device > /sys/class/typec/port0/data_role > > and > > echo host > /sys/class/typec/port0/data_role Could you clarify what your test setup was? Did you control both sides of the USB connection and execute these commands on both sides? > > > Cheers, > Biju Here are my test logs for the "port type switch" functionality. The hd3ss3220 driver doesn't log much at all, I added one debug line (NOT part of the patch series) for testing purposes: diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c index c6922989a3cd..76fea657398a 100644 --- a/drivers/usb/typec/hd3ss3220.c +++ b/drivers/usb/typec/hd3ss3220.c @@ -199,6 +199,7 @@ static const struct typec_operations hd3ss3220_ops = { static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220) { enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220); + dev_info(hd3ss3220->dev, "Propagating role %s\n", usb_role_string(role_state)); usb_role_switch_set_role(hd3ss3220->role_sw, role_state); My test setup is as follows: +-------------------------------------------+ +--------------------+ | My device (Linux 6.12) | | | | +-----------------+ | | Remote device | | | +------I2C----+ | | | | | i.MX8 M Plus | +--------v-------+ | | - phone | | | with USB3 DRD | | | | | - USB-to-Ethernet| | | controller +----+ TI HD3SS3220 +-+------+ dongle | | | | | | | | - computer | | +-----------------+ +----------------+ | | | +-------------------------------------------+ +--------------------+ ======== Case 1: Device tree: power-role = "sink"; ======== # cat /sys/class/typec/port0/port_type [sink] # echo "source" > /sys/class/typec/port0/port_type zsh: permission denied: /sys/class/typec/port0/port_type # echo "dual" > /sys/class/typec/port0/port_type zsh: permission denied: /sys/class/typec/port0/port_type -> plug in ethernet adapter --> nothing happens -> unplug -> plug in Samsung phone [ 147.581160] hd3ss3220 4-0047: Propagating role device # cat /sys/class/typec/port0/data_role host [device] -> unplug [ 485.495874] hd3ss3220 4-0047: Propagating role none ======== Case 2: Device tree: power-role = "source"; ======== # cat /sys/class/typec/port0/port_type [source] # echo "sink" > /sys/class/typec/port0/port_type zsh: permission denied: /sys/class/typec/port0/port_type ->plug in ethernet adapter [ 94.590028] hd3ss3220 4-0047: Propagating role host [ 94.733892] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 94.739427] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 3 [ 94.747464] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220fe6d hci version 0x110 quirks 0x000080a001000010 [ 94.756900] xhci-hcd xhci-hcd.1.auto: irq 198, io mem 0x38100000 [ 94.763060] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 94.768565] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 4 [ 94.776246] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0 SuperSpeed [ 94.783382] hub 3-0:1.0: USB hub found [ 94.787178] hub 3-0:1.0: 1 port detected [ 94.791484] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 94.800068] hub 4-0:1.0: USB hub found [ 94.803859] hub 4-0:1.0: 1 port detected [ 95.781709] usb 4-1: new SuperSpeed USB device number 2 using xhci-hcd [ 96.195802] usbcore: registered new interface driver cdc_ether [ 96.396140] cdc_ncm 4-1:2.0: MAC-Address: a0:ce:c8:9e:54:a4 [ 96.401753] cdc_ncm 4-1:2.0: setting rx_max = 16384 [ 96.419345] cdc_ncm 4-1:2.0: setting tx_max = 16384 [ 96.434385] cdc_ncm 4-1:2.0 eth1: register 'cdc_ncm' at usb-xhci-hcd.1.auto-1, CDC NCM (NO ZLP), a0:ce:c8:9e:54:a4 [ 96.444940] usbcore: registered new interface driver cdc_ncm -> unplug [ 133.501500] hd3ss3220 4-0047: Propagating role none [ 133.506518] xhci-hcd xhci-hcd.1.auto: remove, state 1 [ 133.511636] usb usb4: USB disconnect, device number 1 [ 133.516732] usb 4-1: USB disconnect, device number 2 [ 133.521934] cdc_ncm 4-1:2.0 eth1: unregister 'cdc_ncm' usb-xhci-hcd.1.auto-1, CDC NCM (NO ZLP) [ 133.602023] xhci-hcd xhci-hcd.1.auto: USB bus 4 deregistered [ 133.610348] xhci-hcd xhci-hcd.1.auto: remove, state 4 [ 133.616023] usb usb3: USB disconnect, device number 1 [ 133.624475] xhci-hcd xhci-hcd.1.auto: USB bus 3 deregistered [ 133.733092] using host ethernet address: f8:dc:7a:a0:c0:ae [ 133.733104] using self ethernet address: 1A:22:33:44:55:66 [ 133.739134] g_ncm gadget.0: HOST MAC f8:dc:7a:a0:c0:ae [ 133.749825] g_ncm gadget.0: MAC 1a:22:33:44:55:66 [ 133.754587] g_ncm gadget.0: NCM Gadget [ 133.758354] g_ncm gadget.0: g_ncm ready -> plug in phone --> phone is getting charged -> unplug ========== Case 3: Device tree: power-role = "dual"; ========== # cat /sys/class/typec/port0/port_type [dual] source sink -> plug in ethernet adapter [ 252.688228] hd3ss3220 4-0047: Propagating role host [ 252.823920] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 252.829468] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 3 [ 252.837506] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220fe6d hci version 0x110 quirks 0x000080a001000010 [ 252.846943] xhci-hcd xhci-hcd.1.auto: irq 198, io mem 0x38100000 [ 252.853098] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 252.858614] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 4 [ 252.866307] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0 SuperSpeed [ 252.873424] hub 3-0:1.0: USB hub found [ 252.877221] hub 3-0:1.0: 1 port detected [ 252.881532] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 252.890122] hub 4-0:1.0: USB hub found [ 252.893911] hub 4-0:1.0: 1 port detected [ 253.872375] usb 4-1: new SuperSpeed USB device number 2 using xhci-hcd [ 254.282643] usbcore: registered new interface driver cdc_ether [ 254.483834] cdc_ncm 4-1:2.0: MAC-Address: a0:ce:c8:9e:54:a4 [ 254.489447] cdc_ncm 4-1:2.0: setting rx_max = 16384 [ 254.505900] cdc_ncm 4-1:2.0: setting tx_max = 16384 [ 254.520769] cdc_ncm 4-1:2.0 eth1: register 'cdc_ncm' at usb-xhci-hcd.1.auto-1, CDC NCM (NO ZLP), a0:ce:c8:9e:54:a4 [ 254.531322] usbcore: registered new interface driver cdc_ncm -> unplug [ 273.270877] usb 4-1: USB disconnect, device number 2 [ 273.276084] cdc_ncm 4-1:2.0 eth1: unregister 'cdc_ncm' usb-xhci-hcd.1.auto-1, CDC NCM (NO ZLP) [ 274.192136] hd3ss3220 4-0047: Propagating role none [ 274.197161] xhci-hcd xhci-hcd.1.auto: remove, state 4 [ 274.202251] usb usb4: USB disconnect, device number 1 [ 274.207994] xhci-hcd xhci-hcd.1.auto: USB bus 4 deregistered [ 274.213739] xhci-hcd xhci-hcd.1.auto: remove, state 4 [ 274.218841] usb usb3: USB disconnect, device number 1 [ 274.225032] xhci-hcd xhci-hcd.1.auto: USB bus 3 deregistered [ 274.335934] using host ethernet address: f8:dc:7a:a0:c0:ae [ 274.335947] using self ethernet address: 1A:22:33:44:55:66 [ 274.341994] g_ncm gadget.0: HOST MAC f8:dc:7a:a0:c0:ae [ 274.352692] g_ncm gadget.0: MAC 1a:22:33:44:55:66 [ 274.357455] g_ncm gadget.0: NCM Gadget [ 274.361225] g_ncm gadget.0: g_ncm ready -> plug in Windows PC [ 322.321716] hd3ss3220 4-0047: Propagating role device -> unplug [ 343.825254] hd3ss3220 4-0047: Propagating role none -> plug in phone [ 454.410340] hd3ss3220 4-0047: Propagating role host [ 454.558026] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 454.563594] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 3 [ 454.571652] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220fe6d hci version 0x110 quirks 0x000080a001000010 [ 454.581104] xhci-hcd xhci-hcd.1.auto: irq 198, io mem 0x38100000 [ 454.587295] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 454.592836] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 4 [ 454.600546] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0 SuperSpeed [ 454.607623] hub 3-0:1.0: USB hub found [ 454.611475] hub 3-0:1.0: 1 port detected [ 454.615817] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 454.624451] hub 4-0:1.0: USB hub found [ 454.628295] hub 4-0:1.0: 1 port detected [ 455.085270] usb 4-1: new SuperSpeed USB device number 2 using xhci-hcd [ 455.203088] cdc_acm 4-1:1.1: ttyACM0: USB ACM device [ 455.208249] usbcore: registered new interface driver cdc_acm [ 455.213947] cdc_acm: USB Abstract Control Model driver for USB modems and ISDN adapters -> unplug [ 477.940537] usb 4-1: USB disconnect, device number 2 [ 477.960408] hd3ss3220 4-0047: Propagating role none [ 477.965448] xhci-hcd xhci-hcd.1.auto: remove, state 1 [ 477.970570] usb usb4: USB disconnect, device number 1 [ 477.999753] xhci-hcd xhci-hcd.1.auto: USB bus 4 deregistered [ 478.005519] xhci-hcd xhci-hcd.1.auto: remove, state 4 [ 478.010605] usb usb3: USB disconnect, device number 1 [ 478.016507] xhci-hcd xhci-hcd.1.auto: USB bus 3 deregistered [ 478.124154] using host ethernet address: f8:dc:7a:a0:c0:ae [ 478.124168] using self ethernet address: 1A:22:33:44:55:66 [ 478.130222] g_ncm gadget.0: HOST MAC f8:dc:7a:a0:c0:ae [ 478.140901] g_ncm gadget.0: MAC 1a:22:33:44:55:66 [ 478.145663] g_ncm gadget.0: NCM Gadget [ 478.149433] g_ncm gadget.0: g_ncm ready # echo "source" > /sys/class/typec/port0/port_type # cat /sys/class/typec/port0/port_type # dual [source] sink -> plug in ethernet dongle --> same as before -> plug in phone --> same as before -> plug in Windows PC [ 593.662330] hd3ss3220 4-0047: Propagating role host [ 593.794008] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 593.800172] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 3 [ 593.808248] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220fe6d hci version 0x110 quirks 0x000080a001000010 [ 593.817695] xhci-hcd xhci-hcd.1.auto: irq 198, io mem 0x38100000 [ 593.823871] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller [ 593.829390] xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 4 [ 593.837075] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0 SuperSpeed [ 593.844370] hub 3-0:1.0: USB hub found [ 593.848153] hub 3-0:1.0: 1 port detected [ 593.852335] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 593.860805] hub 4-0:1.0: USB hub found [ 593.864586] hub 4-0:1.0: 1 port detected # echo "sink" > /sys/class/typec/port0/port_type # cat /sys/class/typec/port0/port_type dual source [sink] -> plug in ethernet dongle --> nothing happens -> plug in laptop / phone [ 726.770383] hd3ss3220 4-0047: Propagating role device -> unplug [ 729.842140] hd3ss3220 4-0047: Propagating role none As you can see, setting the port type really changes the behavior of the port w.r.t. the initial connection setup with the peer. I hope this helps. Best regards Oliver