Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
Hi, On Thu, Jul 25, 2013 at 02:10:46AM +0400, Sergei Shtylyov wrote: + 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 asked. usb-xhci is too generic, so is xhci-platform. I rather have: compatible = snps,xhci if this is a synopsys IP -- balbi signature.asc Description: Digital signature
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
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 000..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
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
On 25.07.2013 18:39, Alan Cooper 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 000..b88aee7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -0,0 +1,14 @@
[...]
+
+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@?
Unfortunately, they are all wrong, as many others. It seems almost nobody
reads:
http://www.devicetree.org/Device_Tree_Usage
+ 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.
xhci-pci gets identified by the PCI device class, not the name. Maybe
indeed snps,xhci is better if this is Synopsys core you're doing the binding
for... I have no strong preference, it's just that xhci-platform doesn't
appeal to me.
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
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
On Thu, Jul 25, 2013 at 1:10 PM, Sergei Shtylyov sergei.shtyl...@cogentembedded.com wrote: 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@? Unfortunately, they are all wrong, as many others. It seems almost nobody reads: http://www.devicetree.org/Device_Tree_Usage Can you point me to the section that indicates ohci/ehci/xhci are incorrect? http://devicetree.org/Device_Tree_Usage#Node_Names See also section 2.2.2 in the ePAPR spec. WBR, Sergei I read http://devicetree.org/Device_Tree_Usage#Node_Names to say not to use vendor specific names and xhci is not vendor specific. ePAPR states: The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model. And while xhci does describe the programming interface, it also describes the functionality and fits in better with the already established ehci and ohci node names. Al -- 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
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
On 07/25/2013 11:51 PM, Alan Cooper wrote: 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@? Unfortunately, they are all wrong, as many others. It seems almost nobody reads: http://www.devicetree.org/Device_Tree_Usage Can you point me to the section that indicates ohci/ehci/xhci are incorrect? http://devicetree.org/Device_Tree_Usage#Node_Names See also section 2.2.2 in the ePAPR spec. WBR, Sergei I read http://devicetree.org/Device_Tree_Usage#Node_Names to say not to use vendor specific names and xhci is not vendor specific. ePAPR states: The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model. And while xhci does describe the programming interface, it also describes the functionality No, actually, it says nothing to a person not familiar with the xHCI spec. There's for example also AHCI which is not a USB controller but SATA one. and fits in better with the already established ehci and ohci node names. These are not yet set in stone (see e.g. the ongoing discussion on linux-arm-kernel about the broken bindings, calling up some volunteers to clean them up). Moreover, ePAPR further lists the appropriate device names, usb amongst them. Al 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
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
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.
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 000..b88aee7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -0,0 +1,14 @@
+USB XHCI controllers
+
+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 {
+ compatible = xhci-platform;
+ 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
@@ -14,6 +14,7 @@
#include linux/platform_device.h
#include linux/module.h
#include linux/slab.h
+#include linux/of.h
#include xhci.h
@@ -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 },
+ {},
+};
+MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
+#endif
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.8.1.3
--
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
Re: [PATCH V4] usb: Add Device Tree support to XHCI Platform driver
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 000..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.
+
+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].
+ 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.
+ 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.
[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
