| Message ID | CAL411-qKE=12VZAN9=tUWPqkC5BfSgpQb9aNgCU6rmPLasXNHQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 21 Dec 2015, Peter Chen wrote: > 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: > > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi > @@ -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 = <®_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) That seems reasonable to me. > 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. 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. Alan Stern > 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.
On Mon, Dec 21, 2015 at 2:33 AM, Peter Chen <hzpeterchen@gmail.com> wrote: > On Fri, Dec 18, 2015 at 11:38 PM, Alan Stern <stern@rowland.harvard.edu> wrote: >> On Fri, 18 Dec 2015, Peter Chen wrote: >> >>> On Fri, Dec 18, 2015 at 12:13 AM, Alan Stern <stern@rowland.harvard.edu> wrote: >> >>> > 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. 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. [...] >>> +static int hub_of_children_register(struct usb_device *hdev) >>> +{ >>> + struct device *dev; >>> + >>> + if (hdev->parent) >>> + dev = &hdev->dev; >>> + 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. That should be a solvable problem... > 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: > > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi > @@ -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 = <®_usb_h1_vbus>; > status = "okay"; > + > + devices-pre-operation = <&usbh1_pre_operation> > }; No. The binding should reflect the hardware connections. The hub is a child of the USB controller/root hub. so the hub node had better be a child of the controller in the DT. The clocks and reset are connections to the hub, so they had better be in the hub's DT node. There is really nothing more to discuss on the binding. Anything else you are coming up with is working around kernel issues. > 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. 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. Rob
On Tue, 5 Jan 2016, Rob Herring wrote: > >>> > 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. > > 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. That would be okay with me. It would make things a lot simpler (but it would also waste energy in situations where the devices weren't being used). Alan Stern
On Tue, Jan 05, 2016 at 08:36:31AM -0600, Rob Herring wrote: > > 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)? 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. > > 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.
On Tue, Jan 5, 2016 at 9:20 PM, Peter Chen <hzpeterchen@gmail.com> wrote: > On Tue, Jan 05, 2016 at 08:36:31AM -0600, Rob Herring wrote: >> > 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)? > > 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. >> 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. Surprising, no one ever seems to agree with that solution... There will be people with this problem and unable to update their bootloader. > 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
On Thu, Jan 07, 2016 at 08:18:02AM -0600, Rob Herring wrote: > On Tue, Jan 5, 2016 at 9:20 PM, Peter Chen <hzpeterchen@gmail.com> wrote: > > On Tue, Jan 05, 2016 at 08:36:31AM -0600, Rob Herring wrote: > >> > 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)? > > > > 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. If it is hot-plug, there will be any USB devices at port, and it even has a USB HUB between controller and the device we want to describe. > > >> 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. > > Surprising, no one ever seems to agree with that solution... There > will be people with this problem and unable to update their > bootloader. > See: http://www.spinics.net/lists/linux-usb/msg134788.html I am a little puzzled what's your opinion for this problem. > > 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 I will send RFC patch for let the USB device know device node, but it is not for my issue.
On Thu, Jan 07, 2016 at 08:18:02AM -0600, Rob Herring wrote: > On Tue, Jan 5, 2016 at 9:20 PM, Peter Chen <hzpeterchen@gmail.com> wrote: > > On Tue, Jan 05, 2016 at 08:36:31AM -0600, Rob Herring wrote: > >> > 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)? > > > > 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. > > >> 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. > > Surprising, no one ever seems to agree with that solution... There > will be people with this problem and unable to update their > bootloader. > Hi Rob, I noticed there are still some platform needs to this solution, I would like to see if I can move this on, please review below dts solution. In below case, there are two ports on the root hub, one is USB HUB, the another is a USB wifi. Both of these devices need to have power control before they can work. At USB hcd driver, it will try to look-up all USB devices on this controller, if the usb-pwr-ops is existed, it will try to get its phandle, and do gpio and clock operations. Optional properties: - usb-pwr-ops: the power operations which need to do before this USB device can work. Example: / { ... hub_genesys_1_pwr_ops: hub_genesys_1_pwr_ops { compatible = "usb-pwrseq-simple"; reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */ reset-duration-us = <10>; clocks = <&clks IMX6QDL_CLK_CKO>; }; wifi_nxp_2_pwr_ops: wifi_nxp_2_pwr_ops { compatible = "usb-pwrseq-simple"; reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>, /* wifi_rst */ <&gpio4 7 GPIO_ACTIVE_LOW>; /* wifi_en */ reset-duration-us = <10>; clocks = <&clks IMX6QDL_CLK_CKO1>; }; ... }; &usb1 { status = "okay"; #address-cells = <1>; #size-cells = <0>; hub: genesys@1 { compatible = "usb5e3,608"; reg = <1>; usb-pwr-ops = <&hub_genesys_1_pwr_ops> }; wifi: nxp@2 { compatible = "usb5e3,608"; reg = <2>; usb-pwr-ops = <&wifi_nxp_2_pwr_ops> }; }
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi @@ -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 = <®_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