USB: add generic onboard USB HUB driver

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

Quoted 15 lines · On Monday 14 December 2015 15:26:11 Peter Chen wrote:

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

Quoted 3 lines

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

I still think need to do this properly by representing the hub device as a USB device, using power sequencing code like MMC does.

Arnd

Quoted 15 lines · On Monday 14 December 2015 15:26:11 Peter Chen wrote:

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

Quoted 3 lines

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

I still think need to do this properly by representing the hub device as a USB device, using power sequencing code like MMC does.

Quoted 1 line · Arnd Bergmann

On Monday 14 December 2015 15:26:11 Peter Chen wrote:

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

Quoted 12 lines · On Wednesday 16 December 2015 16:59:39 Rob Herring wrote:

Quoted 1 line · Arnd Bergmann

On Monday 14 December 2015 15:26:11 Peter Chen wrote:

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Quoted 4 lines

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

Yes, good idea. I was also advocating that approach for MMC at some point, but the power sequencing made it in the end.

Quoted 13 lines · Arnd Bergmann

Quoted 12 lines · On Wednesday 16 December 2015 16:59:39 Rob Herring wrote:

Quoted 1 line · Arnd Bergmann

On Monday 14 December 2015 15:26:11 Peter Chen wrote:

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Quoted 8 lines

Quoted 4 lines

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

Yes, good idea. I was also advocating that approach for MMC at some point, but the power sequencing made it in the end.

Currently, my design is much like Rob proposal's. I will populate all children device under HUB's node (controller = 1st level hub), and each special onboard USB device needs a platform driver, it can be probed according to compatible string, and at its probe, it can register itself as usb device if needed, and handle properties described at its node. Sample code like below:

Quoted 23 lines · Peter Chen

Quoted 13 lines · Arnd Bergmann

Quoted 12 lines · On Wednesday 16 December 2015 16:59:39 Rob Herring wrote:

Quoted 1 line · Arnd Bergmann

On Monday 14 December 2015 15:26:11 Peter Chen wrote:

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Quoted 8 lines

Quoted 4 lines

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

Yes, good idea. I was also advocating that approach for MMC at some point, but the power sequencing made it in the end.

Quoted 6 lines

Currently, my design is much like Rob proposal's. I will populate all children device under HUB's node (controller = 1st level hub), and each special onboard USB device needs a platform driver, it can be probed according to compatible string, and at its probe, it can register itself as usb device if needed, and handle properties described at its node. Sample code like below:

It is not like my proposal because I don't think the design should be using a platform driver.

arch/arm/boot/dts/imx6qdl-sabreauto.dtsi line 597

Quoted 13 lines

arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +5 -0

index 8263fc1..7451f7d 100644
@@ -590,6 +590,16 @@
 &usbh1 {
        vbus-supply = <&reg_usb_h1_vbus>;
        status = "okay";
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       hub: usb2415 at 01 {
+               compatible = "generic-onboard-hub";

Please follow the already defined naming scheme for USB devices.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 16 lines · Arnd Bergmann

Quoted 15 lines · On Monday 14 December 2015 15:26:11 Peter Chen wrote:

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

Quoted 6 lines

Quoted 3 lines

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

I still think need to do this properly by representing the hub device as a USB device, using power sequencing code like MMC does.

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

Quoted 33 lines · Rob Herring

Quoted 16 lines · Arnd Bergmann

Quoted 15 lines · On Monday 14 December 2015 15:26:11 Peter Chen wrote:

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

Quoted 6 lines

Quoted 3 lines

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

I still think need to do this properly by representing the hub device as a USB device, using power sequencing code like MMC does.

I agree on doing it properly, but am not sure that pwrseq binding for MMC is proper. The pwrseq binding is fairly limited and working around the driver model IMO. Hubs may also need I2C setup which I don't think could be done generically other than some defined sequence of i2c transactions. The current project I'm working on needs to use I2C to configure the hub to use HSIC mode for example. I really think we need a pre-probe driver hook to handle this. That would allow device specific setup to live in the driver.

Quoted 4 lines

Perhaps a more simple approach would be just forcing driver probe if a DT node is present. I'm not all that familiar with USB drivers, but presumably there is some setup before probe like setting the USB device address. We'd have to allow doing that later during probe.

It's not clear (to me, anyway) how this is meant to work. USB is a completely discoverable bus: There is no way to represent devices statically; they have to be discovered. But a device can't be discovered unless it is functional, so if an onboard hub (or any other sort of USB device) requires power, clocks, or something similar in order to function, it won't be discovered. There won't be any device structure to probe, and "forcing driver probe" won't accomplish anything.

It seems to me that the only way something like this could be made to work is if the necessary actions are keyed off the host controller (and in particular, _not_ the hub driver), presumably at the time the controller is probed.

With anything else, you run the risk that the necessary resources won't get enabled before they are needed.

Quoted 9 lines · Alan Stern

It's not clear (to me, anyway) how this is meant to work. USB is a completely discoverable bus: There is no way to represent devices statically; they have to be discovered. But a device can't be discovered unless it is functional, so if an onboard hub (or any other sort of USB device) requires power, clocks, or something similar in order to function, it won't be discovered. There won't be any device structure to probe, and "forcing driver probe" won't accomplish anything.

Quoted 4 lines

It seems to me that the only way something like this could be made to work is if the necessary actions are keyed off the host controller (and in particular, _not_ the hub driver), presumably at the time the controller is probed.

The reason why I put the code at hub driver is the onboard USB device may be at 2nd level hub, at hub driver, we can know all devices under this level hub.

Quoted 3 lines

With anything else, you run the risk that the necessary resources won't get enabled before they are needed.

Sorry, I can't understand what you mean

Quoted 11 lines · Peter Chen

Quoted 9 lines · Alan Stern

It's not clear (to me, anyway) how this is meant to work. USB is a completely discoverable bus: There is no way to represent devices statically; they have to be discovered. But a device can't be discovered unless it is functional, so if an onboard hub (or any other sort of USB device) requires power, clocks, or something similar in order to function, it won't be discovered. There won't be any device structure to probe, and "forcing driver probe" won't accomplish anything.

Quoted 8 lines

Quoted 4 lines

It seems to me that the only way something like this could be made to work is if the necessary actions are keyed off the host controller (and in particular, _not_ the hub driver), presumably at the time the controller is probed.

The reason why I put the code at hub driver is the onboard USB device may be at 2nd level hub, at hub driver, we can know all devices under this level hub.

Maybe. I'm not convinced. See below.

Quoted 5 lines

Quoted 3 lines

With anything else, you run the risk that the necessary resources won't get enabled before they are needed.

Sorry, I can't understand what you mean

Suppose you have a platform driver to manage the device's resources, and the platform driver is in loadable module. Suppose the device can be detected even before the resources have been initialized, but it can't work correctly.

Then what happens when the platform driver's module doesn't get loaded until _after_ the device has been detected and after the device failed to initialize?

Quoted 6 lines

(quoted diff) +6 -0

+static int hub_of_children_register(struct usb_device *hdev)
+{
+     struct device *dev;
+
+     if (hdev->parent)
+             dev = &hdev->dev;

Review

Quoted 5 lines

(quoted diff) +5 -0

+     else
+             dev = bus_to_hcd(hdev->bus)->self.controller;
+
+     if (!dev->of_node)
+             return 0;

Suppose hdev->parent is not NULL (hdev isn't the root hub -- maybe it's a 2nd-level hub). Then how did hdev->dev->of_node get set?

There are two problems which needs device tree to support, I have below solutions for them, please help to see if it is reasonable.

1. The USB device can't work without external clock, power, reset operation. At device tree, we have a node to describe external signals like clocks, gpios for all onboard devices under this controller. And this node is as phandle for host controller, see below:

arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +17 -0

@@ -108,6 +108,21 @@
                default-brightness-level = <7>;
                status = "okay";
        };
+
+       usbh1_pre_operation: usbh1_pre_operation {
+               clocks = <&clks IMX6QDL_CLK_1>,
+                        <&clks IMX6QDL_CLK_2>,
+                        <&clks IMX6QDL_CLK_3>,
+                        <&clks IMX6QDL_CLK_4>,
+                        <&clks IMX6QDL_CLK_5>,
+                        <&clks IMX6QDL_CLK_6>;
+               reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>,
+                             <&gpio4 7 GPIO_ACTIVE_LOW>,
+                             <&gpio3 25 GPIO_ACTIVE_LOW>,
+                             <&gpio3 27 GPIO_ACTIVE_LOW>,
+                             <&gpio4 4 GPIO_ACTIVE_LOW>,
+                             <&gpio4 6 GPIO_ACTIVE_LOW>;
+       };
 };
 
 &clks {

@@ -590,6 +605,8 @@
 &usbh1 {
        vbus-supply = <&reg_usb_h1_vbus>;
        status = "okay";
+
+       devices-pre-operation = <&usbh1_pre_operation>
 };
 

At code, we add one API of_usb_pre_operation to get clocks and gpios through host controller's device node, and this API is called at usb_add_hcd, and opposite
operation is called at usb_remove_hcd.

This solution is almost the same with MMC power sequence solution. (see drivers/mmc/core/pwrseq.c)

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

This problem is hard to handle, I have a solution, but it can't cover two same devices
under the same HUB ports. My solution is let USB know device node, the main idea is similar with PCIi's (see drivers/of/of_pci.c, drivers/pci/of.c), the most difficulty is
find correct node for USB device after bus enumeration. Once the device is recognized,
the USB core will create a USB device for it, since we know its parent device, and its parent
device (eg, the host controller) has device node, and we can find all children nodes under
this node, if the child's {vid, pid} is the same with {vid, pid} the device descriptor we read, we
assign this node as usb device's node.

At USB class/device driver, we can get the properties/phandles under this device node, and register
them.

At device tree, we need to describe the bus topology for this USB device.

Quoted 45 lines · Peter Chen

There are two problems which needs device tree to support, I have below solutions for them, please help to see if it is reasonable.

1. The USB device can't work without external clock, power, reset operation. At device tree, we have a node to describe external signals like clocks, gpios for all onboard devices under this controller. And this node is as phandle for host controller, see below:

arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +17 -0

@@ -108,6 +108,21 @@
                default-brightness-level = <7>;
                status = "okay";
        };
+
+       usbh1_pre_operation: usbh1_pre_operation {
+               clocks = <&clks IMX6QDL_CLK_1>,
+                        <&clks IMX6QDL_CLK_2>,
+                        <&clks IMX6QDL_CLK_3>,
+                        <&clks IMX6QDL_CLK_4>,
+                        <&clks IMX6QDL_CLK_5>,
+                        <&clks IMX6QDL_CLK_6>;
+               reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>,
+                             <&gpio4 7 GPIO_ACTIVE_LOW>,
+                             <&gpio3 25 GPIO_ACTIVE_LOW>,
+                             <&gpio3 27 GPIO_ACTIVE_LOW>,
+                             <&gpio4 4 GPIO_ACTIVE_LOW>,
+                             <&gpio4 6 GPIO_ACTIVE_LOW>;
+       };
 };
 
 &clks {

@@ -590,6 +605,8 @@
 &usbh1 {
        vbus-supply = <&reg_usb_h1_vbus>;
        status = "okay";
+
+       devices-pre-operation = <&usbh1_pre_operation>
 };
 

At code, we add one API of_usb_pre_operation to get clocks and gpios through host controller's device node, and this API is called at usb_add_hcd, and opposite
operation is called at usb_remove_hcd.

Quoted 2 lines

This solution is almost the same with MMC power sequence solution. (see drivers/mmc/core/pwrseq.c)

That seems reasonable to me.

Quoted 6 lines

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Quoted 14 lines

This problem is hard to handle, I have a solution, but it can't cover two same devices
under the same HUB ports. My solution is let USB know device node, the main idea is similar with PCIi's (see drivers/of/of_pci.c, drivers/pci/of.c), the most difficulty is
find correct node for USB device after bus enumeration. Once the device is recognized,
the USB core will create a USB device for it, since we know its parent device, and its parent
device (eg, the host controller) has device node, and we can find all children nodes under
this node, if the child's {vid, pid} is the same with {vid, pid} the device descriptor we read, we
assign this node as usb device's node.

I don't really understand this. However, you can always specify a USB device by giving its port number on the parent hub, and the hub's port number on _its_ parent hub, and so on back to the root hub and host controller. That works even if you're not using DT or OF or ACPI.

Quoted 5 lines

I don't really understand this. However, you can always specify a USB device by giving its port number on the parent hub, and the hub's port number on _its_ parent hub, and so on back to the root hub and host controller. That works even if you're not using DT or OF or ACPI.

Thanks, so the HUB's physical port number is the same with its logical port number which reported at its descriptor?

If we assumed all HUBs follow it,
then we can use port number to align the devices which we described at DT and detected by USB bus.

If the host controller has DT supported, and there are two ports connects two different onboard devices. When the device is found by the bus, we will know its portnum and parent (see usb_alloc_dev), and we know parent's device node, so we will know two children node by iterate its parent, and match its port number
(property "reg" at below dts node) with udev->portnum we assigned at usb_alloc_dev.
Then we can let the device know DT.

After USB device knows DT, it can handle DT properties at its driver.

drivers/usb/core/usb.c +0 -0

        INIT_LIST_HEAD(&dev->filelist);
 

&usbh1 {
        vbus-supply = <&reg_usb_h1_vbus>;
        status = "okay";

        devices-pre-operation = <&usbh1_pre_operation>

        #address-cells = <1>;
        #size-cells = <0>;
        usb: usb_mfd2415 at 01 {
                compatible = "usb multi-device";
                reg = <0x01>;
                clocks = <&clks IMX6QDL_CLK_CKO>;
                reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
                reset-duration-us = <3000>;
                gpio-controller;
                #gpio-cells = <2>;
        };

        usb: usb_mfd2415 at 02 {
                compatible = "usb multi-device";

                reg = <0x02>;
                clocks = <&clks IMX6QDL_CLK_CKO2>;
                reset-gpios = <&gpio7 13 GPIO_ACTIVE_LOW>;
                reset-duration-us = <3000>;
                gpio-controller;
                #gpio-cells = <2>;
        };
};

Quoted 13 lines · Alan Stern

Quoted 11 lines · Peter Chen

Quoted 9 lines · Alan Stern

It's not clear (to me, anyway) how this is meant to work. USB is a completely discoverable bus: There is no way to represent devices statically; they have to be discovered. But a device can't be discovered unless it is functional, so if an onboard hub (or any other sort of USB device) requires power, clocks, or something similar in order to function, it won't be discovered. There won't be any device structure to probe, and "forcing driver probe" won't accomplish anything.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

It seems to me that the only way something like this could be made to work is if the necessary actions are keyed off the host controller (and in particular, _not_ the hub driver), presumably at the time the controller is probed.

Quoted 6 lines

Quoted 6 lines

(quoted diff) +6 -0

+static int hub_of_children_register(struct usb_device *hdev)
+{
+     struct device *dev;
+
+     if (hdev->parent)
+             dev = &hdev->dev;

Quoted 9 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+     else
+             dev = bus_to_hcd(hdev->bus)->self.controller;
+
+     if (!dev->of_node)
+             return 0;

Suppose hdev->parent is not NULL (hdev isn't the root hub -- maybe it's a 2nd-level hub). Then how did hdev->dev->of_node get set?

Yes, the USB device doesn't know device node.

There are two problems which needs device tree to support, I have below solutions for them, please help to see if it is reasonable.

1. The USB device can't work without external clock, power, reset operation. At device tree, we have a node to describe external signals like clocks, gpios for all onboard devices under this controller. And this node is as phandle for host controller, see below:

arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +15 -0

@@ -108,6 +108,21 @@
                default-brightness-level = <7>;
                status = "okay";
        };
+
+       usbh1_pre_operation: usbh1_pre_operation {
+               clocks = <&clks IMX6QDL_CLK_1>,
+                        <&clks IMX6QDL_CLK_2>,
+                        <&clks IMX6QDL_CLK_3>,
+                        <&clks IMX6QDL_CLK_4>,
+                        <&clks IMX6QDL_CLK_5>,
+                        <&clks IMX6QDL_CLK_6>;
+               reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>,
+                             <&gpio4 7 GPIO_ACTIVE_LOW>,
+                             <&gpio3 25 GPIO_ACTIVE_LOW>,
+                             <&gpio3 27 GPIO_ACTIVE_LOW>,
+                             <&gpio4 4 GPIO_ACTIVE_LOW>,
+                             <&gpio4 6 GPIO_ACTIVE_LOW>;
+       };
 };
 
 &clks {

At code, we add one API of_usb_pre_operation to get clocks and gpios through host controller's device node, and this API is called at usb_add_hcd, and opposite
operation is called at usb_remove_hcd.

This solution is almost the same with MMC power sequence solution. (see drivers/mmc/core/pwrseq.c)

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

It's not clear (to me, anyway) how this is meant to work. USB is a completely discoverable bus: There is no way to represent devices statically; they have to be discovered. But a device can't be discovered unless it is functional, so if an onboard hub (or any other sort of USB device) requires power, clocks, or something similar in order to function, it won't be discovered. There won't be any device structure to probe, and "forcing driver probe" won't accomplish anything.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

It seems to me that the only way something like this could be made to work is if the necessary actions are keyed off the host controller (and in particular, _not_ the hub driver), presumably at the time the controller is probed.

The problem with putting this in the the host controller driver is it assumes the initialization sequence is generic enough to be described in DT and that initialization is the only time we need DT data. The MFD example is a perfect example of another case. Suspend/resume also comes to mind. Even if we could put the control in both places, that is poor design IMO. I'd rather just make it a requirement that the bootloader do enough setup that devices can be discovered.

Quoted 5 lines

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Are you talking about a device hard wired on the same board or something like GPIOs on FTDI chip which could be hot-plugged in any host (including non-DT based)?

For the hotplug case, we will need a way to associate a DT overlay with the USB device and there may not even be a base DT to map the overlay into. In this case, the USB device's driver will need to load the overlay and trigger enumerating the child devices. Anyway, this is a separate issue from your problem.

Quoted 6 lines · Rob Herring

Quoted 5 lines

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Quoted 3 lines

Are you talking about a device hard wired on the same board or something like GPIOs on FTDI chip which could be hot-plugged in any host (including non-DT based)?

I talked about the case that the device hard wired on the board. Hot-plug device's bus topology is unknown, we can't describe it statically at dts.

Quoted 6 lines

For the hotplug case, we will need a way to associate a DT overlay with the USB device and there may not even be a base DT to map the overlay into. In this case, the USB device's driver will need to load the overlay and trigger enumerating the child devices. Anyway, this is a separate issue from your problem.

Since both you and Alan agree with my problem should be fixed at bootloader, I give the kernel solution up.

The another thing I open to discuss is how to let USB devices know its device node, the user reported issue that they can't handle interfaces below in USB device since that.

Quoted 11 lines · Peter Chen

Quoted 9 lines · Rob Herring

Quoted 5 lines

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Are you talking about a device hard wired on the same board or something like GPIOs on FTDI chip which could be hot-plugged in any host (including non-DT based)?

Quoted 3 lines

I talked about the case that the device hard wired on the board. Hot-plug device's bus topology is unknown, we can't describe it statically at dts.

Correct, upstream (the USB side) can't be described, but it is the downstream side we care about describing.

Quoted 8 lines

Quoted 7 lines

For the hotplug case, we will need a way to associate a DT overlay with the USB device and there may not even be a base DT to map the overlay into. In this case, the USB device's driver will need to load the overlay and trigger enumerating the child devices. Anyway, this is a separate issue from your problem.

Quoted 2 lines

Since both you and Alan agree with my problem should be fixed at bootloader, I give the kernel solution up.

Surprising, no one ever seems to agree with that solution... There will be people with this problem and unable to update their bootloader.

Quoted 3 lines

The another thing I open to discuss is how to let USB devices know its device node, the user reported issue that they can't handle interfaces below in USB device since that.

The driver for the USB device would need to load a DT overlay and from that it should be able to get its node and child nodes. The hard part is figuring out where to put the sub tree defined in the overlay as either there is no base DT (think x86) or there is but we don't want to describe the whole USB bus topology in DT (in theory we could populate USB nodes dynamically as USB devices are discovered).

Rob

Quoted 7 lines · Peter Chen

Quoted 6 lines · Rob Herring

Quoted 5 lines

2. There are MFD USB devices, which includes several interfaces under USB device,
like i2c, gpios, etc. Due to lack of device tree support, USB class/device driver doesn't know
which kinds of interfaces are needed for this board.

Quoted 7 lines

Quoted 3 lines

Are you talking about a device hard wired on the same board or something like GPIOs on FTDI chip which could be hot-plugged in any host (including non-DT based)?

I talked about the case that the device hard wired on the board. Hot-plug device's bus topology is unknown, we can't describe it statically at dts.

Correct, upstream (the USB side) can't be described, but it is the downstream side we care about describing.

Quoted 7 lines

Quoted 6 lines

For the hotplug case, we will need a way to associate a DT overlay with the USB device and there may not even be a base DT to map the overlay into. In this case, the USB device's driver will need to load the overlay and trigger enumerating the child devices. Anyway, this is a separate issue from your problem.

Quoted 2 lines

Since both you and Alan agree with my problem should be fixed at bootloader, I give the kernel solution up.

Surprising, no one ever seems to agree with that solution... There will be people with this problem and unable to update their bootloader.

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

Hi Peter,

Quoted 10 lines · Peter Chen

Hi all,

There is a known issue that the USB code can't handle USB HUB's external pins well, in that case, it may cause some onboard USB HUBs can't work since their PHY's clock or reset pin needs to operate.

The user reported this issue at below:
http://www.spinics.net/lists/linux-usb/msg131502.html

Quoted 8 lines

In this patch set, I add a generic onboard USB HUB driver to handle this problem, the external signals will be configured before usb controller's initialization, it much likes we did it at board code before.

The user needs to add this generic hub node at dts to support it.

@The udoo users, help to test please.

This is what I get with your series applied:

[    2.288300] usb 1-1: device descriptor read/64, error -71
[    2.518083] usb 1-1: new full-speed USB device number 3 using ci_hdrc
[    2.738078] usb 1-1: device descriptor read/64, error -71
[    3.058078] usb 1-1: device descriptor read/64, error -71
[    3.288079] usb 1-1: new full-speed USB device number 4 using ci_hdrc
[    3.768069] usb 1-1: device not accepting address 4, error -71
[    3.888084] usb 1-1: new full-speed USB device number 5 using ci_hdrc
[    4.368067] usb 1-1: device not accepting address 5, error -71
[    4.374117] usb usb1-port1: unable to enumerate USB device

Thanks, Fabio.

I am afraid I forget to set gpio as output, would you please apply below patch against my original ones:

Quoted 4 lines · Peter Chen

Thanks, Fabio.

I am afraid I forget to set gpio as output, would you please apply below patch against my original ones:

Same error happens with these changes applied.

Here are more details: if I run a pure 4.3.2 then I do see the USB stick to get detected if I boot it with the USB stick connected to Udoo board:

[    2.170178] usb 1-1.1: new high-speed USB device number 3 using ci_hdrc
[    2.305840] usb-storage 1-1.1:1.0: USB Mass Storage device detected
[    2.314803] scsi host1: usb-storage 1-1.1:1.0
[    2.400283] usb 1-1.3: new high-speed USB device number 4 using ci_hdrc
[    3.327925] scsi 1:0:0:0: Direct-Access     General  USB Flash Disk   1.0  P2
[    3.347070] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks: (4.01 GB/3.73)
[    3.356181] sd 1:0:0:0: [sda] Write Protect is off
[    3.362550] sd 1:0:0:0: [sda] No Caching mode page found
[    3.367899] sd 1:0:0:0: [sda] Assuming drive cache: write through
[    3.387401]  sda: sda1
[    3.400238] sd 1:0:0:0: [sda] Attached SCSI removable disk
[    4.931847] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow contx
[    4.941414] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[    4.961400] Sending DHCP requests ., OK
[    5.380247] IP-Config: Got DHCP answer from 10.29.244.251, my address is 10.1
[    5.390461] IP-Config: Complete:
[    5.393731]      device=eth0, hwaddr=00:c0:08:88:0c:b5, ipaddr=10.29.244.61,4
[    5.404074]      host=10.29.244.61, domain=am.freescale.net, nis-domain=(non)
[    5.411362]      bootserver=0.0.0.0, rootserver=10.29.244.24, rootpath=     0
[    5.423964] ALSA device list:
[    5.426969]   No soundcards found.
[    5.469374] VFS: Mounted root (nfs filesystem) readonly on device 0:14.
[    5.478119] devtmpfs: mounted
[    5.482376] Freeing unused kernel memory: 456K (c0a7e000 - c0af0000)

starting pid 160, tty '': '/etc/rc.d/rcS'
Mounting /proc and /sys
Starting the hotplug events dispatcher udevd
Synthesizing initial hotplug even[ 6.085842] udevd (173): /proc/173/oom_adj .

,but the system hangs here.

If I boot it with the USB stick disconnected, then the system boots until the prompt, but the insertion of the USB stick is never detected afterwards.

With your patch applied, the error message (usb 1-1: device descriptor read/64, error -7) is shown with USB stick connected or disconnected during boot.

Thanks, Fabio, but I am curious how things like that? The USBOH3 clock is not opened, the usb driver will hang when it tries to access registers. If this clock is always on, then, why the system will hang later?

Hi Peter,

Quoted 4 lines · Peter Chen

Thanks, Fabio, but I am curious how things like that? The USBOH3 clock is not opened, the usb driver will hang when it tries to access registers. If this clock is always on, then, why the system will hang later?

I found the issue with your patch. You missed to add the pinctrl node.

With the change below USB is functional in Udoo:

Hi Fabio,
Hi Peter,

Quoted 17 lines · Fabio Estevam

Hi Peter,

Quoted 4 lines · Peter Chen

Thanks, Fabio, but I am curious how things like that? The USBOH3 clock is not opened, the usb driver will hang when it tries to access registers. If this clock is always on, then, why the system will hang later?

I found the issue with your patch. You missed to add the pinctrl node.

With the change below USB is functional in Udoo:

arch/arm/boot/dts/imx6qdl-udoo.dtsi line 29

Quoted 8 lines

arch/arm/boot/dts/imx6qdl-udoo.dtsi +2 -0

@@ -22,6 +22,8 @@
 
        usb_hub1 {
                compatible = "generic-onboard-hub";
+               pinctrl-names = "default";
+               pinctrl-0 = <&pinctrl_usbh>;
                clocks = <&clks IMX6QDL_CLK_CKO>;
                reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
                reset-duration-us = <2>;

Thanks for your work, I didn't notice it previously (sorry).

I can confirm, too that with Peter's patches and the above change the USB support works again on my UDOO DualLite board.

However, I noticed that when you have host USB support configured to be build as modules then (due to its location under "if USB") it is only possible to compile generic onboard USB HUB as module, too.

Then this module would need to be loaded before loading USB support (or quickly after it), otherwise USB enumeration would time out after few secs and loading it later wouldn't help.

Currently, this driver doesn't really need any USB host support and is able to be compiled-in successfully regardless of USB host support configuration with just small change to Makefile and Kconfig.

However I don't know if it is a design goal to not use USB host support or just a current development status, but if it doesn't really need it then it would be great if it could be selected to be build-in into kernel regardless of host USB support setting.

Quoted 39 lines · Maciej S. Szmigiero

Hi Fabio,
Hi Peter,

Quoted 17 lines · Fabio Estevam

Hi Peter,

Quoted 4 lines · Peter Chen

Thanks, Fabio, but I am curious how things like that? The USBOH3 clock is not opened, the usb driver will hang when it tries to access registers. If this clock is always on, then, why the system will hang later?

I found the issue with your patch. You missed to add the pinctrl node.

With the change below USB is functional in Udoo:

arch/arm/boot/dts/imx6qdl-udoo.dtsi line 29

Quoted 8 lines

arch/arm/boot/dts/imx6qdl-udoo.dtsi +2 -0

@@ -22,6 +22,8 @@
 
        usb_hub1 {
                compatible = "generic-onboard-hub";
+               pinctrl-names = "default";
+               pinctrl-0 = <&pinctrl_usbh>;
                clocks = <&clks IMX6QDL_CLK_CKO>;
                reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
                reset-duration-us = <2>;

Thanks for your work, I didn't notice it previously (sorry).

I can confirm, too that with Peter's patches and the above change the USB support works again on my UDOO DualLite board.

However, I noticed that when you have host USB support configured to be build as modules then (due to its location under "if USB") it is only possible to compile generic onboard USB HUB as module, too.

Quoted 3 lines

Then this module would need to be loaded before loading USB support (or quickly after it), otherwise USB enumeration would time out after few secs and loading it later wouldn't help.

Thanks for testing it, it maybe this hardware limitation. The USB device should be back to work whenever do hardware reset, otherwise, this reset is not clean.

Quoted 40 lines · Peter Chen

Quoted 39 lines · Maciej S. Szmigiero

Hi Fabio,
Hi Peter,

Quoted 17 lines · Fabio Estevam

Hi Peter,

Quoted 4 lines · Peter Chen

Thanks, Fabio, but I am curious how things like that? The USBOH3 clock is not opened, the usb driver will hang when it tries to access registers. If this clock is always on, then, why the system will hang later?

I found the issue with your patch. You missed to add the pinctrl node.

With the change below USB is functional in Udoo:

arch/arm/boot/dts/imx6qdl-udoo.dtsi line 29

Quoted 8 lines

arch/arm/boot/dts/imx6qdl-udoo.dtsi +2 -0

@@ -22,6 +22,8 @@
 
        usb_hub1 {
                compatible = "generic-onboard-hub";
+               pinctrl-names = "default";
+               pinctrl-0 = <&pinctrl_usbh>;
                clocks = <&clks IMX6QDL_CLK_CKO>;
                reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
                reset-duration-us = <2>;

Thanks for your work, I didn't notice it previously (sorry).

I can confirm, too that with Peter's patches and the above change the USB support works again on my UDOO DualLite board.

However, I noticed that when you have host USB support configured to be build as modules then (due to its location under "if USB") it is only possible to compile generic onboard USB HUB as module, too.

Quoted 7 lines

Quoted 3 lines

Then this module would need to be loaded before loading USB support (or quickly after it), otherwise USB enumeration would time out after few secs and loading it later wouldn't help.

Thanks for testing it, it maybe this hardware limitation. The USB device should be back to work whenever do hardware reset, otherwise, this reset is not clean.

I looked deeper into it and looks like the reset GPIO have to be explicitly configured as output and also the reset pulse duration have to be longer in order for hub the successfully reset and reattach to USB host (at least on the one UDOO board that I have).

The shortest working time seems to be 3000us - 2000us didn't work even when I changed GPIO pin mode to fast and drive strength to maximum.

Summing up, this change allows me to load generic_onboard_hub driver in any order with respect to USB host modules:

arch/arm/boot/dts/imx6qdl-udoo.dtsi +1 -1

@@ -26,7 +26,7 @@
                pinctrl-0 = <&pinctrl_usbh>;
                clocks = <&clks IMX6QDL_CLK_CKO>;
                reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
-               reset-duration-us = <2>;
+               reset-duration-us = <3000>;
        };
 };
 

drivers/usb/misc/generic_onboard_hub.c +1 -1

@@ -89,7 +89,7 @@ static int usb_hub_generic_probe(struct platform_device *pdev)
        of_property_read_u32(node, "reset-duration-us", &duration_us);
 
        if (gpiod_reset) {
-               gpiod_set_value(gpiod_reset, 1);
+               gpiod_direction_output(gpiod_reset, 1);
                usleep_range(duration_us, duration_us + 10);
                gpiod_set_value(gpiod_reset, 0);
        }

Best regards,
Maciej Szmigiero

--