* [PATCH 3/4] GMAC: dts: add gmac info for rk3288
@ 2014-11-25 9:08 Roger Chen
2014-11-25 9:55 ` Kever Yang
2014-11-25 13:40 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Roger Chen @ 2014-11-25 9:08 UTC (permalink / raw)
To: heiko
Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip, kever.yang,
mark.yao, eddie.cai, roger.chen
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>;
+ };
+
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>;
+ };
+ };
};
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-25 9:08 [PATCH 3/4] GMAC: dts: add gmac info for rk3288 Roger Chen
@ 2014-11-25 9:55 ` Kever Yang
2014-11-25 10:12 ` Heiko Stübner
2014-11-25 13:40 ` Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Kever Yang @ 2014-11-25 9:55 UTC (permalink / raw)
To: Roger Chen, heiko
Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip, mark.yao,
eddie.cai
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>;
> + };
> + };
> };
> };
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-25 9:55 ` Kever Yang
@ 2014-11-25 10:12 ` Heiko Stübner
0 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2014-11-25 10:12 UTC (permalink / raw)
To: Kever Yang
Cc: Roger Chen, peppe.cavallaro, netdev, linux-kernel, linux-rockchip,
mark.yao, eddie.cai
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-25 9:08 [PATCH 3/4] GMAC: dts: add gmac info for rk3288 Roger Chen
2014-11-25 9:55 ` Kever Yang
@ 2014-11-25 13:40 ` Sergei Shtylyov
2014-11-25 14:39 ` Heiko Stübner
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2014-11-25 13:40 UTC (permalink / raw)
To: Roger Chen, heiko
Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip, kever.yang,
mark.yao, eddie.cai
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-25 13:40 ` Sergei Shtylyov
@ 2014-11-25 14:39 ` Heiko Stübner
2014-11-26 2:49 ` Roger
0 siblings, 1 reply; 7+ messages in thread
From: Heiko Stübner @ 2014-11-25 14:39 UTC (permalink / raw)
To: Roger Chen
Cc: Sergei Shtylyov, peppe.cavallaro, netdev, linux-kernel,
linux-rockchip, kever.yang, mark.yao, eddie.cai
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-25 14:39 ` Heiko Stübner
@ 2014-11-26 2:49 ` Roger
2014-11-26 8:47 ` Heiko Stübner
0 siblings, 1 reply; 7+ messages in thread
From: Roger @ 2014-11-26 2:49 UTC (permalink / raw)
To: Heiko Stübner
Cc: Sergei Shtylyov, peppe.cavallaro, netdev, linux-kernel,
linux-rockchip, kever.yang, mark.yao, eddie.cai
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
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
2014-11-26 2:49 ` Roger
@ 2014-11-26 8:47 ` Heiko Stübner
0 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2014-11-26 8:47 UTC (permalink / raw)
To: Roger
Cc: Sergei Shtylyov, peppe.cavallaro, netdev, linux-kernel,
linux-rockchip, kever.yang, mark.yao, eddie.cai
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-26 8:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-25 9:08 [PATCH 3/4] GMAC: dts: add gmac info for rk3288 Roger Chen
2014-11-25 9:55 ` Kever Yang
2014-11-25 10:12 ` Heiko Stübner
2014-11-25 13:40 ` Sergei Shtylyov
2014-11-25 14:39 ` Heiko Stübner
2014-11-26 2:49 ` Roger
2014-11-26 8:47 ` Heiko Stübner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).