diff mbox

[v2,05/11] dt-bindings: Add bindings for phy-da8xx-usb

Message ID 1458181615-27782-6-git-send-email-david@lechnology.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Lechner March 17, 2016, 2:26 a.m. UTC 
Device tree binding for new phy-da8xx-usb driver.

Signed-off-by: David Lechner <david@lechnology.com>
---

v2 changes: This is new patch in v2.


 .../devicetree/bindings/phy/phy-da8xx-usb.txt      | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt

Comments

Rob Herring (Arm) March 19, 2016, 11:56 p.m. UTC | #1 
On Wed, Mar 16, 2016 at 09:26:38PM -0500, David Lechner wrote:
> Device tree binding for new phy-da8xx-usb driver.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> 
> v2 changes: This is new patch in v2.
> 
> 
>  .../devicetree/bindings/phy/phy-da8xx-usb.txt      | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> new file mode 100644
> index 0000000..ed6b710
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> @@ -0,0 +1,34 @@
> +TI DaVinci DA8XX USB PHY
> +
> +Required properties:
> + - compatible: must be "ti,da830-usbphy".
> + - #phy-cells: must be 1.
> + - reg : Address and length of the CFGCHIP2 register.
> +
> +This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
> +controllers on DA8XX SoCs. Consumers of this device should use index 1 for

DA8xx

> +the USB 1.1 phy device and index 2 for the USB 2.0 phy device.
> +
> +Example:
> +
> +	usbphy: usbphy@1c14184 {

TI has been doing the unit addresses wrong I've recently found. It 
should match the reg property, not be the full physical address. Please 
fix all these in the example.

> +		compatible = "ti,da830-usbphy";
> +		#phy-cells = <1>;
> +		reg = <0x14184 4>;
> +	};
> +
> +	usb11: usb11@1e25000 {

usb@...

> +		compatible = "ti,da830-ohci";
> +		reg = <0x225000 0x1000>;
> +		interrupts = <59>;
> +		phys = <&usbphy 1>;
> +		phy-names = "usbphy";
> +	};
> +
> +	usb20: usb@1e00000 {
> +		compatible = "ti,da830-musb";
> +		reg = <0x200000 0x1000>;
> +		interrupts = <58>;
> +		phys = <&usbphy 2>;
> +		phy-names = "usbphy";
> +	};
> -- 
> 1.9.1
>
Sekhar Nori March 23, 2016, 5:06 p.m. UTC | #2 
On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
> Device tree binding for new phy-da8xx-usb driver.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> 
> v2 changes: This is new patch in v2.
> 
> 
>  .../devicetree/bindings/phy/phy-da8xx-usb.txt      | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> new file mode 100644
> index 0000000..ed6b710
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> @@ -0,0 +1,34 @@
> +TI DaVinci DA8XX USB PHY
> +
> +Required properties:
> + - compatible: must be "ti,da830-usbphy".
> + - #phy-cells: must be 1.
> + - reg : Address and length of the CFGCHIP2 register.

I am not sure passing CFGCHIP2 register as reg property to the phy is
future proof. At some point, we do want to move to common clock
framework and at that point USB clocks controlled by CFGCHIP2 will be a
separate driver needing access to the same register.

So I think the CFGCHIP2 access in USB phy driver should happen through a
syscon phandle. This needs to happen now, not later since we cannot
break DT backward-compatibility.

Thanks,
Sekhar
David Lechner March 23, 2016, 5:56 p.m. UTC | #3 
On 03/23/2016 12:06 PM, Sekhar Nori wrote:
> On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
>> Device tree binding for new phy-da8xx-usb driver.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>
>> v2 changes: This is new patch in v2.
>>
>>
>>   .../devicetree/bindings/phy/phy-da8xx-usb.txt      | 34 ++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> new file mode 100644
>> index 0000000..ed6b710
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> @@ -0,0 +1,34 @@
>> +TI DaVinci DA8XX USB PHY
>> +
>> +Required properties:
>> + - compatible: must be "ti,da830-usbphy".
>> + - #phy-cells: must be 1.
>> + - reg : Address and length of the CFGCHIP2 register.
>
> I am not sure passing CFGCHIP2 register as reg property to the phy is
> future proof. At some point, we do want to move to common clock
> framework and at that point USB clocks controlled by CFGCHIP2 will be a
> separate driver needing access to the same register.
>
> So I think the CFGCHIP2 access in USB phy driver should happen through a
> syscon phandle. This needs to happen now, not later since we cannot
> break DT backward-compatibility.
>

I think using "syscon" for the CFGCHIP registers makes sense (based on 
my minimal experience). Would we want one "syscon" device node that 
includes all of the CFGCHIP registers or one each?

Something like this?

cfgchip@1417C {
	compatible = "ti,da830-cfgchip", "syscon";
	reg = <1417C 20>;
}

or this?

cfgchip0@1417C {
	compatible = "ti,da830-cfgchip0", "syscon";
	reg = <1417C 4>;
}

cfgchip1@14180 {
	compatible = "ti,da830-cfgchip1", "syscon";
	reg = <14180 4>;
}

etc.



-or-

Would it be OK if the PHY driver registered clocks? I'm guessing this 
falls into the category of "not such a good idea".
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
new file mode 100644
index 0000000..ed6b710
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
@@ -0,0 +1,34 @@ 
+TI DaVinci DA8XX USB PHY
+
+Required properties:
+ - compatible: must be "ti,da830-usbphy".
+ - #phy-cells: must be 1.
+ - reg : Address and length of the CFGCHIP2 register.
+
+This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
+controllers on DA8XX SoCs. Consumers of this device should use index 1 for
+the USB 1.1 phy device and index 2 for the USB 2.0 phy device.
+
+Example:
+
+	usbphy: usbphy@1c14184 {
+		compatible = "ti,da830-usbphy";
+		#phy-cells = <1>;
+		reg = <0x14184 4>;
+	};
+
+	usb11: usb11@1e25000 {
+		compatible = "ti,da830-ohci";
+		reg = <0x225000 0x1000>;
+		interrupts = <59>;
+		phys = <&usbphy 1>;
+		phy-names = "usbphy";
+	};
+
+	usb20: usb@1e00000 {
+		compatible = "ti,da830-musb";
+		reg = <0x200000 0x1000>;
+		interrupts = <58>;
+		phys = <&usbphy 2>;
+		phy-names = "usbphy";
+	};