Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1535528 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings Rob Herring <robh@kernel.org> - 2016-12-03 22:40 +0100
    Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings Florian Fainelli <f.fainelli@gmail.com> - 2016-12-03 23:50 +0100
      Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings Harini Katakam <harinikatakamlinux@gmail.com> - 2016-12-05 04:10 +0100
    Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings Harini Katakam <harinikatakamlinux@gmail.com> - 2016-12-05 04:00 +0100
      Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings Nicolas Ferre <nicolas.ferre@atmel.com> - 2016-12-05 11:30 +0100

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

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.
> +
> +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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [next] | [standalone]

#1535536

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

[toc] | [prev] | [next] | [standalone]

#1535765

Hi Florian,

On Sun, Dec 4, 2016 at 4:10 AM, Florian Fainelli <f.fainelli@gmail.com> 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

[toc] | [prev] | [next] | [standalone]

#1535763

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.

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

Regards,
Harini

[toc] | [prev] | [next] | [standalone]

#1535981

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web