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 <[email protected]>
---
Documentation/devicetree/bindings/net/marvell-pp2.txt | 10 ++++++----
drivers/net/ethernet/marvell/mvpp2.c | 15 +++++++++++++++
2 files changed, 21 insertions(+), 4 deletions(-)
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);
--
2.14.1
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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi Thomas,
On jeu., sept. 28 2017, Thomas Petazzoni <[email protected]> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com