On 15 July 2016 at 04:28, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> On Thu, Jul 14, 2016 at 05:52:43PM +0200, Rafał Miłecki wrote:
>> On 14 July 2016 at 11:48, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
>> > On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote:
>> >> Thanks for your effort and looking at this closely. You're right, I'm
>> >> interested in referencing USB ports, but I'm using controller phandle
>> >> (and then I specify ports manually).
>> >>
>> >> Having each port described by DT would be helpful, it's just something
>> >> I didn't find implemented, so I started looking for different ways. It
>> >> seems I should have picked a different solution.
>> >>
>> >> So should I work on describing USB ports in DT instead? This looks
>> >> like a complex thing to describe, so I'd like to ask for some guidance
>> >> first. What do you think about following schema/example?
>> >>
>> >> ohci@1000 {
>> >> compatible = "generic-ohci";
>> >> reg = <0x00001000 0x1000>;
>> >> interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
>> >>
>> >> primary-hcd {
>> >> ohci_port0: port@0 {
>> >> reg = <0>;
>> >> };
>> >>
>> >> ohci_port1: port@1 {
>> >> reg = <1>;
>> >> };
>> >> }
>> >> };
>> >>
>> >> ehci@2000 {
>> >> compatible = "generic-ehci";
>> >> reg = <0x00002000 0x1000>;
>> >> interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>> >>
>> >> primary-hcd {
>> >> ehci_port0: port@0 {
>> >> reg = <0>;
>> >> };
>> >>
>> >> ehci_port1: port@1 {
>> >> reg = <1>;
>> >> };
>> >> }
>> >> };
>> >>
>> >> xhci@3000 {
>> >> compatible = "generic-xhci";
>> >> reg = <0x00003000 0x1000>;
>> >> interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>> >>
>> >> primary-hcd {
>> >> };
>> >>
>> >> shared-hcd {
>> >> xhci_port0: port@0 {
>> >> reg = <0>;
>> >> };
>> >> }
>> >> };
>> >>
>> >> With such a DT struct, how could I query port for a Linux-assigned number?
>> >>
>> >> For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns
>> >> number 4 to my XHCI's shared HCD's root hub:
>> >> xhci-hcd 18023000.xhci: xHCI Host Controller
>> >> xhci-hcd 18023000.xhci: new USB bus registered, assigned bus number 4
>> >> hub 4-0:1.0: USB hub found
>> >> hub 4-0:1.0: 1 port detected
>> >>
>> >> If I disable OHCI and EHCI I get:
>> >> xhci-hcd xhci-hcd.0: xHCI Host Controller
>> >> xhci-hcd xhci-hcd.0: new USB bus registered, assigned bus number 2
>> >> hub 2-0:1.0: USB hub found
>> >> hub 2-0:1.0: 1 port detected
>> >>
>> >> So I need my "usbport" trigger driver to be able to get "4-1" in the
>> >> first case and "2-1" in the second case. I guess I should use
>> >> &xhci_port0 but what then? How could I translate it into
>> >> Linux-assigned numbering?
>> >>
>> >
>> > For your current design, you need to fix shared hcd for xHCI problem,
>> > since xHCI has two buses.
>> >
>> > Below I supply another thought, please check if it is feasible.
>> > In below design, you don't need to change any usb codes.
>> >
>> > dts:
>> >
>> > led_1 {
>> > led_gpio_1;
>> > usb_port = &ohci_port0, &ehci_port1;
>> > }
>> >
>> > led_2 {
>> > led_gpio_2;
>> > usb_port = &xhci_port0, &xhci_port1;
>> > }
>> >
>> > ohci@1000 {
>> > compatible = "generic-ohci";
>> > reg = <0x00001000 0x1000>;
>> > interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
>> >
>> > ohci_port0: port@0 {
>> > reg = <0>;
>> > };
>> >
>> > ohci_port1: port@1 {
>> > reg = <1>;
>> > };
>> > };
>> >
>> > ehci@2000 {
>> > compatible = "generic-ehci";
>> > reg = <0x00002000 0x1000>;
>> > interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>> >
>> > ehci_port0: port@0 {
>> > reg = <0>;
>> > };
>> >
>> > ehci_port1: port@1 {
>> > reg = <1>;
>> > };
>> > };
>> >
>> > xhci@3000 {
>> > compatible = "generic-xhci";
>> > reg = <0x00003000 0x1000>;
>> > interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>> >
>> > /* for xhci, port 0 - [N-1] is USB3, N - [M-1] is USB2/1.
>> > * The port 0 and port N is the same physical port
>> > */
>> > xhci_port0: port@0 {
>> > reg = <0>;
>> > };
>> >
>> > xhci_port1: port@1 {
>> > reg = <1>;
>> > };
>> >
>> > };
>> >
>> > At code, compare the usb_device's device_node at usbport_trig_notify
>> > if it is at led_1's usb device list, light on it.
>>
>> This is quite interesting idea, thanks!
>>
>> So I got following checking code:
>>
>> count = of_count_phandle_with_args(np, "usb-ports", NULL);
>> for (i = 0; i < count; i++) {
>> of_parse_phandle_with_args(np, "usb-ports", NULL, i, &args);
>> of_property_read_u32(args.np, "reg", &port);
>> if (args.np->parent == usb_dev->bus->controller->of_node &&
>> port == usb_dev->portnum) {
>> of_node_put(args.np);
>> return true;
>> }
>> of_node_put(args.np);
>> }
>> return false;
>
> No, compares the USB port directly.
>
> count = of_count_phandle_with_args(np, "usb-ports", NULL);
> for (i = 0; i < count; i++) {
> of_parse_phandle_with_args(np, "usb-ports", NULL, i, &args);
> if (args.np == usb_dev->dev.of_node)
> of_node_put(args.np);
> return true;
> }
> of_node_put(args.np);
> }
> return false;
If we mean to use usb_dev->dev.of_node I *need* to modify USB
subsystem, since this pointer is never being set by the current code.
[ 71.410505] usb 1-1: new high-speed USB device number 2 using ehci-platform
[ 71.564928] [usbport_trig_notify] usb_dev:c6ac1000 &usb_dev->dev:c6ac1068
[ 71.579874] [usbport_trig_notify] dev_name(&usb_dev->dev):1-1
[ 71.586580] [usbport_trig_notify] usb_dev->dev.of_node: (null)
Or am I missing something?
>> This works, but I see 3 more problems:
>>
>> 1) How to access list of available USB devices during activation?
>
> You mean during LED activation? eg your usbport_trig_activate?
> Why do you need it?
Yes, I mean usbport_trig_activate. If user plugs in USB device and
*then* activates this trigger, we want to set a proper initial state.
We can't only depend on USB_DEVICE_ADD.
>> 2) What about support for non-DT platforms in usbport driver? Should I
>> still allow specifying ports manually? Are you OK with that?
>
> I am afraid I still don't know how to do it for non-DT platforms.
> You can show your design.
Please take a look at
[PATCH] leds: trigger: Introduce an USB port trigger
https://lkml.org/lkml/2016/7/11/305
Basically my idea was to support:
echo usbport > trigger
echo 4-1 > new_port
echo 2-1 > new_port
>> 3) What about devices with internal hubs? Should we describe their USB
>> ports in DT as well? Any idea how to do this?
>
> Well, the HUB must be hard-wired on board for this LED trigger case.
> So, you can described USB topology well at dts. Please note: the
> USB port phandle at LED node is the physical connector on board which
> the user can plug in USB device, it may be 2nd or more levels from the
> controller.
>
> Below is the example for how to describe 3 levels USB devices, the
> USB ethernet port (axis) is one of the ports at internal HUB (genesys).
>
> http://www.spinics.net/lists/linux-usb/msg143698.html
Thanks for this example. I'm only afraid there isn't a way to say what
port "ethernet: asix@1" is attached to. Would that make sense to add
one more property to the "ethernet: asix@1", e.g.
usb-port: <&ehci_port1>;
(assuming there would be ehci_port1 in your DTS)?
--
Rafał
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
