[3/4] GMAC: dts: add gmac info for rk3288
diff mbox

Message ID 1416906536-26158-1-git-send-email-roger.chen@rock-chips.com
State New, archived
Headers show

Commit Message

Roger Chen Nov. 25, 2014, 9:08 a.m. UTC
add gmac info in rk3288.dtsi for GMAC driver

Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
---
 arch/arm/boot/dts/rk3288.dtsi |   59 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

Comments

Kever Yang Nov. 25, 2014, 9:55 a.m. UTC | #1
Hi Roger,

The Subject should use below prefix:
ARM: dts: rockchip: add gmac info for rk3288

On 11/25/2014 05:08 PM, Roger Chen wrote:
> add gmac info in rk3288.dtsi for GMAC driver
>
> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> ---
>   arch/arm/boot/dts/rk3288.dtsi |   59 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 0f50d5d..949675d 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -137,6 +137,13 @@
>   		#clock-cells = <0>;
>   	};
>   
> +	ext_gmac: external-gmac-clock {
> +		compatible = "fixed-clock";
> +		clock-frequency = <125000000>;
> +		clock-output-names = "ext_gmac";
> +		#clock-cells = <0>;
> +	};
> +
I'm not sure Heiko will happy with both add the new clock source and
gmac node in these patch.

>   	timer {
>   		compatible = "arm,armv7-timer";
>   		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> @@ -490,6 +497,25 @@
>   		reg = <0xff740000 0x1000>;
>   	};
>   
> +	gmac: eth@ff290000 {
> +		compatible = "rockchip,rk3288-gmac";
> +		reg = <0xff290000 0x10000>;
> +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
> +		interrupt-names = "macirq";
> +		rockchip,grf = <&grf>;
> +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
> +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
> +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
> +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
> +		clock-names = "stmmaceth", "clk_mac_pll",
> +			"mac_clk_rx", "mac_clk_tx",
> +			"clk_mac_ref", "clk_mac_refout",
> +			"aclk_mac", "pclk_mac";
> +		phy-mode = "rgmii";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
> +	};
> +
The controller dts node should be sort by the base address, I'm sure 
this is in a
wrong place.

- Kever
>   	cru: clock-controller@ff760000 {
>   		compatible = "rockchip,rk3288-cru";
>   		reg = <0xff760000 0x1000>;
> @@ -1040,5 +1066,38 @@
>   				rockchip,pins = <7 23 3 &pcfg_pull_none>;
>   			};
>   		};
> +
> +		gmac {
> +			rgmii_pin: rgmii-pins {
> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> +						<3 31 3 &pcfg_pull_none>,
> +						<3 26 3 &pcfg_pull_none>,
> +						<3 27 3 &pcfg_pull_none>,
> +						<3 28 3 &pcfg_pull_none>,
> +						<3 29 3 &pcfg_pull_none>,
> +						<3 24 3 &pcfg_pull_none>,
> +						<3 25 3 &pcfg_pull_none>,
> +						<4 0 3 &pcfg_pull_none>,
> +						<4 5 3 &pcfg_pull_none>,
> +						<4 6 3 &pcfg_pull_none>,
> +						<4 9 3 &pcfg_pull_none>,
> +						<4 4 3 &pcfg_pull_none>,
> +						<4 1 3 &pcfg_pull_none>,
> +						<4 3 3 &pcfg_pull_none>;
> +			};
> +
> +			rmii_pin: rmii-pins {
> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> +						<3 31 3 &pcfg_pull_none>,
> +						<3 28 3 &pcfg_pull_none>,
> +						<3 29 3 &pcfg_pull_none>,
> +						<4 0 3 &pcfg_pull_none>,
> +						<4 5 3 &pcfg_pull_none>,
> +						<4 4 3 &pcfg_pull_none>,
> +						<4 1 3 &pcfg_pull_none>,
> +						<4 2 3 &pcfg_pull_none>,
> +						<4 3 3 &pcfg_pull_none>;
> +			};
> +		};
>   	};
>   };
Heiko Stuebner Nov. 25, 2014, 10:12 a.m. UTC | #2
Hi Roger, Kever,

Am Dienstag, 25. November 2014, 17:55:53 schrieb Kever Yang:
> The Subject should use below prefix:
> ARM: dts: rockchip: add gmac info for rk3288
> 
> On 11/25/2014 05:08 PM, Roger Chen wrote:
> > add gmac info in rk3288.dtsi for GMAC driver
> > 
> > Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> > ---
> > 
> >   arch/arm/boot/dts/rk3288.dtsi |   59
> >   +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
> >   insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 0f50d5d..949675d 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -137,6 +137,13 @@
> > 
> >   		#clock-cells = <0>;
> >   	
> >   	};
> > 
> > +	ext_gmac: external-gmac-clock {
> > +		compatible = "fixed-clock";
> > +		clock-frequency = <125000000>;
> > +		clock-output-names = "ext_gmac";
> > +		#clock-cells = <0>;
> > +	};
> > +
> 
> I'm not sure Heiko will happy with both add the new clock source and
> gmac node in these patch.

correct. The ext_gmac is an external clock input providing a special clock to 
the gmac. This makes it a board-specific property, which should also only be 
added on boards having this external source - and of course in the board-
specific dts.


> 
> >   	timer {
> >   	
> >   		compatible = "arm,armv7-timer";
> >   		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> >   		IRQ_TYPE_LEVEL_HIGH)>,
> > 
> > @@ -490,6 +497,25 @@
> > 
> >   		reg = <0xff740000 0x1000>;
> >   	
> >   	};
> > 
> > +	gmac: eth@ff290000 {
> > +		compatible = "rockchip,rk3288-gmac";
> > +		reg = <0xff290000 0x10000>;
> > +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/

please remote the "irq=59" comment, as it is not necessary


> > +		interrupt-names = "macirq";
> > +		rockchip,grf = <&grf>;
> > +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
> > +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
> > +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
> > +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
> > +		clock-names = "stmmaceth", "clk_mac_pll",
> > +			"mac_clk_rx", "mac_clk_tx",
> > +			"clk_mac_ref", "clk_mac_refout",
> > +			"aclk_mac", "pclk_mac";
> > +		phy-mode = "rgmii";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
> > +	};
> > +
> 
> The controller dts node should be sort by the base address, I'm sure
> this is in a
> wrong place.

correct, it is on the wrong place


Heiko
Sergei Shtylyov Nov. 25, 2014, 1:40 p.m. UTC | #3
Hello.

On 11/25/2014 12:08 PM, Roger Chen wrote:

> add gmac info in rk3288.dtsi for GMAC driver

> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> ---
>   arch/arm/boot/dts/rk3288.dtsi |   59 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)

> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 0f50d5d..949675d 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
[...]
> @@ -490,6 +497,25 @@
>   		reg = <0xff740000 0x1000>;
>   	};
>
> +	gmac: eth@ff290000 {

    Please name the node "ethernet@ff290000" to comply with the ePAPR standard.

> +		compatible = "rockchip,rk3288-gmac";
> +		reg = <0xff290000 0x10000>;
> +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
> +		interrupt-names = "macirq";
> +		rockchip,grf = <&grf>;
> +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
> +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
> +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
> +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
> +		clock-names = "stmmaceth", "clk_mac_pll",
> +			"mac_clk_rx", "mac_clk_tx",
> +			"clk_mac_ref", "clk_mac_refout",
> +			"aclk_mac", "pclk_mac";
> +		phy-mode = "rgmii";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;

    Hm, pinctrl props in a .dtsi file? Those are usually board dependent.

[...]
> @@ -1040,5 +1066,38 @@
>   				rockchip,pins = <7 23 3 &pcfg_pull_none>;
>   			};
>   		};
> +
> +		gmac {
> +			rgmii_pin: rgmii-pins {
> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> +						<3 31 3 &pcfg_pull_none>,
> +						<3 26 3 &pcfg_pull_none>,
> +						<3 27 3 &pcfg_pull_none>,
> +						<3 28 3 &pcfg_pull_none>,
> +						<3 29 3 &pcfg_pull_none>,
> +						<3 24 3 &pcfg_pull_none>,
> +						<3 25 3 &pcfg_pull_none>,
> +						<4 0 3 &pcfg_pull_none>,
> +						<4 5 3 &pcfg_pull_none>,
> +						<4 6 3 &pcfg_pull_none>,
> +						<4 9 3 &pcfg_pull_none>,
> +						<4 4 3 &pcfg_pull_none>,
> +						<4 1 3 &pcfg_pull_none>,
> +						<4 3 3 &pcfg_pull_none>;
> +			};
> +
> +			rmii_pin: rmii-pins {
> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> +						<3 31 3 &pcfg_pull_none>,
> +						<3 28 3 &pcfg_pull_none>,
> +						<3 29 3 &pcfg_pull_none>,
> +						<4 0 3 &pcfg_pull_none>,
> +						<4 5 3 &pcfg_pull_none>,
> +						<4 4 3 &pcfg_pull_none>,
> +						<4 1 3 &pcfg_pull_none>,
> +						<4 2 3 &pcfg_pull_none>,
> +						<4 3 3 &pcfg_pull_none>;
> +			};
> +		};

    These are usually define in the board .dts file...

WBR, Sergei
Heiko Stuebner Nov. 25, 2014, 2:39 p.m. UTC | #4
Am Dienstag, 25. November 2014, 16:40:59 schrieb Sergei Shtylyov:
> Hello.
> 
> On 11/25/2014 12:08 PM, Roger Chen wrote:
> > add gmac info in rk3288.dtsi for GMAC driver
> > 
> > Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> > ---
> > 
> >   arch/arm/boot/dts/rk3288.dtsi |   59
> >   +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
> >   insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 0f50d5d..949675d 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> 
> [...]
> 
> > @@ -490,6 +497,25 @@
> > 
> >   		reg = <0xff740000 0x1000>;
> >   	
> >   	};
> > 
> > +	gmac: eth@ff290000 {
> 
>     Please name the node "ethernet@ff290000" to comply with the ePAPR
> standard.
> > +		compatible = "rockchip,rk3288-gmac";
> > +		reg = <0xff290000 0x10000>;
> > +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
> > +		interrupt-names = "macirq";
> > +		rockchip,grf = <&grf>;
> > +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
> > +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
> > +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
> > +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
> > +		clock-names = "stmmaceth", "clk_mac_pll",
> > +			"mac_clk_rx", "mac_clk_tx",
> > +			"clk_mac_ref", "clk_mac_refout",
> > +			"aclk_mac", "pclk_mac";
> > +		phy-mode = "rgmii";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
> 
>     Hm, pinctrl props in a .dtsi file? Those are usually board dependent.

yep, especially as there is a board-dependent selection needed of what to use 
[rgmii or rmii] depending on the phy on the board.


> 
> [...]
> 
> > @@ -1040,5 +1066,38 @@
> > 
> >   				rockchip,pins = <7 23 3 &pcfg_pull_none>;
> >   			
> >   			};
> >   		
> >   		};
> > 
> > +
> > +		gmac {
> > +			rgmii_pin: rgmii-pins {

please add the "s" to the label - "rgmii_pins"


> > +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> > +						<3 31 3 &pcfg_pull_none>,
> > +						<3 26 3 &pcfg_pull_none>,
> > +						<3 27 3 &pcfg_pull_none>,
> > +						<3 28 3 &pcfg_pull_none>,
> > +						<3 29 3 &pcfg_pull_none>,
> > +						<3 24 3 &pcfg_pull_none>,
> > +						<3 25 3 &pcfg_pull_none>,
> > +						<4 0 3 &pcfg_pull_none>,
> > +						<4 5 3 &pcfg_pull_none>,
> > +						<4 6 3 &pcfg_pull_none>,
> > +						<4 9 3 &pcfg_pull_none>,
> > +						<4 4 3 &pcfg_pull_none>,
> > +						<4 1 3 &pcfg_pull_none>,
> > +						<4 3 3 &pcfg_pull_none>;
> > +			};
> > +
> > +			rmii_pin: rmii-pins {

same here


> > +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
> > +						<3 31 3 &pcfg_pull_none>,
> > +						<3 28 3 &pcfg_pull_none>,
> > +						<3 29 3 &pcfg_pull_none>,
> > +						<4 0 3 &pcfg_pull_none>,
> > +						<4 5 3 &pcfg_pull_none>,
> > +						<4 4 3 &pcfg_pull_none>,
> > +						<4 1 3 &pcfg_pull_none>,
> > +						<4 2 3 &pcfg_pull_none>,
> > +						<4 3 3 &pcfg_pull_none>;
> > +			};
> > +		};
> 
>     These are usually define in the board .dts file...

The pinctrl settings itself are soc-specific, i.e. the pins and their settings 
to use to enable r{g}mii functionality are the same for all boards using this 
soc, so the pinctrl definitions should stay here and not be redefined in each 
and every board file.


Heiko
Roger Chen Nov. 26, 2014, 2:49 a.m. UTC | #5
On 2014/11/25 22:39, Heiko Stübner wrote:
> Am Dienstag, 25. November 2014, 16:40:59 schrieb Sergei Shtylyov:
>> Hello.
>>
>> On 11/25/2014 12:08 PM, Roger Chen wrote:
>>> add gmac info in rk3288.dtsi for GMAC driver
>>>
>>> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
>>> ---
>>>
>>>    arch/arm/boot/dts/rk3288.dtsi |   59
>>>    +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
>>>    insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
>>> index 0f50d5d..949675d 100644
>>> --- a/arch/arm/boot/dts/rk3288.dtsi
>>> +++ b/arch/arm/boot/dts/rk3288.dtsi
>> [...]
>>
>>> @@ -490,6 +497,25 @@
>>>
>>>    		reg = <0xff740000 0x1000>;
>>>    	
>>>    	};
>>>
>>> +	gmac: eth@ff290000 {
>>      Please name the node "ethernet@ff290000" to comply with the ePAPR
>> standard.
>>> +		compatible = "rockchip,rk3288-gmac";
>>> +		reg = <0xff290000 0x10000>;
>>> +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
>>> +		interrupt-names = "macirq";
>>> +		rockchip,grf = <&grf>;
>>> +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
>>> +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
>>> +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
>>> +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
>>> +		clock-names = "stmmaceth", "clk_mac_pll",
>>> +			"mac_clk_rx", "mac_clk_tx",
>>> +			"clk_mac_ref", "clk_mac_refout",
>>> +			"aclk_mac", "pclk_mac";
>>> +		phy-mode = "rgmii";
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
>>      Hm, pinctrl props in a .dtsi file? Those are usually board dependent.
> yep, especially as there is a board-dependent selection needed of what to use
> [rgmii or rmii] depending on the phy on the board.

of course pinctrl props is redefined in the rk3288-evb-rk808.dts,
IMO, pinctrl props in .dtsi is used to fit for the "phy-mode" prop as default.
if pinctrl props should be removed, how about "phy-mode" prop?

>> [...]
>>
>>> @@ -1040,5 +1066,38 @@
>>>
>>>    				rockchip,pins = <7 23 3 &pcfg_pull_none>;
>>>    			
>>>    			};
>>>    		
>>>    		};
>>>
>>> +
>>> +		gmac {
>>> +			rgmii_pin: rgmii-pins {
> please add the "s" to the label - "rgmii_pins"
>
>
>>> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
>>> +						<3 31 3 &pcfg_pull_none>,
>>> +						<3 26 3 &pcfg_pull_none>,
>>> +						<3 27 3 &pcfg_pull_none>,
>>> +						<3 28 3 &pcfg_pull_none>,
>>> +						<3 29 3 &pcfg_pull_none>,
>>> +						<3 24 3 &pcfg_pull_none>,
>>> +						<3 25 3 &pcfg_pull_none>,
>>> +						<4 0 3 &pcfg_pull_none>,
>>> +						<4 5 3 &pcfg_pull_none>,
>>> +						<4 6 3 &pcfg_pull_none>,
>>> +						<4 9 3 &pcfg_pull_none>,
>>> +						<4 4 3 &pcfg_pull_none>,
>>> +						<4 1 3 &pcfg_pull_none>,
>>> +						<4 3 3 &pcfg_pull_none>;
>>> +			};
>>> +
>>> +			rmii_pin: rmii-pins {
> same here
>
>
>>> +				rockchip,pins = <3 30 3 &pcfg_pull_none>,
>>> +						<3 31 3 &pcfg_pull_none>,
>>> +						<3 28 3 &pcfg_pull_none>,
>>> +						<3 29 3 &pcfg_pull_none>,
>>> +						<4 0 3 &pcfg_pull_none>,
>>> +						<4 5 3 &pcfg_pull_none>,
>>> +						<4 4 3 &pcfg_pull_none>,
>>> +						<4 1 3 &pcfg_pull_none>,
>>> +						<4 2 3 &pcfg_pull_none>,
>>> +						<4 3 3 &pcfg_pull_none>;
>>> +			};
>>> +		};
>>      These are usually define in the board .dts file...
> The pinctrl settings itself are soc-specific, i.e. the pins and their settings
> to use to enable r{g}mii functionality are the same for all boards using this
> soc, so the pinctrl definitions should stay here and not be redefined in each
> and every board file.
>
>
> Heiko
>
>
>
Heiko Stuebner Nov. 26, 2014, 8:47 a.m. UTC | #6
Am Mittwoch, 26. November 2014, 10:49:17 schrieb Roger:
> On 2014/11/25 22:39, Heiko Stübner wrote:
> > Am Dienstag, 25. November 2014, 16:40:59 schrieb Sergei Shtylyov:
> >> Hello.
> >> 
> >> On 11/25/2014 12:08 PM, Roger Chen wrote:
> >>> add gmac info in rk3288.dtsi for GMAC driver
> >>> 
> >>> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> >>> ---
> >>> 
> >>>    arch/arm/boot/dts/rk3288.dtsi |   59
> >>>    +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
> >>>    insertions(+)
> >>> 
> >>> diff --git a/arch/arm/boot/dts/rk3288.dtsi
> >>> b/arch/arm/boot/dts/rk3288.dtsi
> >>> index 0f50d5d..949675d 100644
> >>> --- a/arch/arm/boot/dts/rk3288.dtsi
> >>> +++ b/arch/arm/boot/dts/rk3288.dtsi
> >> 
> >> [...]
> >> 
> >>> @@ -490,6 +497,25 @@
> >>> 
> >>>    		reg = <0xff740000 0x1000>;
> >>>    	
> >>>    	};
> >>> 
> >>> +	gmac: eth@ff290000 {
> >>> 
> >>      Please name the node "ethernet@ff290000" to comply with the ePAPR
> >> 
> >> standard.
> >> 
> >>> +		compatible = "rockchip,rk3288-gmac";
> >>> +		reg = <0xff290000 0x10000>;
> >>> +		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
> >>> +		interrupt-names = "macirq";
> >>> +		rockchip,grf = <&grf>;
> >>> +		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
> >>> +			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
> >>> +			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
> >>> +			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
> >>> +		clock-names = "stmmaceth", "clk_mac_pll",
> >>> +			"mac_clk_rx", "mac_clk_tx",
> >>> +			"clk_mac_ref", "clk_mac_refout",
> >>> +			"aclk_mac", "pclk_mac";
> >>> +		phy-mode = "rgmii";
> >>> +		pinctrl-names = "default";
> >>> +		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
> >>> 
> >>      Hm, pinctrl props in a .dtsi file? Those are usually board
> >>      dependent.
> > 
> > yep, especially as there is a board-dependent selection needed of what to
> > use [rgmii or rmii] depending on the phy on the board.
> 
> of course pinctrl props is redefined in the rk3288-evb-rk808.dts,
> IMO, pinctrl props in .dtsi is used to fit for the "phy-mode" prop as
> default. if pinctrl props should be removed, how about "phy-mode" prop?

yep, the phy-mode should also move to the board file - as it of course also 
depends on the phy-chip used there.

Patch
diff mbox

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 0f50d5d..949675d 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -137,6 +137,13 @@ 
 		#clock-cells = <0>;
 	};
 
+	ext_gmac: external-gmac-clock {
+		compatible = "fixed-clock";
+		clock-frequency = <125000000>;
+		clock-output-names = "ext_gmac";
+		#clock-cells = <0>;
+	};
+
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
@@ -490,6 +497,25 @@ 
 		reg = <0xff740000 0x1000>;
 	};
 
+	gmac: eth@ff290000 {
+		compatible = "rockchip,rk3288-gmac";
+		reg = <0xff290000 0x10000>;
+		interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;  /*irq=59*/
+		interrupt-names = "macirq";
+		rockchip,grf = <&grf>;
+		clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
+			<&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
+			<&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
+			<&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
+		clock-names = "stmmaceth", "clk_mac_pll",
+			"mac_clk_rx", "mac_clk_tx",
+			"clk_mac_ref", "clk_mac_refout",
+			"aclk_mac", "pclk_mac";
+		phy-mode = "rgmii";
+		pinctrl-names = "default";
+		pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
+	};
+
 	cru: clock-controller@ff760000 {
 		compatible = "rockchip,rk3288-cru";
 		reg = <0xff760000 0x1000>;
@@ -1040,5 +1066,38 @@ 
 				rockchip,pins = <7 23 3 &pcfg_pull_none>;
 			};
 		};
+
+		gmac {
+			rgmii_pin: rgmii-pins {
+				rockchip,pins = <3 30 3 &pcfg_pull_none>,
+						<3 31 3 &pcfg_pull_none>,
+						<3 26 3 &pcfg_pull_none>,
+						<3 27 3 &pcfg_pull_none>,
+						<3 28 3 &pcfg_pull_none>,
+						<3 29 3 &pcfg_pull_none>,
+						<3 24 3 &pcfg_pull_none>,
+						<3 25 3 &pcfg_pull_none>,
+						<4 0 3 &pcfg_pull_none>,
+						<4 5 3 &pcfg_pull_none>,
+						<4 6 3 &pcfg_pull_none>,
+						<4 9 3 &pcfg_pull_none>,
+						<4 4 3 &pcfg_pull_none>,
+						<4 1 3 &pcfg_pull_none>,
+						<4 3 3 &pcfg_pull_none>;
+			};
+
+			rmii_pin: rmii-pins {
+				rockchip,pins = <3 30 3 &pcfg_pull_none>,
+						<3 31 3 &pcfg_pull_none>,
+						<3 28 3 &pcfg_pull_none>,
+						<3 29 3 &pcfg_pull_none>,
+						<4 0 3 &pcfg_pull_none>,
+						<4 5 3 &pcfg_pull_none>,
+						<4 4 3 &pcfg_pull_none>,
+						<4 1 3 &pcfg_pull_none>,
+						<4 2 3 &pcfg_pull_none>,
+						<4 3 3 &pcfg_pull_none>;
+			};
+		};
 	};
 };