From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Chen Subject: Re: [PATCH V2 0/1] usb: add HCD providers Date: Thu, 14 Jul 2016 17:48:36 +0800 Message-ID: <20160714094836.GB28730@shlinux2> References: <1468326921-26485-1-git-send-email-zajec5@gmail.com> <1468413734-9569-1-git-send-email-zajec5@gmail.com> <87lh15isi7.fsf@linux.intel.com> <87inw9ir4k.fsf@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-pf0-f170.google.com ([209.85.192.170]:35381 "EHLO mail-pf0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751058AbcGNJ4Y (ORCPT ); Thu, 14 Jul 2016 05:56:24 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-leds-owner@vger.kernel.org List-Id: linux-leds@vger.kernel.org To: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: Felipe Balbi , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "open list:LED SUBSYSTEM" , "devicetree@vger.kernel.org" On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafa=C5=82 Mi=C5=82ecki wrote= : > On 13 July 2016 at 15:50, Felipe Balbi = wrote: > > Rafa=C5=82 Mi=C5=82ecki writes: > >> On 13 July 2016 at 15:20, Felipe Balbi wrote: > >>> Rafa=C5=82 Mi=C5=82ecki writes: > >>>> Hi again, > >>>> > >>>> This is my second try of getting HCD providers into usb subsyste= m. > >>>> > >>>> During discussion of V1 I realized there are about 26 drivers ad= ding a > >>>> single HCD and all of them would need to be modified. So instead= I > >>>> decided to put relevant code in usb_add_hcd. It checks if the HC= D we > >>>> register is a primary one and if so, it registers a proper provi= der. > >>>> > >>>> Please note that of_hcd_xlate_simple was also extended to allow = getting > >>>> shared HCD (which is used e.g. in case of XHCI). > >>>> > >>>> So now you can have something like: > >>>> > >>>> ohci: ohci@21000 { > >>>> #usb-cells =3D <0>; > >>>> compatible =3D "generic-ohci"; > >>>> reg =3D <0x00001000 0x1000>; > >>>> interrupts =3D ; > >>>> }; > >>>> > >>>> ehci: ehci@22000 { > >>>> #usb-cells =3D <0>; > >>>> compatible =3D "generic-ehci"; > >>>> reg =3D <0x00002000 0x1000>; > >>>> interrupts =3D ; > >>>> }; > >>>> > >>>> xhci: xhci@23000 { > >>>> #usb-cells =3D <1>; > >>>> compatible =3D "generic-xhci"; > >>>> reg =3D <0x00003000 0x1000>; > >>>> interrupts =3D ; > >>>> }; > >>>> > >>>> The last (second) patch is not supposed to be applied, it's used= only as > >>>> a proof and example of how providers can be used. > >>> > >>> nowhere here (or in previous patch) you clarify why exactly you n= eed > >>> this. What is your LED trigger supposed to do? Why can't it handl= e ports > >>> changing number in different boots? Why do we need this at all? W= hy is > >>> your code DT-specific? > >>> > >>> There are still too many 'unknowns' here. > >> > >> Are you sure you saw my reply to Peter's question? > >> > >> http://www.spinics.net/lists/linux-usb/msg143708.html > >> http://marc.info/?l=3Dlinux-usb&m=3D146838735627093&w=3D2 > >> > >> I think it should answer (some of?) your questions. Can you read i= t > >> and see if it gets a bit clearer? > > > > well, all that says is that you're writing a LED trigger to toggle = LED > > when a USB device gets added to a specified port. I don't think you= need > > the actual port number for that. You should have a phandle to the a= ctual > > port, whatever its number is, or a phandle to the (root-)Hub and a = port > > number from that hub. > > > > The problem, really, is that DT descriptor of USB Hosts is very, ve= ry > > minimal. Perhaps there's something more extensively defined from th= e > > original Open Firmware USB Addendum. >=20 > 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). >=20 > Having each port described by DT would be helpful, it's just somethin= g > I didn't find implemented, so I started looking for different ways. I= t > seems I should have picked a different solution. >=20 > 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 guidanc= e > first. What do you think about following schema/example? >=20 > ohci@1000 { > compatible =3D "generic-ohci"; > reg =3D <0x00001000 0x1000>; > interrupts =3D ; >=20 > primary-hcd { > ohci_port0: port@0 { > reg =3D <0>; > }; >=20 > ohci_port1: port@1 { > reg =3D <1>; > }; > } > }; >=20 > ehci@2000 { > compatible =3D "generic-ehci"; > reg =3D <0x00002000 0x1000>; > interrupts =3D ; >=20 > primary-hcd { > ehci_port0: port@0 { > reg =3D <0>; > }; >=20 > ehci_port1: port@1 { > reg =3D <1>; > }; > } > }; >=20 > xhci@3000 { > compatible =3D "generic-xhci"; > reg =3D <0x00003000 0x1000>; > interrupts =3D ; >=20 > primary-hcd { > }; >=20 > shared-hcd { > xhci_port0: port@0 { > reg =3D <0>; > }; > } > }; >=20 > With such a DT struct, how could I query port for a Linux-assigned nu= mber? >=20 > 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 >=20 > 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 >=20 > 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? >=20 =46or 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 =3D &ohci_port0, &ehci_port1; } led_2 { led_gpio_2; usb_port =3D &xhci_port0, &xhci_port1; } ohci@1000 { compatible =3D "generic-ohci"; reg =3D <0x00001000 0x1000>; interrupts =3D ; ohci_port0: port@0 { reg =3D <0>; }; ohci_port1: port@1 { reg =3D <1>; }; }; ehci@2000 { compatible =3D "generic-ehci"; reg =3D <0x00002000 0x1000>; interrupts =3D ; ehci_port0: port@0 { reg =3D <0>; }; ehci_port1: port@1 { reg =3D <1>; }; }; xhci@3000 { compatible =3D "generic-xhci"; reg =3D <0x00003000 0x1000>; interrupts =3D ; /* 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 =3D <0>; }; xhci_port1: port@1 { reg =3D <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. --=20 Best Regards, Peter Chen