Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

2016-10-13 Thread LABBE Corentin
On Mon, Oct 10, 2016 at 10:13:35AM -0500, Rob Herring wrote:
> On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt  | 70 
> > ++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 000..92e4ef3b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,70 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: should be one of the following string:
> > +   "allwinner,sun8i-a83t-emac"
> > +   "allwinner,sun8i-h3-emac"
> > +   "allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register for the device.
> > +- syscon: A phandle to the syscon of the SoC
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> > Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> > Default is 0)
> > +Both delay properties does not have units, there are arbitrary value.
> 
> They have to have some sort of units. Some number of clocks perhaps. Or 
> just say what register field they correspond to.
> 

I have re-read all 3 datasheets (A64/H3/A83T) and made string search for 
finding any information.
But still found nothing, no unit, no more informations than I already wrote in 
this file.

For the register field, just saying that it is used in the syscon register 
EMAC_CLK_REG is sufficient ?

> > +The TX/RX clock delay chain settings are board specific and could be found
> > +in vendor FEX files.
> > +

Regards

Corentin Labbe


Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

2016-10-10 Thread Rob Herring
On Mon, Oct 10, 2016 at 02:30:23PM +0200, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt  | 70 
> > ++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 000..92e4ef3b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,70 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: should be one of the following string:
> > +   "allwinner,sun8i-a83t-emac"
> > +   "allwinner,sun8i-h3-emac"
> > +   "allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register for the device.
> > +- syscon: A phandle to the syscon of the SoC
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> > Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> > Default is 0)
> > +Both delay properties does not have units, there are arbitrary value.
> > +The TX/RX clock delay chain settings are board specific and could be found
> > +in vendor FEX files.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,leds-active-low: EPHY LEDs are active low
> > +
> > +Required child node of emac:
> > +- mdio bus node: should be named mdio
> > +
> > +Required properties of the mdio node:
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of the mdio node. See phy.txt for the generic PHY bindings.
> > +
> > +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> > +- clocks: an extra phandle to the reference clock for the EPHY
> > +- resets: an extra phandle to the reset control for the EPHY
> > +
> > +Example:
> > +
> > +emac: ethernet@01c0b000 {
> > +   compatible = "allwinner,sun8i-h3-emac";
> > +   syscon = <&syscon>;
> > +   reg = <0x01c0b000 0x104>;
> > +   interrupts = ;
> > +   resets = <&ccu RST_BUS_EMAC>;
> > +   reset-names = "ahb";
> > +   clocks = <&ccu CLK_BUS_EMAC>;
> > +   clock-names = "ahb";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   phy = <&int_mii_phy>;
> > +   phy-mode = "mii";
> > +   allwinner,leds-active-low;
> > +   mdio: mdio {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   int_mii_phy: ethernet-phy@1 {
> > +   reg = <1>;
> > +   clocks = <&ccu CLK_BUS_EPHY>;
> > +   resets = <&ccu RST_BUS_EPHY>;
> 
> That works for me, let's see how the DT maintainers feel about it.

The phy should have a compatible string since you have extra properties.

Rob


Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

2016-10-10 Thread Rob Herring
On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> Allwinner sun8i-emac driver.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../bindings/net/allwinner,sun8i-emac.txt  | 70 
> ++
>  1 file changed, 70 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> new file mode 100644
> index 000..92e4ef3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> @@ -0,0 +1,70 @@
> +* Allwinner sun8i EMAC ethernet controller
> +
> +Required properties:
> +- compatible: should be one of the following string:
> + "allwinner,sun8i-a83t-emac"
> + "allwinner,sun8i-h3-emac"
> + "allwinner,sun50i-a64-emac"
> +- reg: address and length of the register for the device.
> +- syscon: A phandle to the syscon of the SoC
> +- interrupts: interrupt for the device
> +- clocks: A phandle to the reference clock for this device
> +- clock-names: should be "ahb"
> +- resets: A phandle to the reset control for this device
> +- reset-names: should be "ahb"
> +- phy-mode: See ethernet.txt
> +- phy-handle: See ethernet.txt
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +Optional properties:
> +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> Default is 0)
> +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> Default is 0)
> +Both delay properties does not have units, there are arbitrary value.

They have to have some sort of units. Some number of clocks perhaps. Or 
just say what register field they correspond to.

> +The TX/RX clock delay chain settings are board specific and could be found
> +in vendor FEX files.
> +
> +Optional properties for "allwinner,sun8i-h3-emac":
> +- allwinner,leds-active-low: EPHY LEDs are active low
> +
> +Required child node of emac:
> +- mdio bus node: should be named mdio
> +
> +Required properties of the mdio node:
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +The device node referenced by "phy" or "phy-handle" should be a child node
> +of the mdio node. See phy.txt for the generic PHY bindings.
> +
> +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> +- clocks: an extra phandle to the reference clock for the EPHY
> +- resets: an extra phandle to the reset control for the EPHY
> +
> +Example:
> +
> +emac: ethernet@01c0b000 {

Drop leading 0.

> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = ;
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "ahb";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "ahb";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy = <&int_mii_phy>;
> + phy-mode = "mii";
> + allwinner,leds-active-low;
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + };
> + };
> +};
> -- 
> 2.7.3
> 


Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

2016-10-10 Thread Maxime Ripard
On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> Allwinner sun8i-emac driver.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../bindings/net/allwinner,sun8i-emac.txt  | 70 
> ++
>  1 file changed, 70 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> new file mode 100644
> index 000..92e4ef3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> @@ -0,0 +1,70 @@
> +* Allwinner sun8i EMAC ethernet controller
> +
> +Required properties:
> +- compatible: should be one of the following string:
> + "allwinner,sun8i-a83t-emac"
> + "allwinner,sun8i-h3-emac"
> + "allwinner,sun50i-a64-emac"
> +- reg: address and length of the register for the device.
> +- syscon: A phandle to the syscon of the SoC
> +- interrupts: interrupt for the device
> +- clocks: A phandle to the reference clock for this device
> +- clock-names: should be "ahb"
> +- resets: A phandle to the reset control for this device
> +- reset-names: should be "ahb"
> +- phy-mode: See ethernet.txt
> +- phy-handle: See ethernet.txt
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +Optional properties:
> +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> Default is 0)
> +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> Default is 0)
> +Both delay properties does not have units, there are arbitrary value.
> +The TX/RX clock delay chain settings are board specific and could be found
> +in vendor FEX files.
> +
> +Optional properties for "allwinner,sun8i-h3-emac":
> +- allwinner,leds-active-low: EPHY LEDs are active low
> +
> +Required child node of emac:
> +- mdio bus node: should be named mdio
> +
> +Required properties of the mdio node:
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +The device node referenced by "phy" or "phy-handle" should be a child node
> +of the mdio node. See phy.txt for the generic PHY bindings.
> +
> +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> +- clocks: an extra phandle to the reference clock for the EPHY
> +- resets: an extra phandle to the reset control for the EPHY
> +
> +Example:
> +
> +emac: ethernet@01c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = ;
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "ahb";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "ahb";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy = <&int_mii_phy>;
> + phy-mode = "mii";
> + allwinner,leds-active-low;
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;

That works for me, let's see how the DT maintainers feel about it.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


[PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

2016-10-07 Thread Corentin Labbe
This patch adds documentation for Device-Tree bindings for the
Allwinner sun8i-emac driver.

Signed-off-by: Corentin Labbe 
---
 .../bindings/net/allwinner,sun8i-emac.txt  | 70 ++
 1 file changed, 70 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
new file mode 100644
index 000..92e4ef3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
@@ -0,0 +1,70 @@
+* Allwinner sun8i EMAC ethernet controller
+
+Required properties:
+- compatible: should be one of the following string:
+   "allwinner,sun8i-a83t-emac"
+   "allwinner,sun8i-h3-emac"
+   "allwinner,sun50i-a64-emac"
+- reg: address and length of the register for the device.
+- syscon: A phandle to the syscon of the SoC
+- interrupts: interrupt for the device
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "ahb"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "ahb"
+- phy-mode: See ethernet.txt
+- phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+Optional properties:
+- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
Default is 0)
+- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
Default is 0)
+Both delay properties does not have units, there are arbitrary value.
+The TX/RX clock delay chain settings are board specific and could be found
+in vendor FEX files.
+
+Optional properties for "allwinner,sun8i-h3-emac":
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+Required child node of emac:
+- mdio bus node: should be named mdio
+
+Required properties of the mdio node:
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+The device node referenced by "phy" or "phy-handle" should be a child node
+of the mdio node. See phy.txt for the generic PHY bindings.
+
+Required properties of the phy node with "allwinner,sun8i-h3-emac":
+- clocks: an extra phandle to the reference clock for the EPHY
+- resets: an extra phandle to the reset control for the EPHY
+
+Example:
+
+emac: ethernet@01c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   syscon = <&syscon>;
+   reg = <0x01c0b000 0x104>;
+   interrupts = ;
+   resets = <&ccu RST_BUS_EMAC>;
+   reset-names = "ahb";
+   clocks = <&ccu CLK_BUS_EMAC>;
+   clock-names = "ahb";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy = <&int_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   mdio: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = <&ccu CLK_BUS_EPHY>;
+   resets = <&ccu RST_BUS_EPHY>;
+   };
+   };
+};
-- 
2.7.3