[PATCH V3 1/5] dt-bindings: net: renesas,etheravb: Add additional clocks

[PATCH V3 1/5] dt-bindings: net: renesas,etheravb: Add additional clocks

[Adam Ford]

The AVB driver assumes there is an external crystal, but it could
be clocked by other means.  In order to enable a programmable
clock, it needs to be added to the clocks list and enabled in the
driver.  Since there currently only one clock, there is no
clock-names list either.

Update bindings to add the additional optional clock, and explicitly
name both of them.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
---
V3:  No Change
V2:  No Change

diff --git a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
index de9dd574a2f9..7b32363ad8b4 100644
--- a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
+++ b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
@@ -49,7 +49,16 @@ properties:
   interrupt-names: true
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: AVB functional clock
+      - description: Optional TXC reference clock
+
+  clock-names:
+    items:
+      - const: fck
+      - const: refclk
 
   iommus:
     maxItems: 1
-- 
2.25.1

[PATCH V3 2/5] ARM: dts: renesas: Add fck to etheravb-rcar-gen2 clock-names list

[Adam Ford]

The bindings have been updated to support two clocks, but the
original clock now requires the name fck.  Add a clock-names
list in the device tree with fck in it.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V3:  No Change
V2:  No Change

diff --git a/arch/arm/boot/dts/r8a7742.dtsi b/arch/arm/boot/dts/r8a7742.dtsi
index 6a78c813057b..6b922f664fcd 100644
--- a/arch/arm/boot/dts/r8a7742.dtsi
+++ b/arch/arm/boot/dts/r8a7742.dtsi
@@ -750,6 +750,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index f444e418f408..084bf3e039cf 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -702,6 +702,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7743_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7744.dtsi b/arch/arm/boot/dts/r8a7744.dtsi
index 0442aad4f9db..d01eba99ceb0 100644
--- a/arch/arm/boot/dts/r8a7744.dtsi
+++ b/arch/arm/boot/dts/r8a7744.dtsi
@@ -702,6 +702,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7744_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 0f14ac22921d..d0d45a369047 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -645,6 +645,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7745_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a77470.dtsi b/arch/arm/boot/dts/r8a77470.dtsi
index 691b1a131c87..ae90a001d663 100644
--- a/arch/arm/boot/dts/r8a77470.dtsi
+++ b/arch/arm/boot/dts/r8a77470.dtsi
@@ -537,6 +537,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77470_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index b0569b4ea5c8..af9cd3324f4c 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -768,6 +768,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7790_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 87f0d6dc3e5a..2354af7fa83f 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -728,6 +728,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7792.dtsi b/arch/arm/boot/dts/r8a7792.dtsi
index f5b299bfcb23..60c184ab1b49 100644
--- a/arch/arm/boot/dts/r8a7792.dtsi
+++ b/arch/arm/boot/dts/r8a7792.dtsi
@@ -537,6 +537,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7792_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index cd5e2904068a..18cc6f6b588d 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -598,6 +598,7 @@ avb: ethernet@e6800000 {
 			reg = <0 0xe6800000 0 0x800>, <0 0xee0e8000 0 0x4000>;
 			interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7794_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			#address-cells = <1>;
-- 
2.25.1

Re: [PATCH V3 2/5] ARM: dts: renesas: Add fck to etheravb-rcar-gen2 clock-names list

[Andrew Lunn]

On Wed, Feb 24, 2021 at 05:51:42AM -0600, Adam Ford wrote:
> The bindings have been updated to support two clocks, but the
> original clock now requires the name fck.  Add a clock-names
> list in the device tree with fck in it.

Hi Adam

I think requires is too strong. As far as i can see, you don't
introduce a change using the name 'fck'. So the name is optional,
which is good, because otherwise you would break backwards
compatibility with DT blobs.

Is the plan to merge this whole patchset via netdev? If so, you need
to repost anyway, once netdev reopens. So maybe you can change the
wording?

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 2/5] ARM: dts: renesas: Add fck to etheravb-rcar-gen2 clock-names list

[Geert Uytterhoeven]

Hi Andrew,

On Wed, Feb 24, 2021 at 2:45 PM Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Feb 24, 2021 at 05:51:42AM -0600, Adam Ford wrote:
> > The bindings have been updated to support two clocks, but the
> > original clock now requires the name fck.  Add a clock-names
> > list in the device tree with fck in it.
>
> I think requires is too strong. As far as i can see, you don't
> introduce a change using the name 'fck'. So the name is optional,
> which is good, because otherwise you would break backwards
> compatibility with DT blobs.
>
> Is the plan to merge this whole patchset via netdev? If so, you need
> to repost anyway, once netdev reopens. So maybe you can change the
> wording?

The DTS patches should go in through the renesas and soc trees.
I can apply them as soon as the DT binding patch has been accepted.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH V3 3/5] arm64: dts: renesas: Add fck to etheravb-rcar-gen3 clock-names list

[Adam Ford]

The bindings have been updated to support two clocks, but the
original clock now requires the name fck.  Add a clock-names
list in the device tree with fck in it.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V3:  No Change
V2:  No Change

diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
index d64fb8b1b86c..ec4feb7df775 100644
--- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
@@ -1127,6 +1127,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A774A1_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a774b1.dtsi b/arch/arm64/boot/dts/renesas/r8a774b1.dtsi
index 5b05474dc272..1ff62b2be1f3 100644
--- a/arch/arm64/boot/dts/renesas/r8a774b1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774b1.dtsi
@@ -1001,6 +1001,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A774B1_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
index 20fa3caa050e..a4d9c6b31574 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
@@ -957,6 +957,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A774C0_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index 8eb006cbd9af..fec5839163ec 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -1230,6 +1230,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A774E1_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77951.dtsi b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
index 5c39152e4570..1e622ab8a044 100644
--- a/arch/arm64/boot/dts/renesas/r8a77951.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
@@ -1312,6 +1312,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77960.dtsi b/arch/arm64/boot/dts/renesas/r8a77960.dtsi
index 25d947a81b29..a3d1c33cbc1d 100644
--- a/arch/arm64/boot/dts/renesas/r8a77960.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77960.dtsi
@@ -1188,6 +1188,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
index e8c31ebec097..55a3ba3c844f 100644
--- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
@@ -1144,6 +1144,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77961_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 657b20d3533b..dd4c0e621b9c 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1050,6 +1050,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi b/arch/arm64/boot/dts/renesas/r8a77970.dtsi
index 5a5d5649332a..d4b0b9952619 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi
@@ -612,6 +612,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77980.dtsi b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
index ec7ca72399ec..992a577a3b17 100644
--- a/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -664,6 +664,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 5010f23fafcc..cc56267e0850 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -1000,6 +1000,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 2319271c881b..84dba3719381 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -760,6 +760,7 @@ avb: ethernet@e6800000 {
 					  "ch20", "ch21", "ch22", "ch23",
 					  "ch24";
 			clocks = <&cpg CPG_MOD 812>;
+			clock-names = "fck";
 			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
 			resets = <&cpg 812>;
 			phy-mode = "rgmii";
-- 
2.25.1

Re: [PATCH V3 3/5] arm64: dts: renesas: Add fck to etheravb-rcar-gen3 clock-names list

[Andrew Lunn]

On Wed, Feb 24, 2021 at 05:51:43AM -0600, Adam Ford wrote:
> The bindings have been updated to support two clocks, but the
> original clock now requires the name fck.  Add a clock-names
> list in the device tree with fck in it.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 3/5] arm64: dts: renesas: Add fck to etheravb-rcar-gen3 clock-names list

[Geert Uytterhoeven]

On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> The bindings have been updated to support two clocks, but the
> original clock now requires the name fck.  Add a clock-names
> list in the device tree with fck in it.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

queueing in renesas-devel for v5.14, with an additional update for
the recently added r8a779a0.dtsi.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH V3 4/5] net: ethernet: ravb: Enable optional refclk

[Adam Ford]

For devices that use a programmable clock for the AVB reference clock,
the driver may need to enable them.  Add code to find the optional clock
and enable it when available.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3:  Change 'avb' to 'AVB'
     Remove unnessary else statement and pointer maniupluation when 
     enabling the refclock. 
     Add disable_unprepare call in remove funtion.
      
V2:  The previous patch to fetch the fclk was dropped.  In its place
     is code to enable the refclk

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 7453b17a37a2..ff363797bd2b 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -994,6 +994,7 @@ struct ravb_private {
 	struct platform_device *pdev;
 	void __iomem *addr;
 	struct clk *clk;
+	struct clk *refclk;
 	struct mdiobb_ctrl mdiobb;
 	u32 num_rx_ring[NUM_RX_QUEUE];
 	u32 num_tx_ring[NUM_TX_QUEUE];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index bd30505fbc57..614448e6eb24 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev)
 		goto out_release;
 	}
 
+	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
+	if (IS_ERR(priv->refclk)) {
+		error = PTR_ERR(priv->refclk);
+		goto out_release;
+	}
+	clk_prepare_enable(priv->refclk);
+
 	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
 	ndev->min_mtu = ETH_MIN_MTU;
 
@@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev)
 	if (priv->chip_id != RCAR_GEN2)
 		ravb_ptp_stop(ndev);
 
+	if (priv->refclk)
+		clk_disable_unprepare(priv->refclk);
+
 	dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
 			  priv->desc_bat_dma);
 	/* Set reset mode */
-- 
2.25.1

Re: [PATCH V3 4/5] net: ethernet: ravb: Enable optional refclk

[Andrew Lunn]

> @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev)
>  	if (priv->chip_id != RCAR_GEN2)
>  		ravb_ptp_stop(ndev);
>  
> +	if (priv->refclk)
> +		clk_disable_unprepare(priv->refclk);
> +

Hi Adam

You don't need the if (). The clk API is happy with a NULL pointer and
will do the right thing. Otherwise:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 4/5] net: ethernet: ravb: Enable optional refclk

[Geert Uytterhoeven]

Hi Adam,

On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> For devices that use a programmable clock for the AVB reference clock,
> the driver may need to enable them.  Add code to find the optional clock
> and enable it when available.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev)
>                 goto out_release;
>         }
>
> +       priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
> +       if (IS_ERR(priv->refclk)) {
> +               error = PTR_ERR(priv->refclk);
> +               goto out_release;
> +       }
> +       clk_prepare_enable(priv->refclk);
> +

Shouldn't the reference clock be disabled in case of any failure below?

>         ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
>         ndev->min_mtu = ETH_MIN_MTU;
>
> @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev)
>         if (priv->chip_id != RCAR_GEN2)
>                 ravb_ptp_stop(ndev);
>
> +       if (priv->refclk)
> +               clk_disable_unprepare(priv->refclk);
> +
>         dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
>                           priv->desc_bat_dma);
>         /* Set reset mode */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3 4/5] net: ethernet: ravb: Enable optional refclk

[Adam Ford]

On Thu, Mar 4, 2021 at 2:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > For devices that use a programmable clock for the AVB reference clock,
> > the driver may need to enable them.  Add code to find the optional clock
> > and enable it when available.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev)
> >                 goto out_release;
> >         }
> >
> > +       priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
> > +       if (IS_ERR(priv->refclk)) {
> > +               error = PTR_ERR(priv->refclk);
> > +               goto out_release;
> > +       }
> > +       clk_prepare_enable(priv->refclk);
> > +
>
> Shouldn't the reference clock be disabled in case of any failure below?
>
I'll generate a V4.

Should I just regenerate this patch since it seems like the rest are
OK, or should I regenerate the whole series?

adam
> >         ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
> >         ndev->min_mtu = ETH_MIN_MTU;
> >
> > @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev)
> >         if (priv->chip_id != RCAR_GEN2)
> >                 ravb_ptp_stop(ndev);
> >
> > +       if (priv->refclk)
> > +               clk_disable_unprepare(priv->refclk);
> > +
> >         dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
> >                           priv->desc_bat_dma);
> >         /* Set reset mode */
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH V3 4/5] net: ethernet: ravb: Enable optional refclk

[Geert Uytterhoeven]

Hi Adam,

On Mon, Mar 29, 2021 at 2:45 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Mar 4, 2021 at 2:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > For devices that use a programmable clock for the AVB reference clock,
> > > the driver may need to enable them.  Add code to find the optional clock
> > > and enable it when available.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev)
> > >                 goto out_release;
> > >         }
> > >
> > > +       priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
> > > +       if (IS_ERR(priv->refclk)) {
> > > +               error = PTR_ERR(priv->refclk);
> > > +               goto out_release;
> > > +       }
> > > +       clk_prepare_enable(priv->refclk);
> > > +
> >
> > Shouldn't the reference clock be disabled in case of any failure below?
> >
> I'll generate a V4.
>
> Should I just regenerate this patch since it seems like the rest are
> OK, or should I regenerate the whole series?

As the DT bindings haven't been applied yet, I think it would be
best if you would send a v4 with just the patches for the netdev
tree (i.e. DT bindings patch 1 and driver patch 4).

I will take the DT patches from this series, once the bindings have been
accepted.

Thank you!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Adam Ford]

The AVB refererence clock assumes an external clock that runs
automatically.  Because the Versaclock is wired to provide the
AVB refclock, the device tree needs to reference it in order for the
driver to start the clock.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3:  New to series

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index 8d3a4d6ee885..75355c354c38 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -53,6 +53,8 @@ &avb {
 	phy-handle = <&phy0>;
 	rx-internal-delay-ps = <1800>;
 	tx-internal-delay-ps = <2000>;
+	clocks = <&cpg CPG_MOD 812>, <&versaclock5 4>;
+	clock-names = "fck", "refclk";
 	status = "okay";
 
 	phy0: ethernet-phy@0 {
-- 
2.25.1

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Geert Uytterhoeven]

On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> The AVB refererence clock assumes an external clock that runs

reference

> automatically.  Because the Versaclock is wired to provide the
> AVB refclock, the device tree needs to reference it in order for the
> driver to start the clock.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel (with the typo fixed) once the DT
bindings have been accepted.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Adam Ford]

On Thu, Mar 4, 2021 at 2:04 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > The AVB refererence clock assumes an external clock that runs
>
> reference
>
> > automatically.  Because the Versaclock is wired to provide the
> > AVB refclock, the device tree needs to reference it in order for the
> > driver to start the clock.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel (with the typo fixed) once the DT
> bindings have been accepted.
>

Who do I need to ping to get the DT bindings accepted?  They have an
acked-by from Rob.

adam

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Geert Uytterhoeven]

Hi Adam,

On Thu, Mar 18, 2021 at 1:44 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Mar 4, 2021 at 2:04 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > The AVB refererence clock assumes an external clock that runs
> >
> > reference
> >
> > > automatically.  Because the Versaclock is wired to provide the
> > > AVB refclock, the device tree needs to reference it in order for the
> > > driver to start the clock.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in renesas-devel (with the typo fixed) once the DT
> > bindings have been accepted.
> >
>
> Who do I need to ping to get the DT bindings accepted?  They have an
> acked-by from Rob.

Sergei, can you please have a look at the DT binding change?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Sergei Shtylyov]

On 18.03.2021 15:44, Adam Ford wrote:

>>> The AVB refererence clock assumes an external clock that runs
>>
>> reference
>>
>>> automatically.  Because the Versaclock is wired to provide the
>>> AVB refclock, the device tree needs to reference it in order for the
>>> driver to start the clock.
>>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> i.e. will queue in renesas-devel (with the typo fixed) once the DT
>> bindings have been accepted.
>>
> 
> Who do I need to ping to get the DT bindings accepted?  They have an
> acked-by from Rob.

    Normally, the bindings get picked up by a subsystem maintainer... or Rob :-)

[...]

MBR, Sergei

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Adam Ford]

On Thu, Mar 4, 2021 at 2:04 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > The AVB refererence clock assumes an external clock that runs
>
> reference
>
> > automatically.  Because the Versaclock is wired to provide the
> > AVB refclock, the device tree needs to reference it in order for the
> > driver to start the clock.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel (with the typo fixed) once the DT
> bindings have been accepted.

Geert,

Since the refclk update and corresponding dt-bindings are in net-next,
are you OK applying the rest of the DT changes so they can get into
5.13?

adam
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH V3 5/5] arm64: dts: renesas: beacon kits: Setup AVB refclk

[Geert Uytterhoeven]

Hi Adam,

On Sat, Apr 17, 2021 at 3:54 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Mar 4, 2021 at 2:04 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > The AVB refererence clock assumes an external clock that runs
> >
> > reference
> >
> > > automatically.  Because the Versaclock is wired to provide the
> > > AVB refclock, the device tree needs to reference it in order for the
> > > driver to start the clock.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in renesas-devel (with the typo fixed) once the DT
> > bindings have been accepted.
>
> Geert,
>
> Since the refclk update and corresponding dt-bindings are in net-next,
> are you OK applying the rest of the DT changes so they can get into
> 5.13?

Queueing in renesas-devel for v5.14, as the soc deadline for v5.13
has already passed two weeks ago.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3 1/5] dt-bindings: net: renesas,etheravb: Add additional clocks

[Sergei Shtylyov]

Hi!

On 2/24/21 2:51 PM, Adam Ford wrote:

> The AVB driver assumes there is an external crystal, but it could
> be clocked by other means.  In order to enable a programmable
> clock, it needs to be added to the clocks list and enabled in the
> driver.  Since there currently only one clock, there is no
> clock-names list either.
> 
> Update bindings to add the additional optional clock, and explicitly
> name both of them.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Rob Herring <robh@kernel.org>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

[...]

PS: Sorry for the dalay reviewing...

MBR, Sergei