| Message ID | 20211207143757.21895-1-heikki.krogerus@linux.intel.com |
|---|---|
| Headers | show |
| Series | acpi: Store _PLD information and convert users | expand |
On Tue, Dec 07, 2021 at 05:46:06PM +0200, Andy Shevchenko wrote: > On Tue, Dec 07, 2021 at 05:37:56PM +0300, Heikki Krogerus wrote: > > Instead of trying to keep track of the connections to the > > USB Type-C connectors separately, letting the component > > framework take care of that. > > > > From now on every USB Type-C connector will register itself > > as "aggregate" - component master - and anything that can be > > connected to it can then simply register itself as a generic > > component. > > > > The matching of the components and the connector shall rely > > on ACPI _PLD initially. Before registering itself as the > > aggregate, the connector will check the list of other > > devices that share the same ACPI _PLD with it, and add a > > component match entry for each one of them. Because only > > ACPI is supported for now, the driver shall only be build > > when ACPI is supported. > > > > This removes the need for the custom API that the driver > > exposed. The components and the connector can therefore > > exist completely independently of each other. The order in > > which they are registered, as well as are they modules or > > not, is now irrelevant. > > ... > > > +static int typec_port_compare(struct device *dev, void *fwnode) > > { > > > + return dev_fwnode(dev) == fwnode; > > } > > NIH device_match_fwnode() > > ... > > > + /* Component match for every device that shares the same _PLD. */ > > + list_for_each_entry(adev, &location->devices, location_list) { > > > + if (adev == ACPI_COMPANION(&con->dev)) > > device_match_acpi_dev() Ah, both look like great helpers. I'll this in v2. Thanks.
Hi Heikki, On Tue, Dec 7, 2021 at 6:37 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Hi, > > This removes the need for the drivers to always separately evaluate > the _PLD. With the USB Type-C connector and USB port mapping this > allows us to start using the component framework and remove the custom > APIs. > > So far the only users of the _PLD information have been the USB > drivers, but it seems it will be used also at least in some camera > drivers later. These nevertheless touch mostly USB drivers. > > Rafael, is it still OK if Greg takes these? > > Prashant, can you test these? I've applied the patches to a system with the requisite _PLD entries in firmware, and I'm not sure I can see the connectors getting created correctly. My setup is: Chromebook ------> Dell WD19TB dock (in USB+DisplayPort Alternate Mode) ----> USB Thumb drive. Here is the lsusb -t output before connecting the dock (omitting unrelated busses): localhost ~ # lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 Here is the lsusb -t output (omitting unrelated busses): localhost ~ # lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 |__ Port 2: Dev 15, If 0, Class=Hub, Driver=hub/4p, 10000M |__ Port 3: Dev 16, If 0, Class=Hub, Driver=hub/4p, 5000M |__ Port 3: Dev 18, If 0, Class=Mass Storage, Driver=usb-storage, 5000M |__ Port 4: Dev 17, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M I see the connector symlink for the root hub: localhost ~ # cd /sys/bus/usb/devices localhost /sys/bus/usb/devices # ls 2-2/port/connector data_role device firmware_node port1-cable port1-partner power power_operation_mode power_role preferred_role subsystem supported_accessory_modes uevent usb2-port2 usb3-port2 usb_power_delivery_revision usb_typec_revision vconn_source But for none of the children devices: localhost /sys/bus/usb/devices # ls 2-2.3/port/connector ls: cannot access '2-2.3/port/connector': No such file or directory localhost /sys/bus/usb/devices # ls 2-2.3.3/port/connector ls: cannot access '2-2.3.3/port/connector': No such file or directory localhost /sys/bus/usb/devices # ls 2-2.3\:1.0/port/connector ls: cannot access '2-2.3:1.0/port/connector': No such file or directory localhost /sys/bus/usb/devices # ls 2-2.3.3\:1.0/port/connector ls: cannot access '2-2.3.3:1.0/port/connector': No such file or directory Is this as you intended with the series? My interpretation was that each connected usb device would get a "connector" symlink, but I may have misinterpreted this. Best regards, -Prashant
Hi, Thanks for testing these.. On Wed, Dec 08, 2021 at 07:45:26PM -0800, Prashant Malani wrote: > Hi Heikki, > > On Tue, Dec 7, 2021 at 6:37 AM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > Hi, > > > > This removes the need for the drivers to always separately evaluate > > the _PLD. With the USB Type-C connector and USB port mapping this > > allows us to start using the component framework and remove the custom > > APIs. > > > > So far the only users of the _PLD information have been the USB > > drivers, but it seems it will be used also at least in some camera > > drivers later. These nevertheless touch mostly USB drivers. > > > > Rafael, is it still OK if Greg takes these? > > > > Prashant, can you test these? > > I've applied the patches to a system with the requisite _PLD entries > in firmware, and I'm not sure I can see the connectors getting created > correctly. > > My setup is: > > Chromebook ------> Dell WD19TB dock (in USB+DisplayPort Alternate > Mode) ----> USB Thumb drive. > > Here is the lsusb -t output before connecting the dock (omitting > unrelated busses): > localhost ~ # lsusb -t > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > > Here is the lsusb -t output (omitting unrelated busses): > localhost ~ # lsusb -t > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > |__ Port 2: Dev 15, If 0, Class=Hub, Driver=hub/4p, 10000M > |__ Port 3: Dev 16, If 0, Class=Hub, Driver=hub/4p, 5000M > |__ Port 3: Dev 18, If 0, Class=Mass Storage, > Driver=usb-storage, 5000M > |__ Port 4: Dev 17, If 0, Class=Vendor Specific Class, > Driver=r8152, 5000M > > I see the connector symlink for the root hub: > > localhost ~ # cd /sys/bus/usb/devices > localhost /sys/bus/usb/devices # ls 2-2/port/connector > data_role device firmware_node port1-cable port1-partner power > power_operation_mode power_role preferred_role subsystem > supported_accessory_modes uevent usb2-port2 usb3-port2 > usb_power_delivery_revision usb_typec_revision vconn_source > > But for none of the children devices: > > localhost /sys/bus/usb/devices # ls 2-2.3/port/connector > ls: cannot access '2-2.3/port/connector': No such file or directory > localhost /sys/bus/usb/devices # ls 2-2.3.3/port/connector > ls: cannot access '2-2.3.3/port/connector': No such file or directory > localhost /sys/bus/usb/devices # ls 2-2.3\:1.0/port/connector > ls: cannot access '2-2.3:1.0/port/connector': No such file or directory > localhost /sys/bus/usb/devices # ls 2-2.3.3\:1.0/port/connector > ls: cannot access '2-2.3.3:1.0/port/connector': No such file or directory > > Is this as you intended with the series? My interpretation was that > each connected usb device would get a "connector" symlink, but I may > have misinterpreted this. It is as intended. The usb ports on the board will have the connector symlink, not the devices attached to them - the firmware is only aware of the connectors on the board of course. It looks like this series is working as it should. If you want to extend this solution so that also every device in the usb topology will have the link to the connector on board, then that should be now possible, but that is out side of the scope of this series. You need to propose that separately. But I must ask, why can't you just walk down the topology until you reach the on-board ports that will have the connector links? thanks,
Hey Heikki, On Thu, Dec 9, 2021 at 2:06 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Hi, > > Thanks for testing these.. > > On Wed, Dec 08, 2021 at 07:45:26PM -0800, Prashant Malani wrote: > > Hi Heikki, > > > > On Tue, Dec 7, 2021 at 6:37 AM Heikki Krogerus > > <heikki.krogerus@linux.intel.com> wrote: > > > > > > Hi, > > > > > > This removes the need for the drivers to always separately evaluate > > > the _PLD. With the USB Type-C connector and USB port mapping this > > > allows us to start using the component framework and remove the custom > > > APIs. > > > > > > So far the only users of the _PLD information have been the USB > > > drivers, but it seems it will be used also at least in some camera > > > drivers later. These nevertheless touch mostly USB drivers. > > > > > > Rafael, is it still OK if Greg takes these? > > > > > > Prashant, can you test these? > > > > I've applied the patches to a system with the requisite _PLD entries > > in firmware, and I'm not sure I can see the connectors getting created > > correctly. > > > > My setup is: > > > > Chromebook ------> Dell WD19TB dock (in USB+DisplayPort Alternate > > Mode) ----> USB Thumb drive. > > > > Here is the lsusb -t output before connecting the dock (omitting > > unrelated busses): > > localhost ~ # lsusb -t > > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > > > > Here is the lsusb -t output (omitting unrelated busses): > > localhost ~ # lsusb -t > > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > > |__ Port 2: Dev 15, If 0, Class=Hub, Driver=hub/4p, 10000M > > |__ Port 3: Dev 16, If 0, Class=Hub, Driver=hub/4p, 5000M > > |__ Port 3: Dev 18, If 0, Class=Mass Storage, > > Driver=usb-storage, 5000M > > |__ Port 4: Dev 17, If 0, Class=Vendor Specific Class, > > Driver=r8152, 5000M > > > > I see the connector symlink for the root hub: > > > > localhost ~ # cd /sys/bus/usb/devices > > localhost /sys/bus/usb/devices # ls 2-2/port/connector > > data_role device firmware_node port1-cable port1-partner power > > power_operation_mode power_role preferred_role subsystem > > supported_accessory_modes uevent usb2-port2 usb3-port2 > > usb_power_delivery_revision usb_typec_revision vconn_source > > > > But for none of the children devices: > > > > localhost /sys/bus/usb/devices # ls 2-2.3/port/connector > > ls: cannot access '2-2.3/port/connector': No such file or directory > > localhost /sys/bus/usb/devices # ls 2-2.3.3/port/connector > > ls: cannot access '2-2.3.3/port/connector': No such file or directory > > localhost /sys/bus/usb/devices # ls 2-2.3\:1.0/port/connector > > ls: cannot access '2-2.3:1.0/port/connector': No such file or directory > > localhost /sys/bus/usb/devices # ls 2-2.3.3\:1.0/port/connector > > ls: cannot access '2-2.3.3:1.0/port/connector': No such file or directory > > > > Is this as you intended with the series? My interpretation was that > > each connected usb device would get a "connector" symlink, but I may > > have misinterpreted this. > > It is as intended. The usb ports on the board will have the connector > symlink, not the devices attached to them - the firmware is only aware > of the connectors on the board of course. It looks like this series is > working as it should. Thanks for clarifying my understanding here. > > If you want to extend this solution so that also every device in the > usb topology will have the link to the connector on board, then that > should be now possible, but that is out side of the scope of this > series. You need to propose that separately. > > But I must ask, why can't you just walk down the topology until you > reach the on-board ports that will have the connector links? > Right, we can certainly do that; having it in each device is just convenient. But as you said, that's the subject of another series. You mentioned there would be a v2, so I'll add my Tested-By then. Best regards,
On Thu, Dec 09, 2021 at 11:45:27AM -0800, Prashant Malani wrote: > Hey Heikki, > > On Thu, Dec 9, 2021 at 2:06 AM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > Hi, > > > > Thanks for testing these.. > > > > On Wed, Dec 08, 2021 at 07:45:26PM -0800, Prashant Malani wrote: > > > Hi Heikki, > > > > > > On Tue, Dec 7, 2021 at 6:37 AM Heikki Krogerus > > > <heikki.krogerus@linux.intel.com> wrote: > > > > > > > > Hi, > > > > > > > > This removes the need for the drivers to always separately evaluate > > > > the _PLD. With the USB Type-C connector and USB port mapping this > > > > allows us to start using the component framework and remove the custom > > > > APIs. > > > > > > > > So far the only users of the _PLD information have been the USB > > > > drivers, but it seems it will be used also at least in some camera > > > > drivers later. These nevertheless touch mostly USB drivers. > > > > > > > > Rafael, is it still OK if Greg takes these? > > > > > > > > Prashant, can you test these? > > > > > > I've applied the patches to a system with the requisite _PLD entries > > > in firmware, and I'm not sure I can see the connectors getting created > > > correctly. > > > > > > My setup is: > > > > > > Chromebook ------> Dell WD19TB dock (in USB+DisplayPort Alternate > > > Mode) ----> USB Thumb drive. > > > > > > Here is the lsusb -t output before connecting the dock (omitting > > > unrelated busses): > > > localhost ~ # lsusb -t > > > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > > > > > > Here is the lsusb -t output (omitting unrelated busses): > > > localhost ~ # lsusb -t > > > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/3p, 10000M/x2 > > > |__ Port 2: Dev 15, If 0, Class=Hub, Driver=hub/4p, 10000M > > > |__ Port 3: Dev 16, If 0, Class=Hub, Driver=hub/4p, 5000M > > > |__ Port 3: Dev 18, If 0, Class=Mass Storage, > > > Driver=usb-storage, 5000M > > > |__ Port 4: Dev 17, If 0, Class=Vendor Specific Class, > > > Driver=r8152, 5000M > > > > > > I see the connector symlink for the root hub: > > > > > > localhost ~ # cd /sys/bus/usb/devices > > > localhost /sys/bus/usb/devices # ls 2-2/port/connector > > > data_role device firmware_node port1-cable port1-partner power > > > power_operation_mode power_role preferred_role subsystem > > > supported_accessory_modes uevent usb2-port2 usb3-port2 > > > usb_power_delivery_revision usb_typec_revision vconn_source > > > > > > But for none of the children devices: > > > > > > localhost /sys/bus/usb/devices # ls 2-2.3/port/connector > > > ls: cannot access '2-2.3/port/connector': No such file or directory > > > localhost /sys/bus/usb/devices # ls 2-2.3.3/port/connector > > > ls: cannot access '2-2.3.3/port/connector': No such file or directory > > > localhost /sys/bus/usb/devices # ls 2-2.3\:1.0/port/connector > > > ls: cannot access '2-2.3:1.0/port/connector': No such file or directory > > > localhost /sys/bus/usb/devices # ls 2-2.3.3\:1.0/port/connector > > > ls: cannot access '2-2.3.3:1.0/port/connector': No such file or directory > > > > > > Is this as you intended with the series? My interpretation was that > > > each connected usb device would get a "connector" symlink, but I may > > > have misinterpreted this. > > > > It is as intended. The usb ports on the board will have the connector > > symlink, not the devices attached to them - the firmware is only aware > > of the connectors on the board of course. It looks like this series is > > working as it should. > > Thanks for clarifying my understanding here. > > > > > If you want to extend this solution so that also every device in the > > usb topology will have the link to the connector on board, then that > > should be now possible, but that is out side of the scope of this > > series. You need to propose that separately. > > > > But I must ask, why can't you just walk down the topology until you > > reach the on-board ports that will have the connector links? > > > > Right, we can certainly do that; having it in each device is just > convenient. But as you said, that's the subject of another series. > > You mentioned there would be a v2, so I'll add my Tested-By then. Great! I don't know does this help you, or if this is exactly what you had in mind, but I am planning to link the USB device attached to the connector to the USB Type-C partner device that we have (the USB device attached to the connector is the Type-C partner). So in your example, the Dell dock USB hub will have the symlink to the Type-C partner device, but the USB mass storage device attached to the dock will not (because it's not our USB Type-C partner - the dock is our USB Type-C partner). But I want to do that separately, as the next step in any case. thanks,