On Wed, Jul 24, 2013 at 6:10 PM, Sergei Shtylyov
<sergei.shtyl...@cogentembedded.com> wrote:
> Hello.
>
>
> On 07/25/2013 01:21 AM, Sarah Sharp wrote:
>
>> It looks like all the feedback has been addressed, but I'm no device
>> tree expert.
>
>
>> Felipe, Matthijs, and Sergei, does this look good? If so, I'll queue to
>> my xhci tree.
>
>
> Not quite there yet. Too bad I couldn't notice all the small issues at
> once...
>
>
>> Sarah Sharp
>
>
>> On Tue, Jul 23, 2013 at 06:35:33PM -0400, Al Cooper wrote:
>>>
>>> Add Device Tree match table to xhci-plat.c. Add DT bindings document.
>
>
>>> Signed-off-by: Al Cooper <alcoop...@gmail.com>
>>> ---
>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 14 ++++++++++++++
>>> drivers/usb/host/xhci-plat.c | 10 ++++++++++
>>> 2 files changed, 24 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt
>
>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> new file mode 100644
>>> index 0000000..b88aee7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> @@ -0,0 +1,14 @@
>>> +USB XHCI controllers
>
>
> I think the proper name is xHCI.
OK
>
>
>>> +
>>> +Required properties:
>>> + - compatible: should be "xhci-platform".
>>> + - reg: should contain address and length of the standard XHCI
>>> + register set for the device.
>>> + - interrupts: one XHCI interrupt should be described here.
>>> +
>>> +Example:
>>> + xhci@f0931000 {
>
>
> The node should be named just "usb", not "xhci" (no programming interface
> specific names), according to the ePAPR spec [1].
>
What about the existing node names "ohci@" and "ehci@"?
>>> + compatible = "xhci-platform";
>
>
> It again looks somewhat like a driver name, not a device name. What made
> you change the value from "usb-xhci", Al? Look at [eo]hci-omap.txt in that
> directory.
I changed the name because MODULE_DEVICE_TABLE() now uses the name and
that means the hotplug system will use it to identify the driver and
it seems like the
name should be unique enough to avoid confusion with something like
the xhci-pci driver.
>
>
>>> + reg = <0xf0931000 0x8c8>;
>>> + interrupts = <0x0 0x4e 0x0>;
>>> + };
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index d718134..d547f24 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>
> [...]
>
>>> @@ -212,11 +213,20 @@ static int xhci_plat_remove(struct platform_device
>>> *dev)
>>> return 0;
>>> }
>>>
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id usb_xhci_of_match[] = {
>>> + { .compatible = "xhci-platform" },
>>> + {},
>
>
> Consistency asks for a space inside {}. AFAIK, that's the usual Linux
> style.
>
OK
> [1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
>
> WBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
