| Message ID | 1403427899-32154-2-git-send-email-robert.jarzmik@free.fr |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sun, Jun 22, 2014 at 10:04:58AM +0100, Robert Jarzmik wrote: > Add documentation for device-tree binding of arm PXA 27x udc (usb > device) driver. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > Cc: devicetree@vger.kernel.org > > --- > Since V1: change OF id mrvl,pxa27x_udc -> marvell,pxa27x-udc > This is a consequence of other DT reviews on the marvell > namings. > --- > Documentation/devicetree/bindings/usb/pxa-usb.txt | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/pxa-usb.txt b/Documentation/devicetree/bindings/usb/pxa-usb.txt > index 79729a9..0e4863f 100644 > --- a/Documentation/devicetree/bindings/usb/pxa-usb.txt > +++ b/Documentation/devicetree/bindings/usb/pxa-usb.txt > @@ -29,3 +29,23 @@ Example: > marvell,port-mode = <2>; /* PMM_GLOBAL_MODE */ > }; > > +UDC > + > +Required properties: > + - compatible: Should be "marvell,pxa27x-udc" for USB controllers > + used in device mode. We typically don't like wildcard compatible strings in DT. > + > +Optional properties: > + - "udc-pullup-gpio" - gpio activated to control the USB D+ pullup. > + - "udc-pullup-gpio-inverted" - boolean inverting gpio pullup logic. Use the GPIO bindings. > + > +Example: > + > + pxa27x_udc: udc@40600000 { > + compatible = "marvell,pxa27x-udc"; > + reg = <0x40600000 0x10000>; > + interrupts = <11>; > + clocks = <&pxa2xx_clks 11>; > + clock-names = "pxa27x-udc"; Neither of these were mentioned above. The name of the clock input doesn't make sense. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Rutland <mark.rutland@arm.com> writes: > On Sun, Jun 22, 2014 at 10:04:58AM +0100, Robert Jarzmik wrote: >> index 79729a9..0e4863f 100644 >> --- a/Documentation/devicetree/bindings/usb/pxa-usb.txt >> +++ b/Documentation/devicetree/bindings/usb/pxa-usb.txt >> @@ -29,3 +29,23 @@ Example: >> marvell,port-mode = <2>; /* PMM_GLOBAL_MODE */ >> }; >> >> +UDC >> + >> +Required properties: >> + - compatible: Should be "marvell,pxa27x-udc" for USB controllers >> + used in device mode. > > We typically don't like wildcard compatible strings in DT. Same as in the other patch, "marvell,pxa270-udc". > >> + >> +Optional properties: >> + - "udc-pullup-gpio" - gpio activated to control the USB D+ pullup. >> + - "udc-pullup-gpio-inverted" - boolean inverting gpio pullup logic. > > Use the GPIO bindings. OK. I'm presuming in this case you think of something like : udc-pullup-gpio = <&gpio 22 GPIO_ACTIVE_LOW> Which translates into: "udc-pullup-gpio" - gpio activated to control the USB D+ pullup (see gpio.txt) > >> + >> +Example: >> + >> + pxa27x_udc: udc@40600000 { >> + compatible = "marvell,pxa27x-udc"; >> + reg = <0x40600000 0x10000>; >> + interrupts = <11>; >> + clocks = <&pxa2xx_clks 11>; >> + clock-names = "pxa27x-udc"; > > Neither of these were mentioned above. Ah you mean I shall describe the standard reg, interrupts as mandatory standard options I take it. OK. > > The name of the clock input doesn't make sense. I don't understand. With [1] does it make any more sense ? If not you'll have to expand a bit more the "doesn't make sense". Cheers. -- Robert [1] http://www.spinics.net/lists/arm-kernel/msg337522.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 25, 2014 at 08:54:01PM +0100, Robert Jarzmik wrote: > Mark Rutland <mark.rutland@arm.com> writes: > > > On Sun, Jun 22, 2014 at 10:04:58AM +0100, Robert Jarzmik wrote: > >> index 79729a9..0e4863f 100644 > >> --- a/Documentation/devicetree/bindings/usb/pxa-usb.txt > >> +++ b/Documentation/devicetree/bindings/usb/pxa-usb.txt > >> @@ -29,3 +29,23 @@ Example: > >> marvell,port-mode = <2>; /* PMM_GLOBAL_MODE */ > >> }; > >> > >> +UDC > >> + > >> +Required properties: > >> + - compatible: Should be "marvell,pxa27x-udc" for USB controllers > >> + used in device mode. > > > > We typically don't like wildcard compatible strings in DT. > Same as in the other patch, "marvell,pxa270-udc". > > > >> + > >> +Optional properties: > >> + - "udc-pullup-gpio" - gpio activated to control the USB D+ pullup. > >> + - "udc-pullup-gpio-inverted" - boolean inverting gpio pullup logic. > > > > Use the GPIO bindings. > OK. I'm presuming in this case you think of something like : > udc-pullup-gpio = <&gpio 22 GPIO_ACTIVE_LOW> > Which translates into: > "udc-pullup-gpio" - gpio activated to control the USB D+ pullup (see > gpio.txt) Something like that, yes. > > > >> + > >> +Example: > >> + > >> + pxa27x_udc: udc@40600000 { > >> + compatible = "marvell,pxa27x-udc"; > >> + reg = <0x40600000 0x10000>; > >> + interrupts = <11>; > >> + clocks = <&pxa2xx_clks 11>; > >> + clock-names = "pxa27x-udc"; > > > > Neither of these were mentioned above. > Ah you mean I shall describe the standard reg, interrupts as mandatory standard > options I take it. OK. Yes. While the properties are standard, their precise meanings (e.g. which interrupt or address space region), and whether a particular binding uses them is not. > > The name of the clock input doesn't make sense. > I don't understand. With [1] does it make any more sense ? If not you'll have to > expand a bit more the "doesn't make sense". My concern is that clock-names is supposed to describe the name of the input clock line from the view of the IP block. "pxa27x-udc" doesn't sound like the name of a clock input line from the view of the UDC block. I assume the clock input line you care about has a more specific name than "pxa27x-udc"? Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Rutland <mark.rutland@arm.com> writes: > On Wed, Jun 25, 2014 at 08:54:01PM +0100, Robert Jarzmik wrote: >> > The name of the clock input doesn't make sense. >> I don't understand. With [1] does it make any more sense ? If not you'll have to >> expand a bit more the "doesn't make sense". > > My concern is that clock-names is supposed to describe the name of the > input clock line from the view of the IP block. "pxa27x-udc" doesn't > sound like the name of a clock input line from the view of the UDC > block. > > I assume the clock input line you care about has a more specific name > than "pxa27x-udc"? Not as far as I know. The technical reference manual call it "udc clock", so it's even "less" specific ... Marvell engineers have probably the internal schematics and the name of the clock, but outsiders like me only have "udc" ... Cheers.
On Sun, Jun 29, 2014 at 10:29:49AM +0100, Robert Jarzmik wrote: > Mark Rutland <mark.rutland@arm.com> writes: > > > On Wed, Jun 25, 2014 at 08:54:01PM +0100, Robert Jarzmik wrote: > >> > The name of the clock input doesn't make sense. > >> I don't understand. With [1] does it make any more sense ? If not you'll have to > >> expand a bit more the "doesn't make sense". > > > > My concern is that clock-names is supposed to describe the name of the > > input clock line from the view of the IP block. "pxa27x-udc" doesn't > > sound like the name of a clock input line from the view of the UDC > > block. > > > > I assume the clock input line you care about has a more specific name > > than "pxa27x-udc"? > Not as far as I know. The technical reference manual call it "udc clock", so > it's even "less" specific ... Not from the point of view of the device. The clock-names are namespaced to the particular binding, so they're equally specific. Given the above I'd recommend naming the clock "udc" or "udc_clk". That doesn't pretend to be overly specific, and matches the TRM. > Marvell engineers have probably the internal schematics and the name of the > clock, but outsiders like me only have "udc" ... Sure, not having the full specs is always a pain. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/usb/pxa-usb.txt b/Documentation/devicetree/bindings/usb/pxa-usb.txt index 79729a9..0e4863f 100644 --- a/Documentation/devicetree/bindings/usb/pxa-usb.txt +++ b/Documentation/devicetree/bindings/usb/pxa-usb.txt @@ -29,3 +29,23 @@ Example: marvell,port-mode = <2>; /* PMM_GLOBAL_MODE */ }; +UDC + +Required properties: + - compatible: Should be "marvell,pxa27x-udc" for USB controllers + used in device mode. + +Optional properties: + - "udc-pullup-gpio" - gpio activated to control the USB D+ pullup. + - "udc-pullup-gpio-inverted" - boolean inverting gpio pullup logic. + +Example: + + pxa27x_udc: udc@40600000 { + compatible = "marvell,pxa27x-udc"; + reg = <0x40600000 0x10000>; + interrupts = <11>; + clocks = <&pxa2xx_clks 11>; + clock-names = "pxa27x-udc"; + udc-pullup-gpio = <22>; + };
Add documentation for device-tree binding of arm PXA 27x udc (usb device) driver. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Cc: devicetree@vger.kernel.org --- Since V1: change OF id mrvl,pxa27x_udc -> marvell,pxa27x-udc This is a consequence of other DT reviews on the marvell namings. --- Documentation/devicetree/bindings/usb/pxa-usb.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)