[RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Add documentations for macb mdio driver.

Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/net/macb-mdio.txt          |   31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt

diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt b/Documentation/devicetree/bindings/net/macb-mdio.txt
new file mode 100644
index 0000000..11c70a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
@@ -0,0 +1,31 @@
+* Cadence MACB MDIO controller
+
+Required properties:
+- compatible: Should be "cdns,macb-mdio"
+- reg: Address and length of the register set of MAC to be used
+- clock-names: Tuple listing input clock names.
+	Required elements: 'pclk', 'hclk'
+	Optional elements: 'tx_clk'
+- clocks: Phandles to input clocks.
+
+Examples:
+
+	mdio {
+		compatible = "cdns,macb-mdio";
+		reg = <0x0 0xff0b0000 0x0 0x1000>;
+		clocks = <&clk125>, <&clk125>, <&clk125>;
+		clock-names = "pclk", "hclk", "tx_clk";
+		ethernet_phyC: ethernet-phy@C {
+			reg = <C>;
+		};
+		ethernet_phy7: ethernet-phy@7 {
+			reg = <7>;
+		};
+		ethernet_phy3: ethernet-phy@3 {
+			reg = <3>;
+		};
+		ethernet_phy8: ethernet-phy@8 {
+			reg = <8>;
+		};
+	};
+
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Andrew Lunn]

> +	mdio {
> +		compatible = "cdns,macb-mdio";
> +		reg = <0x0 0xff0b0000 0x0 0x1000>;
> +		clocks = <&clk125>, <&clk125>, <&clk125>;
> +		clock-names = "pclk", "hclk", "tx_clk";
> +		ethernet_phyC: ethernet-phy@C {
> +			reg = <C>;

I think that needs an 0x prefix.

  Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Andrew Lunn]

Hi Harini

Is this backward compatible? Will devices using the old binding still
work?

  /* Disable RX and TX (XXX: Should we halt the transmission
   * more gracefully?)
    */
-   macb_writel(bp, NCR, 0);
+   ctrl = macb_readl(bp, NCR);
+   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+   macb_writel(bp, NCR, ctrl);
 
	/* Clear the stats registers (XXX: Update stats first?) */
-	macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
+	ctrl |= MACB_BIT(CLRSTAT);
+	macb_writel(bp, NCR, ctrl);
 
	/* Clear all status flags */
 	macb_writel(bp, TSR, -1);

It is not clear to me what this part has to do with MDIO.

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

HI Andrew,

On Fri, May 13, 2016 at 11:13 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> Hi Harini
>
> Is this backward compatible? Will devices using the old binding still
> work?

It isn't right now.
I will have to assign the bus read/write functions conditionally in order to
do that - I'll see if I can make it clean.

>
>   /* Disable RX and TX (XXX: Should we halt the transmission
>    * more gracefully?)
>     */
> -   macb_writel(bp, NCR, 0);
> +   ctrl = macb_readl(bp, NCR);
> +   ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
> +   macb_writel(bp, NCR, ctrl);
>
>         /* Clear the stats registers (XXX: Update stats first?) */
> -       macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
> +       ctrl |= MACB_BIT(CLRSTAT);
> +       macb_writel(bp, NCR, ctrl);
>
>         /* Clear all status flags */
>         macb_writel(bp, TSR, -1);
>
> It is not clear to me what this part has to do with MDIO.
>

Sorry, I'll move this to a separate patch in my next version.
It is intended to write those registers without disturbing reserved bits.

Regards,
Harini

[RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Add documentations for macb mdio driver.

Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
 .../devicetree/bindings/net/macb-mdio.txt          | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt

diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt b/Documentation/devicetree/bindings/net/macb-mdio.txt
new file mode 100644
index 0000000..014cedf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
@@ -0,0 +1,31 @@
+* Cadence MACB MDIO controller
+
+Required properties:
+- compatible: Should be "cdns,macb-mdio"
+- reg: Address and length of the register set of MAC to be used
+- clock-names: Tuple listing input clock names.
+	Required elements: 'pclk', 'hclk'
+	Optional elements: 'tx_clk'
+- clocks: Phandles to input clocks.
+
+Examples:
+
+	mdio {
+		compatible = "cdns,macb-mdio";
+		reg = <0x0 0xff0b0000 0x0 0x1000>;
+		clocks = <&clk125>, <&clk125>, <&clk125>;
+		clock-names = "pclk", "hclk", "tx_clk";
+		ethernet_phyC: ethernet-phy@C {
+			reg = <0xC>;
+		};
+		ethernet_phy7: ethernet-phy@7 {
+			reg = <0x7>;
+		};
+		ethernet_phy3: ethernet-phy@3 {
+			reg = <0x3>;
+		};
+		ethernet_phy8: ethernet-phy@8 {
+			reg = <0x8>;
+		};
+	};
+
-- 
2.7.4

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Rob Herring]

On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
> Add documentations for macb mdio driver.

Bindings document h/w, not drivers.

> 
> Signed-off-by: Harini Katakam <harinik-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/net/macb-mdio.txt          | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt b/Documentation/devicetree/bindings/net/macb-mdio.txt
> new file mode 100644
> index 0000000..014cedf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
> @@ -0,0 +1,31 @@
> +* Cadence MACB MDIO controller
> +
> +Required properties:
> +- compatible: Should be "cdns,macb-mdio"

Only one version ever? This needs more specific compatible strings.

> +- reg: Address and length of the register set of MAC to be used
> +- clock-names: Tuple listing input clock names.
> +	Required elements: 'pclk', 'hclk'
> +	Optional elements: 'tx_clk'
> +- clocks: Phandles to input clocks.
> +
> +Examples:
> +
> +	mdio {
> +		compatible = "cdns,macb-mdio";
> +		reg = <0x0 0xff0b0000 0x0 0x1000>;
> +		clocks = <&clk125>, <&clk125>, <&clk125>;
> +		clock-names = "pclk", "hclk", "tx_clk";
> +		ethernet_phyC: ethernet-phy@C {

lowercase hex for unit addresses please

> +			reg = <0xC>;
> +		};
> +		ethernet_phy7: ethernet-phy@7 {
> +			reg = <0x7>;
> +		};
> +		ethernet_phy3: ethernet-phy@3 {
> +			reg = <0x3>;
> +		};
> +		ethernet_phy8: ethernet-phy@8 {
> +			reg = <0x8>;
> +		};
> +	};
> +
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Florian Fainelli]

Le 12/03/16 à 13:35, Rob Herring a écrit :
> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
>> Add documentations for macb mdio driver.
> 
> Bindings document h/w, not drivers.
> 
>>
>> Signed-off-by: Harini Katakam <harinik@xilinx.com>
>> ---
>>  .../devicetree/bindings/net/macb-mdio.txt          | 31 ++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt b/Documentation/devicetree/bindings/net/macb-mdio.txt
>> new file mode 100644
>> index 0000000..014cedf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
>> @@ -0,0 +1,31 @@
>> +* Cadence MACB MDIO controller
>> +
>> +Required properties:
>> +- compatible: Should be "cdns,macb-mdio"
> 
> Only one version ever? This needs more specific compatible strings.
> 
>> +- reg: Address and length of the register set of MAC to be used
>> +- clock-names: Tuple listing input clock names.
>> +	Required elements: 'pclk', 'hclk'
>> +	Optional elements: 'tx_clk'
>> +- clocks: Phandles to input clocks.

You are also missing mandatory properties:

#address-cells = <1> and #size-cells = <0>

Where is patch 1? Can you make sure you have the same recipient list for
both patches in this series so we can review both the binding and driver?

Thanks!
-- 
Florian

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Rob,


Thanks for the review.
On Sun, Dec 4, 2016 at 3:05 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
<snip>
>> +Required properties:
>> +- compatible: Should be "cdns,macb-mdio"
>
> Only one version ever? This needs more specific compatible strings.
>

This is part of the Cadence MAC in a way.
I can use revision number from the Cadence spec I was working with.
But it is not necessarily specific that version.

I'll take care of the other comments in the next version.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Harini Katakam]

Hi Florian,

On Sun, Dec 4, 2016 at 4:10 AM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Le 12/03/16 à 13:35, Rob Herring a écrit :
>> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
>>> +- reg: Address and length of the register set of MAC to be used
>>> +- clock-names: Tuple listing input clock names.
>>> +    Required elements: 'pclk', 'hclk'
>>> +    Optional elements: 'tx_clk'
>>> +- clocks: Phandles to input clocks.
>
> You are also missing mandatory properties:
>
> #address-cells = <1> and #size-cells = <0>
>
> Where is patch 1? Can you make sure you have the same recipient list for
> both patches in this series so we can review both the binding and driver?
>

Thanks for review, I'll update.

I did send the cover letter, patch 1 and 2 to the same recipient list.
I can see them on the mailing list. The first patch is called:
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
I hope you can find it.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings

[Nicolas Ferre]

Le 05/12/2016 à 03:55, Harini Katakam a écrit :
> Hi Rob,
> 
> 
> Thanks for the review.
> On Sun, Dec 4, 2016 at 3:05 AM, Rob Herring <robh@kernel.org> wrote:
>> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
> <snip>
>>> +Required properties:
>>> +- compatible: Should be "cdns,macb-mdio"
>>
>> Only one version ever? This needs more specific compatible strings.
>>
> 
> This is part of the Cadence MAC in a way.
> I can use revision number from the Cadence spec I was working with.
> But it is not necessarily specific that version.

Yes it is:
you must specify the first precise SoC needing/implementing it:
"cdns,zynqmp-gem-mdio" or "xlnx,zynq-gem-mdio" or anything looking like
this.
So, if an Atmel implementation is slightly different, we can still use
our own "atmel,sama5d2-gem-mdio" or "cdns,sama5d2-gem-mdio"
compatibility string.
On the other hand, if it's strictly the same, we can use the
"xlnx,zynq-gem-mdio" compatibility without any problem...

> I'll take care of the other comments in the next version.
> 
> Regards,
> Harini
> 


-- 
Nicolas Ferre