diff mbox

[PATCH_V4,1/3] dt: usb: jz4740: Add DT binding document for OHCI

Message ID 1422958640-25723-2-git-send-email-Zubair.Kakakhel@imgtec.com
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Zubair Lutfullah Kakakhel Feb. 3, 2015, 10:17 a.m. UTC
From: Paul Burton <paul.burton@imgtec.com>

Add the binding documentation for the JZ4740 OHCI controller.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
The jz4740 is platform only at the moment.

But DT support is being added

See http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/

V4 Changes
Removed clock binding because of pending work in clock tree. Will add
binding later. Rather than introduce a bad binding now and change later.

V3 Changes: named binding jz4740 instead of confusing it with jz47xx.
The driver name is jz4740. And will not be renamed

V2 Changes: Removed interrupt parent binding as that can be inherited.
Forgot a binding for clock-names
---
 .../devicetree/bindings/usb/ingenic,jz4740-ohci.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt

Comments

Lars-Peter Clausen Feb. 3, 2015, 10:32 a.m. UTC | #1
On 02/03/2015 11:17 AM, Zubair Lutfullah Kakakhel wrote:
[...]
> V4 Changes
> Removed clock binding because of pending work in clock tree. Will add
> binding later. Rather than introduce a bad binding now and change later.

But this patch is introducing a bad binding. The part needs the clock to 
work. The binding you are specifying right now says that it works fine 
without any clocks.

 > [...]
> +Example for jz4740:
> +
> +/ {
> +	ohci: jz4780-ohci@0x134a0000 {

s/jz4780/jz4740

> +		compatible = "ingenic,jz4740-ohci";
> +		reg = <0x134a0000 0x10000>;
> +
> +		interrupts = <5>;
> +	};
> +};
> +
>

--
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
Zubair Lutfullah Kakakhel Feb. 3, 2015, 10:53 a.m. UTC | #2
On 03/02/15 10:32, Lars-Peter Clausen wrote:
> On 02/03/2015 11:17 AM, Zubair Lutfullah Kakakhel wrote:
> [...]
>> V4 Changes
>> Removed clock binding because of pending work in clock tree. Will add
>> binding later. Rather than introduce a bad binding now and change later.
> 
> But this patch is introducing a bad binding. The part needs the clock to work. The binding you are specifying right now says that it works fine without any clocks.

Facing a chicken and egg problem with these patches here..

When the new clock driver gets in we can add the correct clock binding and replace
devm_clk_get(&pdev->dev, "uhc"); with devm_clk_get(&pdev->dev, NULL);

Specifying the current binding got NAKed by Arnd who mentioned that clock names shouldn't be needed as required properties.
And even if needed, then it shouldn't be "uhc" and more close to what other ohci drivers have.

Hence rather than bad binding now, we'll add a binding later..

> 
>> [...]
>> +Example for jz4740:
>> +
>> +/ {
>> +    ohci: jz4780-ohci@0x134a0000 {
> 
> s/jz4780/jz4740

Thanks

ZubairLK

> 
>> +        compatible = "ingenic,jz4740-ohci";
>> +        reg = <0x134a0000 0x10000>;
>> +
>> +        interrupts = <5>;
>> +    };
>> +};
>> +
>>
> 
--
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
Sergei Shtylyov Feb. 3, 2015, 11:54 a.m. UTC | #3
Hello.

On 2/3/2015 1:17 PM, Zubair Lutfullah Kakakhel wrote:

> From: Paul Burton <paul.burton@imgtec.com>

> Add the binding documentation for the JZ4740 OHCI controller.

> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

[...]

> diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt b/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt
> new file mode 100644
> index 0000000..29c1934
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt
> @@ -0,0 +1,21 @@
> +Ingenic JZ4740 SoC OHCI controller binding
> +
> +The Ingenic JZ4740 SoC includes an OHCI compliant USB host controller
> +interface for use with USB 1.1 devices.
> +
> +Required properties:
> + - compatible: Should be "ingenic,jz4740-ohci"
> + - reg: Should contain the address & size of the OHCI controller registers.
> + - interrupts: Should specify the interrupt line number
> +
> +Example for jz4740:
> +
> +/ {
> +	ohci: jz4780-ohci@0x134a0000 {

    The name should be "usb@134a0000", according to the ePAPR standard.

[...]

WBR, Sergei

--
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
Lars-Peter Clausen Feb. 3, 2015, 3:13 p.m. UTC | #4
On 02/03/2015 11:53 AM, Zubair Lutfullah Kakakhel wrote:
>
> On 03/02/15 10:32, Lars-Peter Clausen wrote:
>> On 02/03/2015 11:17 AM, Zubair Lutfullah Kakakhel wrote:
>> [...]
>>> V4 Changes
>>> Removed clock binding because of pending work in clock tree. Will add
>>> binding later. Rather than introduce a bad binding now and change later.
>>
>> But this patch is introducing a bad binding. The part needs the clock to work. The binding you are specifying right now says that it works fine without any clocks.
>
> Facing a chicken and egg problem with these patches here..
>
> When the new clock driver gets in we can add the correct clock binding and replace
> devm_clk_get(&pdev->dev, "uhc"); with devm_clk_get(&pdev->dev, NULL);
>
> Specifying the current binding got NAKed by Arnd who mentioned that clock names shouldn't be needed as required properties.
> And even if needed, then it shouldn't be "uhc" and more close to what other ohci drivers have.

It's not a chicken and egg problem. The order is very clear, first convert 
the clock driver to the CCF, then update the driver to pass NULL instead of 
"uhc", then add the devicetree bindings.

- Lars

--
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 mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt b/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt
new file mode 100644
index 0000000..29c1934
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ingenic,jz4740-ohci.txt
@@ -0,0 +1,21 @@ 
+Ingenic JZ4740 SoC OHCI controller binding
+
+The Ingenic JZ4740 SoC includes an OHCI compliant USB host controller
+interface for use with USB 1.1 devices.
+
+Required properties:
+ - compatible: Should be "ingenic,jz4740-ohci"
+ - reg: Should contain the address & size of the OHCI controller registers.
+ - interrupts: Should specify the interrupt line number
+
+Example for jz4740:
+
+/ {
+	ohci: jz4780-ohci@0x134a0000 {
+		compatible = "ingenic,jz4740-ohci";
+		reg = <0x134a0000 0x10000>;
+
+		interrupts = <5>;
+	};
+};
+