[PATCH 0/4] Fix hang of Armada 8040 SoC in orion-mdio

[PATCH 0/4] Fix hang of Armada 8040 SoC in orion-mdio

[josua]

From: Josua Mayer <josua@solid-run.com>

With a modular kernel as configured by Debian a hang was observed with
the Armada 8040 SoC in the Clearfog GT and Macchiatobin boards.

The 8040 SoC actually requires four clocks to be enabled for the mdio
interface to function. All 4 clocks are already specified in
armada-cp110.dtsi. It has however been missed that the orion-mdio driver
only supports enabling up to three clocks.

This patch-set allows the orion-mdio driver to handle four clocks and
adds a warning when more clocks are specified to prevent this particular
oversight in the future.

Josua Mayer (4):
  dt-bindings: allow up to four clocks for orion-mdio
  net: mvmdio: allow up to four clocks to be specified for orion-mdio
  net: mvmdio: print warning when orion-mdio has too many clocks
  net: mvmdio: defer probe of orion-mdio if a clock is not ready

 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt |  2 +-
 drivers/net/ethernet/marvell/mvmdio.c                        | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.16.4

[PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[josua]

From: Josua Mayer <josua@solid-run.com>

Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
Update the binding to allow the extra clock to be specified.

Cc: stable@vger.kernel.org
Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 42cd81090a2c..3f3cfc1d8d4d 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -16,7 +16,7 @@ Required properties:
 
 Optional properties:
 - interrupts: interrupt line number for the SMI error/done interrupt
-- clocks: phandle for up to three required clocks for the MDIO instance
+- clocks: phandle for up to four required clocks for the MDIO instance
 
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
-- 
2.16.4

[PATCH 2/4] net: mvmdio: allow up to four clocks to be specified for orion-mdio

[josua]

From: Josua Mayer <josua@solid-run.com>

Allow up to four clocks to be specified and enabled for the orion-mdio
interface, which are required by the Armada 8k and defined in
armada-cp110.dtsi.

Fixes a hang in probing the mvmdio driver that was encountered on the
Clearfog GT 8K with all drivers built as modules, but also affects other
boards such as the MacchiatoBIN.

Cc: stable@vger.kernel.org
Fixes: 96cb43423822 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index c5dac6bd2be4..e17d563e97a6 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -64,7 +64,7 @@
 
 struct orion_mdio_dev {
 	void __iomem *regs;
-	struct clk *clk[3];
+	struct clk *clk[4];
 	/*
 	 * If we have access to the error interrupt pin (which is
 	 * somewhat misnamed as it not only reflects internal errors
-- 
2.16.4

[PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[josua]

From: Josua Mayer <josua@solid-run.com>

Print a warning when device tree specifies more than the maximum of four
clocks supported by orion-mdio. Because reading from mdio can lock up
the Armada 8k when a required clock is not initialized, it is important
to notify the user when a specified clock is ignored.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index e17d563e97a6..89a99bf8e87b 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		clk_prepare_enable(dev->clk[i]);
 	}
 
+	if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
+		dev_warn(dev, "unsupported number of clocks, limiting to the first "
+			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
+
 	dev->err_interrupt = platform_get_irq(pdev, 0);
 	if (dev->err_interrupt > 0 &&
 	    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
-- 
2.16.4

[PATCH 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready

[josua]

From: Josua Mayer <josua@solid-run.com>

Defer probing of the orion-mdio interface when enabling of either of the
clocks defer probing. This avoids locking up the Armada 8k SoC when mdio
is used before all clocks have been enabled.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 89a99bf8e87b..1034013426ad 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -321,6 +321,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
+		if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
+			ret = -EPROBE_DEFER;
+			goto out_clk;
+		}
 		if (IS_ERR(dev->clk[i]))
 			break;
 		clk_prepare_enable(dev->clk[i]);
@@ -366,6 +370,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	if (dev->err_interrupt > 0)
 		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 
+out_clk:
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		if (IS_ERR(dev->clk[i]))
 			break;
-- 
2.16.4

Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Andrew Lunn]

On Sat, Jul 06, 2019 at 05:18:57PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clock to be specified.
> 
> Cc: stable@vger.kernel.org
> Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>

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

    Andrew

Re: [PATCH 2/4] net: mvmdio: allow up to four clocks to be specified for orion-mdio

[Andrew Lunn]

On Sat, Jul 06, 2019 at 05:18:58PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Allow up to four clocks to be specified and enabled for the orion-mdio
> interface, which are required by the Armada 8k and defined in
> armada-cp110.dtsi.
> 
> Fixes a hang in probing the mvmdio driver that was encountered on the
> Clearfog GT 8K with all drivers built as modules, but also affects other
> boards such as the MacchiatoBIN.
> 
> Cc: stable@vger.kernel.org
> Fixes: 96cb43423822 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>

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

    Andrew

Re: [PATCH 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready

[Andrew Lunn]

On Sat, Jul 06, 2019 at 05:19:00PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Defer probing of the orion-mdio interface when enabling of either of the
> clocks defer probing.

Hi Josua

I'm having trouble parsing that sentence.

How about:

Defer probing of the orion-mdio interface when getting a clock returns
EPROBE_DEFER.

Otherwise:

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

    Andrew




 This avoids locking up the Armada 8k SoC when mdio
> is used before all clocks have been enabled.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index 89a99bf8e87b..1034013426ad 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -321,6 +321,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>  		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
> +		if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
> +			ret = -EPROBE_DEFER;
> +			goto out_clk;
> +		}
>  		if (IS_ERR(dev->clk[i]))
>  			break;
>  		clk_prepare_enable(dev->clk[i]);
> @@ -366,6 +370,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  	if (dev->err_interrupt > 0)
>  		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
>  
> +out_clk:
>  	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>  		if (IS_ERR(dev->clk[i]))
>  			break;
> -- 
> 2.16.4
> 

Re: [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[Andrew Lunn]

On Sat, Jul 06, 2019 at 05:18:59PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Print a warning when device tree specifies more than the maximum of four
> clocks supported by orion-mdio. Because reading from mdio can lock up
> the Armada 8k when a required clock is not initialized, it is important
> to notify the user when a specified clock is ignored.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index e17d563e97a6..89a99bf8e87b 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  		clk_prepare_enable(dev->clk[i]);
>  	}
>  
> +	if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
> +		dev_warn(dev, "unsupported number of clocks, limiting to the first "
> +			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
> +

Hi Josua

Humm. Say getting clock 0 returned -EINVAL, or some other error code.
We break out of the loop, since such errors are being ignored. We then
hit this new code. Getting clock 1 works, and then we incorrectly
print this message.

Rather than getting the i'th clock, get ARRAY_SIZE(dev->clk)'th clock.

       Andrew

Re: [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[Josua Mayer]

Hi Andrew,

Am 06.07.19 um 18:09 schrieb Andrew Lunn:
> On Sat, Jul 06, 2019 at 05:18:59PM +0200, josua@solid-run.com wrote:
>> From: Josua Mayer <josua@solid-run.com>
>>
>> Print a warning when device tree specifies more than the maximum of four
>> clocks supported by orion-mdio. Because reading from mdio can lock up
>> the Armada 8k when a required clock is not initialized, it is important
>> to notify the user when a specified clock is ignored.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>  drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
>> index e17d563e97a6..89a99bf8e87b 100644
>> --- a/drivers/net/ethernet/marvell/mvmdio.c
>> +++ b/drivers/net/ethernet/marvell/mvmdio.c
>> @@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>>  		clk_prepare_enable(dev->clk[i]);
>>  	}
>>  
>> +	if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
>> +		dev_warn(dev, "unsupported number of clocks, limiting to the first "
>> +			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
>> +
> Hi Josua
>
> Humm. Say getting clock 0 returned -EINVAL, or some other error code.
> We break out of the loop, since such errors are being ignored. We then
> hit this new code. Getting clock 1 works, and then we incorrectly
> print this message.

Good point about breaking out of the loop! I did indeed not take the
break condition of the loop into account.

I can not exactly follow your example though. The first failure of
of_clk_get will trigger break and exit the loop,
and then we would indeed print the wrong message ... .

>
> Rather than getting the i'th clock, get ARRAY_SIZE(dev->clk)'th clock.
I will gladly make the change in a v2.
>        Andrew

-
Josua

Re: [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5216 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.2-rc7 next-20190705]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/josua-solid-run-com/Fix-hang-of-Armada-8040-SoC-in-orion-mdio/20190707-111919
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/node.h:18:0,
                    from include/linux/cpu.h:17,
                    from include/linux/of_device.h:5,
                    from drivers/net/ethernet/marvell/mvmdio.c:26:
   drivers/net/ethernet/marvell/mvmdio.c: In function 'orion_mdio_probe':
>> drivers/net/ethernet/marvell/mvmdio.c:330:12: error: passing argument 1 of '_dev_warn' from incompatible pointer type [-Werror=incompatible-pointer-types]
      dev_warn(dev, "unsupported number of clocks, limiting to the first "
               ^
   include/linux/device.h:1487:12: note: in definition of macro 'dev_warn'
     _dev_warn(dev, dev_fmt(fmt), ##__VA_ARGS__)
               ^~~
   include/linux/device.h:1425:6: note: expected 'const struct device *' but argument is of type 'struct orion_mdio_dev *'
    void _dev_warn(const struct device *dev, const char *fmt, ...);
         ^~~~~~~~~
   cc1: some warnings being treated as errors

vim +/_dev_warn +330 drivers/net/ethernet/marvell/mvmdio.c

   275	
   276	static int orion_mdio_probe(struct platform_device *pdev)
   277	{
   278		enum orion_mdio_bus_type type;
   279		struct resource *r;
   280		struct mii_bus *bus;
   281		struct orion_mdio_dev *dev;
   282		int i, ret;
   283	
   284		type = (enum orion_mdio_bus_type)of_device_get_match_data(&pdev->dev);
   285	
   286		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   287		if (!r) {
   288			dev_err(&pdev->dev, "No SMI register address given\n");
   289			return -ENODEV;
   290		}
   291	
   292		bus = devm_mdiobus_alloc_size(&pdev->dev,
   293					      sizeof(struct orion_mdio_dev));
   294		if (!bus)
   295			return -ENOMEM;
   296	
   297		switch (type) {
   298		case BUS_TYPE_SMI:
   299			bus->read = orion_mdio_smi_read;
   300			bus->write = orion_mdio_smi_write;
   301			break;
   302		case BUS_TYPE_XSMI:
   303			bus->read = orion_mdio_xsmi_read;
   304			bus->write = orion_mdio_xsmi_write;
   305			break;
   306		}
   307	
   308		bus->name = "orion_mdio_bus";
   309		snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii",
   310			 dev_name(&pdev->dev));
   311		bus->parent = &pdev->dev;
   312	
   313		dev = bus->priv;
   314		dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
   315		if (!dev->regs) {
   316			dev_err(&pdev->dev, "Unable to remap SMI register\n");
   317			return -ENODEV;
   318		}
   319	
   320		init_waitqueue_head(&dev->smi_busy_wait);
   321	
   322		for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
   323			dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
   324			if (IS_ERR(dev->clk[i]))
   325				break;
   326			clk_prepare_enable(dev->clk[i]);
   327		}
   328	
   329		if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
 > 330			dev_warn(dev, "unsupported number of clocks, limiting to the first "
   331				 __stringify(ARRAY_SIZE(dev->clk)) "\n");
   332	
   333		dev->err_interrupt = platform_get_irq(pdev, 0);
   334		if (dev->err_interrupt > 0 &&
   335		    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
   336			dev_err(&pdev->dev,
   337				"disabling interrupt, resource size is too small\n");
   338			dev->err_interrupt = 0;
   339		}
   340		if (dev->err_interrupt > 0) {
   341			ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
   342						orion_mdio_err_irq,
   343						IRQF_SHARED, pdev->name, dev);
   344			if (ret)
   345				goto out_mdio;
   346	
   347			writel(MVMDIO_ERR_INT_SMI_DONE,
   348				dev->regs + MVMDIO_ERR_INT_MASK);
   349	
   350		} else if (dev->err_interrupt == -EPROBE_DEFER) {
   351			ret = -EPROBE_DEFER;
   352			goto out_mdio;
   353		}
   354	
   355		ret = of_mdiobus_register(bus, pdev->dev.of_node);
   356		if (ret < 0) {
   357			dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
   358			goto out_mdio;
   359		}
   360	
   361		platform_set_drvdata(pdev, bus);
   362	
   363		return 0;
   364	
   365	out_mdio:
   366		if (dev->err_interrupt > 0)
   367			writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
   368	
   369		for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
   370			if (IS_ERR(dev->clk[i]))
   371				break;
   372			clk_disable_unprepare(dev->clk[i]);
   373			clk_put(dev->clk[i]);
   374		}
   375	
   376		return ret;
   377	}
   378	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70930 bytes --]

Re: [PATCH 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6091 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.2-rc7 next-20190705]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/josua-solid-run-com/Fix-hang-of-Armada-8040-SoC-in-orion-mdio/20190707-111919
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/marvell/mvmdio.c: In function 'orion_mdio_probe':
>> drivers/net/ethernet/marvell/mvmdio.c:324:30: warning: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
      if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
                                 ^
   In file included from include/linux/clk.h:12:0,
                    from drivers/net/ethernet/marvell/mvmdio.c:20:
   include/linux/err.h:29:33: note: expected 'const void *' but argument is of type 'int'
    static inline long __must_check PTR_ERR(__force const void *ptr)
                                    ^~~~~~~
>> drivers/net/ethernet/marvell/mvmdio.c:324:19: warning: comparison between pointer and integer
      if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
                      ^~
   In file included from include/linux/node.h:18:0,
                    from include/linux/cpu.h:17,
                    from include/linux/of_device.h:5,
                    from drivers/net/ethernet/marvell/mvmdio.c:26:
   drivers/net/ethernet/marvell/mvmdio.c:334:12: error: passing argument 1 of '_dev_warn' from incompatible pointer type [-Werror=incompatible-pointer-types]
      dev_warn(dev, "unsupported number of clocks, limiting to the first "
               ^
   include/linux/device.h:1487:12: note: in definition of macro 'dev_warn'
     _dev_warn(dev, dev_fmt(fmt), ##__VA_ARGS__)
               ^~~
   include/linux/device.h:1425:6: note: expected 'const struct device *' but argument is of type 'struct orion_mdio_dev *'
    void _dev_warn(const struct device *dev, const char *fmt, ...);
         ^~~~~~~~~
   cc1: some warnings being treated as errors

vim +/PTR_ERR +324 drivers/net/ethernet/marvell/mvmdio.c

   275	
   276	static int orion_mdio_probe(struct platform_device *pdev)
   277	{
   278		enum orion_mdio_bus_type type;
   279		struct resource *r;
   280		struct mii_bus *bus;
   281		struct orion_mdio_dev *dev;
   282		int i, ret;
   283	
   284		type = (enum orion_mdio_bus_type)of_device_get_match_data(&pdev->dev);
   285	
   286		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   287		if (!r) {
   288			dev_err(&pdev->dev, "No SMI register address given\n");
   289			return -ENODEV;
   290		}
   291	
   292		bus = devm_mdiobus_alloc_size(&pdev->dev,
   293					      sizeof(struct orion_mdio_dev));
   294		if (!bus)
   295			return -ENOMEM;
   296	
   297		switch (type) {
   298		case BUS_TYPE_SMI:
   299			bus->read = orion_mdio_smi_read;
   300			bus->write = orion_mdio_smi_write;
   301			break;
   302		case BUS_TYPE_XSMI:
   303			bus->read = orion_mdio_xsmi_read;
   304			bus->write = orion_mdio_xsmi_write;
   305			break;
   306		}
   307	
   308		bus->name = "orion_mdio_bus";
   309		snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii",
   310			 dev_name(&pdev->dev));
   311		bus->parent = &pdev->dev;
   312	
   313		dev = bus->priv;
   314		dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
   315		if (!dev->regs) {
   316			dev_err(&pdev->dev, "Unable to remap SMI register\n");
   317			return -ENODEV;
   318		}
   319	
   320		init_waitqueue_head(&dev->smi_busy_wait);
   321	
   322		for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
   323			dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
 > 324			if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
   325				ret = -EPROBE_DEFER;
   326				goto out_clk;
   327			}
   328			if (IS_ERR(dev->clk[i]))
   329				break;
   330			clk_prepare_enable(dev->clk[i]);
   331		}
   332	
   333		if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
   334			dev_warn(dev, "unsupported number of clocks, limiting to the first "
   335				 __stringify(ARRAY_SIZE(dev->clk)) "\n");
   336	
   337		dev->err_interrupt = platform_get_irq(pdev, 0);
   338		if (dev->err_interrupt > 0 &&
   339		    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
   340			dev_err(&pdev->dev,
   341				"disabling interrupt, resource size is too small\n");
   342			dev->err_interrupt = 0;
   343		}
   344		if (dev->err_interrupt > 0) {
   345			ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
   346						orion_mdio_err_irq,
   347						IRQF_SHARED, pdev->name, dev);
   348			if (ret)
   349				goto out_mdio;
   350	
   351			writel(MVMDIO_ERR_INT_SMI_DONE,
   352				dev->regs + MVMDIO_ERR_INT_MASK);
   353	
   354		} else if (dev->err_interrupt == -EPROBE_DEFER) {
   355			ret = -EPROBE_DEFER;
   356			goto out_mdio;
   357		}
   358	
   359		ret = of_mdiobus_register(bus, pdev->dev.of_node);
   360		if (ret < 0) {
   361			dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
   362			goto out_mdio;
   363		}
   364	
   365		platform_set_drvdata(pdev, bus);
   366	
   367		return 0;
   368	
   369	out_mdio:
   370		if (dev->err_interrupt > 0)
   371			writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
   372	
   373	out_clk:
   374		for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
   375			if (IS_ERR(dev->clk[i]))
   376				break;
   377			clk_disable_unprepare(dev->clk[i]);
   378			clk_put(dev->clk[i]);
   379		}
   380	
   381		return ret;
   382	}
   383	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70930 bytes --]

Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Rob Herring]

On Sat, Jul 6, 2019 at 9:31 AM <josua@solid-run.com> wrote:
>
> From: Josua Mayer <josua@solid-run.com>
>
> Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clock to be specified.
>
> Cc: stable@vger.kernel.org
> Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> index 42cd81090a2c..3f3cfc1d8d4d 100644
> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> @@ -16,7 +16,7 @@ Required properties:
>
>  Optional properties:
>  - interrupts: interrupt line number for the SMI error/done interrupt
> -- clocks: phandle for up to three required clocks for the MDIO instance
> +- clocks: phandle for up to four required clocks for the MDIO instance

This needs to enumerate exactly what the clocks are. Shouldn't there
be an additional clock-names value too?

Rob

Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Andrew Lunn]

> >  Optional properties:
> >  - interrupts: interrupt line number for the SMI error/done interrupt
> > -- clocks: phandle for up to three required clocks for the MDIO instance
> > +- clocks: phandle for up to four required clocks for the MDIO instance
> 
> This needs to enumerate exactly what the clocks are. Shouldn't there
> be an additional clock-names value too?

Hi Rob

The driver does not care what they are called. It just turns them all
on, and turns them off again when removed.

    Andrew

[PATCH v2 0/4] Fix hang of Armada 8040 SoC in orion-mdio

[josua]

From: Josua Mayer <josua.mayer@jm0.eu>

With a modular kernel as configured by Debian a hang was observed with
the Armada 8040 SoC in the Clearfog GT and Macchiatobin boards.

The 8040 SoC actually requires four clocks to be enabled for the mdio
interface to function. All 4 clocks are already specified in
armada-cp110.dtsi. It has however been missed that the orion-mdio driver
only supports enabling up to three clocks.

This patch-set allows the orion-mdio driver to handle four clocks and
adds a warning when more clocks are specified to prevent this particular
oversight in the future.

Changes since v1:
- fixed condition for priting the warning (Andrew Lunn)
- rephrased commit description for deferred probing (Andrew Lunn)
- fixed compiler warnings (kbuild test robot)

Josua Mayer (4):
  dt-bindings: allow up to four clocks for orion-mdio
  net: mvmdio: allow up to four clocks to be specified for orion-mdio
  net: mvmdio: print warning when orion-mdio has too many clocks
  net: mvmdio: defer probe of orion-mdio if a clock is not ready

 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt |  2 +-
 drivers/net/ethernet/marvell/mvmdio.c                        | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.16.4

[PATCH v2 1/4] dt-bindings: allow up to four clocks for orion-mdio

[josua]

From: Josua Mayer <josua@solid-run.com>

Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
Update the binding to allow the extra clock to be specified.

Cc: stable@vger.kernel.org
Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 42cd81090a2c..3f3cfc1d8d4d 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -16,7 +16,7 @@ Required properties:
 
 Optional properties:
 - interrupts: interrupt line number for the SMI error/done interrupt
-- clocks: phandle for up to three required clocks for the MDIO instance
+- clocks: phandle for up to four required clocks for the MDIO instance
 
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
-- 
2.16.4

[PATCH v2 2/4] net: mvmdio: allow up to four clocks to be specified for orion-mdio

[josua]

From: Josua Mayer <josua@solid-run.com>

Allow up to four clocks to be specified and enabled for the orion-mdio
interface, which are required by the Armada 8k and defined in
armada-cp110.dtsi.

Fixes a hang in probing the mvmdio driver that was encountered on the
Clearfog GT 8K with all drivers built as modules, but also affects other
boards such as the MacchiatoBIN.

Cc: stable@vger.kernel.org
Fixes: 96cb43423822 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index c5dac6bd2be4..e17d563e97a6 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -64,7 +64,7 @@
 
 struct orion_mdio_dev {
 	void __iomem *regs;
-	struct clk *clk[3];
+	struct clk *clk[4];
 	/*
 	 * If we have access to the error interrupt pin (which is
 	 * somewhat misnamed as it not only reflects internal errors
-- 
2.16.4

[PATCH v2 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[josua]

From: Josua Mayer <josua@solid-run.com>

Print a warning when device tree specifies more than the maximum of four
clocks supported by orion-mdio. Because reading from mdio can lock up
the Armada 8k when a required clock is not initialized, it is important
to notify the user when a specified clock is ignored.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index e17d563e97a6..eba18065a4da 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		clk_prepare_enable(dev->clk[i]);
 	}
 
+	if (!IS_ERR(of_clk_get(pdev->dev.of_node, ARRAY_SIZE(dev->clk))))
+		dev_warn(&pdev->dev, "unsupported number of clocks, limiting to the first "
+			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
+
 	dev->err_interrupt = platform_get_irq(pdev, 0);
 	if (dev->err_interrupt > 0 &&
 	    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
-- 
2.16.4

[PATCH v2 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready

[josua]

From: Josua Mayer <josua@solid-run.com>

Defer probing of the orion-mdio interface when getting a clock returns
EPROBE_DEFER. This avoids locking up the Armada 8k SoC when mdio is used
before all clocks have been enabled.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index eba18065a4da..f660cc2b8258 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -321,6 +321,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
+		if (PTR_ERR(dev->clk[i]) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto out_clk;
+		}
 		if (IS_ERR(dev->clk[i]))
 			break;
 		clk_prepare_enable(dev->clk[i]);
@@ -366,6 +370,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	if (dev->err_interrupt > 0)
 		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 
+out_clk:
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		if (IS_ERR(dev->clk[i]))
 			break;
-- 
2.16.4

Re: [PATCH v2 3/4] net: mvmdio: print warning when orion-mdio has too many clocks

[Andrew Lunn]

On Tue, Jul 09, 2019 at 03:01:00PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Print a warning when device tree specifies more than the maximum of four
> clocks supported by orion-mdio. Because reading from mdio can lock up
> the Armada 8k when a required clock is not initialized, it is important
> to notify the user when a specified clock is ignored.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

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

    Andrew

Re: [PATCH v2 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready

[Andrew Lunn]

On Tue, Jul 09, 2019 at 03:01:01PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Defer probing of the orion-mdio interface when getting a clock returns
> EPROBE_DEFER. This avoids locking up the Armada 8k SoC when mdio is used
> before all clocks have been enabled.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

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

    Andrew

Re: [PATCH v2 0/4] Fix hang of Armada 8040 SoC in orion-mdio

[David Miller]

From: josua@solid-run.com
Date: Tue,  9 Jul 2019 15:00:57 +0200

> From: Josua Mayer <josua.mayer@jm0.eu>
> 
> With a modular kernel as configured by Debian a hang was observed with
> the Armada 8040 SoC in the Clearfog GT and Macchiatobin boards.
> 
> The 8040 SoC actually requires four clocks to be enabled for the mdio
> interface to function. All 4 clocks are already specified in
> armada-cp110.dtsi. It has however been missed that the orion-mdio driver
> only supports enabling up to three clocks.
> 
> This patch-set allows the orion-mdio driver to handle four clocks and
> adds a warning when more clocks are specified to prevent this particular
> oversight in the future.
> 
> Changes since v1:
> - fixed condition for priting the warning (Andrew Lunn)
> - rephrased commit description for deferred probing (Andrew Lunn)
> - fixed compiler warnings (kbuild test robot)

Series applied, thanks Josua.

Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Rob Herring]

On Mon, Jul 8, 2019 at 8:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > >  Optional properties:
> > >  - interrupts: interrupt line number for the SMI error/done interrupt
> > > -- clocks: phandle for up to three required clocks for the MDIO instance
> > > +- clocks: phandle for up to four required clocks for the MDIO instance
> >
> > This needs to enumerate exactly what the clocks are. Shouldn't there
> > be an additional clock-names value too?
>
> Hi Rob
>
> The driver does not care what they are called. It just turns them all
> on, and turns them off again when removed.

That's fine for the driver to do, but this is the hardware description.

It's not just what they are called, but how many too. Is 1 clock in
the DT valid? 0? It would be unusual for a given piece of h/w to
function with a variable number of clocks.

Rob

Re: [PATCH v2 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Rob Herring]

On Tue, Jul 9, 2019 at 7:13 AM <josua@solid-run.com> wrote:
>
> From: Josua Mayer <josua@solid-run.com>
>
> Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clock to be specified.
>
> Cc: stable@vger.kernel.org
> Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please don't resend patches when the last one is still under discussion.

Rob

Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio

[Andrew Lunn]

On Tue, Jul 09, 2019 at 04:03:28PM -0600, Rob Herring wrote:
> On Mon, Jul 8, 2019 at 8:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > >  Optional properties:
> > > >  - interrupts: interrupt line number for the SMI error/done interrupt
> > > > -- clocks: phandle for up to three required clocks for the MDIO instance
> > > > +- clocks: phandle for up to four required clocks for the MDIO instance
> > >
> > > This needs to enumerate exactly what the clocks are. Shouldn't there
> > > be an additional clock-names value too?
> >
> > Hi Rob
> >
> > The driver does not care what they are called. It just turns them all
> > on, and turns them off again when removed.
> 
> That's fine for the driver to do, but this is the hardware description.
> 
> It's not just what they are called, but how many too. Is 1 clock in
> the DT valid? 0? It would be unusual for a given piece of h/w to
> function with a variable number of clocks.

Hi Rob

The orion5x has 0 clocks. kirkwood, dove, Armada XP, 370 375, 380
has 1 clock. Armada 37xx has 4.

So yes, 1 clock is valid. 0 clocks is also valid. The piece of
hardware itself does not care how many clocks are feeding it, so long
as they are all turned on.

   Andrew