On 15 July 2016 at 08:22, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> On Fri, Jul 15, 2016 at 07:48:11AM +0200, Rafał Miłecki wrote:
>> >> > 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?
>>
>
> You may need below patches:
>
> commit 69bec725985324e79b1c47ea287815ac4ddb0521
> Author: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> Date: Fri Feb 19 17:26:15 2016 +0800
>
> USB: core: let USB device know device node
>
> commit 7222c832254a75dcd67d683df75753d4a4e125bb
> Author: Nicolai Stange <nicstange@xxxxxxxxx>
> Date: Thu Mar 17 23:53:02 2016 +0100
>
> usb/core: usb_alloc_dev(): fix setting of ->portnum
F*k, I just implemented the same thing on my own and I was going to
submit it :/ Thanks for pointing these commits.
>> >> 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.
>>
>
> Oh, I see, I asked it before.
>
> Either you need to register USB notifier before activation
It won't work if someone builds usbport as a module and loads it after
connecting USB devices.
> Or you need to implement something like usb_node_to_dev
> eg: like usb_for_each_dev. If device's state is USB_STATE_CONFIGURED
> this USB device is available.
I think I'll need that.
>> >> 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
>>
>
> I know your patch, how you plan to support non-DT platforms before?
I didn't have any better idea than just letting user space do some
magic. I don't think we can develop any clever solution for non-DT but
I still want to give these platforms some options. I think I can live
with some more tricky code in user space.
>> >> 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.
>
> Why? You know your hardware design, you should know the topology, eg
> This physical port is from ehci port, and which port number for its
> internal hub port number. If not, how we describe USB devices on
> dts.
Please ignore that question.
--
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
