| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<ZR1P278MB1022A148E41AECEC8718DBD39F352@ZR1P278MB1022.CHEP278.PROD.OUTLOOK.COM>
Date: Mon, 2 Dec 2024 14:27:37 +0000
From: Facklam, Olivér <oliver.facklam@...hlke.com>
To: Biju Das <biju.das.jz@...renesas.com>
CC: Biju Das <biju.das@...renesas.com>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, "linux-usb@...r.kernel.org"
<linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "von Heyl, Benedict"
<benedict.vonheyl@...hlke.com>, Först, Mathis
<mathis.foerst@...hlke.com>, "Glettig, Michael"
<Michael.Glettig@...hlke.com>, Heikki Krogerus
<heikki.krogerus@...ux.intel.com>
Subject: RE: [PATCH v2 3/4] usb: typec: hd3ss3220: support configuring port
type
Hello Biju,
Thanks for your response and sorry for the delay.
> -----Original Message-----
> From: Biju Das <biju.das.jz@...renesas.com>
> 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@...hlke.com>
> > 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@...ux.intel.com>
> > > 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@...ux.intel.com>
> > > > > 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@...hlke.com>
> > > > > > ---
> > > > > > 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@...x.de/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
Powered by blists - more mailing lists