| Message ID | 20170928153923.29003-1-gregory.clement@free-electrons.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: mvpp2: Fix clock resource by adding an optional bus clock | expand |
Hello, On Thu, 28 Sep 2017 17:39:23 +0200, Gregory CLEMENT wrote: > diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt > index 7e2dad08a12e..49e1be6bb6ba 100644 > --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt > +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt > @@ -21,8 +21,9 @@ Required properties: > - main controller clock (for both armada-375-pp2 and armada-7k-pp2) > - GOP clock (for both armada-375-pp2 and armada-7k-pp2) > - MG clock (only for armada-7k-pp2) > -- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and > - "mg_clk" (the latter only for armada-7k-pp2). > + - AXI clock (only for armada-7k-pp2) > +- clock-names: names of used clocks, must be "pp_clk", "gop_clk", "mg_clk" > + and "axi"(the 2 latter only for armada-7k-pp2). Should be "axi_clk" not "axi", otherwise your binding documentation doesn't match the driver and example. Also, missing space after "axi". > > The ethernet ports are represented by subnodes. At least one port is > required. > @@ -78,8 +79,9 @@ Example for marvell,armada-7k-pp2: > cpm_ethernet: ethernet@0 { > compatible = "marvell,armada-7k-pp22"; > reg = <0x0 0x100000>, <0x129000 0xb000>; > - clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>; > - clock-names = "pp_clk", "gop_clk", "gp_clk"; > + clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, > + <&cpm_syscon0 1 5>, <&cpm_syscon0 1 18>; Please indent the second line with one more tab. > + clock-names = "pp_clk", "gop_clk", "gp_clk", "axi_clk"; > > eth0: eth0 { > interrupts = <ICU_GRP_NSR 39 IRQ_TYPE_LEVEL_HIGH>, > diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c > index dd0ee2691c86..33b6791df2bb 100644 > --- a/drivers/net/ethernet/marvell/mvpp2.c > +++ b/drivers/net/ethernet/marvell/mvpp2.c > @@ -792,6 +792,7 @@ struct mvpp2 { > struct clk *pp_clk; > struct clk *gop_clk; > struct clk *mg_clk; > + struct clk *axi_clk; > > /* List of pointers to port structures */ > struct mvpp2_port **port_list; > @@ -7963,6 +7964,16 @@ static int mvpp2_probe(struct platform_device *pdev) > err = clk_prepare_enable(priv->mg_clk); > if (err < 0) > goto err_gop_clk; > + > + priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk"); > + if (IS_ERR(priv->axi_clk)) { > + err = PTR_ERR(priv->axi_clk); > + priv->axi_clk = NULL; You should handle -EPROBE_DEFER here. Indeed, if we have -EPROBE_DEFER, we shouldn't treat it as "the clock doesn't exist, so let's skip it and continue", but rather as "the clock exists, but isn't ready to use yet, let's try later". Thanks! Thomas
Hi Thomas, On jeu., sept. 28 2017, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: >> /* List of pointers to port structures */ >> struct mvpp2_port **port_list; >> @@ -7963,6 +7964,16 @@ static int mvpp2_probe(struct platform_device *pdev) >> err = clk_prepare_enable(priv->mg_clk); >> if (err < 0) >> goto err_gop_clk; >> + >> + priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk"); >> + if (IS_ERR(priv->axi_clk)) { >> + err = PTR_ERR(priv->axi_clk); >> + priv->axi_clk = NULL; > > You should handle -EPROBE_DEFER here. Indeed, if we have -EPROBE_DEFER, > we shouldn't treat it as "the clock doesn't exist, so let's skip it and > continue", but rather as "the clock exists, but isn't ready to use yet, > let's try later". You're totally right, I will send a v2 (I will also fix the other issue you spotted). Thanks, Gregory > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt index 7e2dad08a12e..49e1be6bb6ba 100644 --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt @@ -21,8 +21,9 @@ Required properties: - main controller clock (for both armada-375-pp2 and armada-7k-pp2) - GOP clock (for both armada-375-pp2 and armada-7k-pp2) - MG clock (only for armada-7k-pp2) -- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and - "mg_clk" (the latter only for armada-7k-pp2). + - AXI clock (only for armada-7k-pp2) +- clock-names: names of used clocks, must be "pp_clk", "gop_clk", "mg_clk" + and "axi"(the 2 latter only for armada-7k-pp2). The ethernet ports are represented by subnodes. At least one port is required. @@ -78,8 +79,9 @@ Example for marvell,armada-7k-pp2: cpm_ethernet: ethernet@0 { compatible = "marvell,armada-7k-pp22"; reg = <0x0 0x100000>, <0x129000 0xb000>; - clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>; - clock-names = "pp_clk", "gop_clk", "gp_clk"; + clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, + <&cpm_syscon0 1 5>, <&cpm_syscon0 1 18>; + clock-names = "pp_clk", "gop_clk", "gp_clk", "axi_clk"; eth0: eth0 { interrupts = <ICU_GRP_NSR 39 IRQ_TYPE_LEVEL_HIGH>, diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index dd0ee2691c86..33b6791df2bb 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -792,6 +792,7 @@ struct mvpp2 { struct clk *pp_clk; struct clk *gop_clk; struct clk *mg_clk; + struct clk *axi_clk; /* List of pointers to port structures */ struct mvpp2_port **port_list; @@ -7963,6 +7964,16 @@ static int mvpp2_probe(struct platform_device *pdev) err = clk_prepare_enable(priv->mg_clk); if (err < 0) goto err_gop_clk; + + priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk"); + if (IS_ERR(priv->axi_clk)) { + err = PTR_ERR(priv->axi_clk); + priv->axi_clk = NULL; + } else { + err = clk_prepare_enable(priv->axi_clk); + if (err < 0) + goto err_gop_clk; + } } /* Get system's tclk rate */ @@ -8015,6 +8026,8 @@ static int mvpp2_probe(struct platform_device *pdev) return 0; err_mg_clk: + if (priv->axi_clk) + clk_disable_unprepare(priv->axi_clk); if (priv->hw_version == MVPP22) clk_disable_unprepare(priv->mg_clk); err_gop_clk: @@ -8052,6 +8065,8 @@ static int mvpp2_remove(struct platform_device *pdev) aggr_txq->descs_dma); } + if (priv->axi_clk) + clk_disable_unprepare(priv->axi_clk); clk_disable_unprepare(priv->mg_clk); clk_disable_unprepare(priv->pp_clk); clk_disable_unprepare(priv->gop_clk);
On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock is optional because not all the SoCs need them but at least for Armada 7K/8K it is actually mandatory. The binding documentation is updating accordingly. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- Documentation/devicetree/bindings/net/marvell-pp2.txt | 10 ++++++---- drivers/net/ethernet/marvell/mvpp2.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-)