[PATCH 0/5] net: mdio-mux: Misc fix

[PATCH 0/5] net: mdio-mux: Misc fix

[Corentin Labbe]

Hello

This patch series fix minor problems found when working on the
dwmac-sun8i syscon mdio-mux.

Regards

Corentin Labbe (5):
  net: mdio-mux: Fix NULL Comparison style
  net: mdio-mux: Remove unnecessary 'out of memory' message
  net: mdio-mux: printing driver version is useless
  net: mdio-mux-mmioreg: Can handle 8/16/32 bits registers
  net: mdio-mux: fix unbalanced put_device

 drivers/net/phy/Kconfig    |  2 +-
 drivers/net/phy/mdio-mux.c | 15 ++++-----------
 2 files changed, 5 insertions(+), 12 deletions(-)

-- 
2.13.5

[PATCH 1/5] net: mdio-mux: Fix NULL Comparison style

[Corentin Labbe]

This patch fix checkpatch warning about NULL Comparison style.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 942ceaf3fd3f..b18ad7082b88 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -120,7 +120,7 @@ int mdio_mux_init(struct device *dev,
 	}
 
 	pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
-	if (pb == NULL) {
+	if (!pb) {
 		ret_val = -ENOMEM;
 		goto err_pb_kz;
 	}
@@ -144,7 +144,7 @@ int mdio_mux_init(struct device *dev,
 		}
 
 		cb = devm_kzalloc(dev, sizeof(*cb), GFP_KERNEL);
-		if (cb == NULL) {
+		if (!cb) {
 			dev_err(dev,
 				"Error: Failed to allocate memory for child %pOF\n",
 				child_bus_node);
-- 
2.13.5

Re: [PATCH 1/5] net: mdio-mux: Fix NULL Comparison style

[Andrew Lunn]

On Wed, Aug 30, 2017 at 07:46:47PM +0200, Corentin Labbe wrote:
> This patch fix checkpatch warning about NULL Comparison style.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

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

    Andrew

[PATCH 2/5] net: mdio-mux: Remove unnecessary 'out of memory' message

[Corentin Labbe]

This patch fix checkpatch warning about unnecessary 'out of memory'
message.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index b18ad7082b88..5e08e89465c5 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -145,9 +145,6 @@ int mdio_mux_init(struct device *dev,
 
 		cb = devm_kzalloc(dev, sizeof(*cb), GFP_KERNEL);
 		if (!cb) {
-			dev_err(dev,
-				"Error: Failed to allocate memory for child %pOF\n",
-				child_bus_node);
 			ret_val = -ENOMEM;
 			continue;
 		}
@@ -156,9 +153,6 @@ int mdio_mux_init(struct device *dev,
 
 		cb->mii_bus = mdiobus_alloc();
 		if (!cb->mii_bus) {
-			dev_err(dev,
-				"Error: Failed to allocate MDIO bus for child %pOF\n",
-				child_bus_node);
 			ret_val = -ENOMEM;
 			devm_kfree(dev, cb);
 			continue;
-- 
2.13.5

Re: [PATCH 2/5] net: mdio-mux: Remove unnecessary 'out of memory' message

[Andrew Lunn]

On Wed, Aug 30, 2017 at 07:46:48PM +0200, Corentin Labbe wrote:
> This patch fix checkpatch warning about unnecessary 'out of memory'
> message.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

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

    Andrew

[PATCH 3/5] net: mdio-mux: printing driver version is useless

[Corentin Labbe]

Remove the driver version information because this information
is not useful in an upstream kernel driver.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 5e08e89465c5..9a889a529b9c 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -179,7 +179,6 @@ int mdio_mux_init(struct device *dev,
 	}
 	if (pb->children) {
 		*mux_handle = pb;
-		dev_info(dev, "Version " DRV_VERSION "\n");
 		return 0;
 	}
 
-- 
2.13.5

Re: [PATCH 3/5] net: mdio-mux: printing driver version is useless

[Andrew Lunn]

On Wed, Aug 30, 2017 at 07:46:49PM +0200, Corentin Labbe wrote:
> Remove the driver version information because this information
> is not useful in an upstream kernel driver.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Hi Corentin

You may as well remove DRV_VERSION from this file, not just here.

    Andrew

[PATCH 4/5] net: mdio-mux-mmioreg: Can handle 8/16/32 bits registers

[Corentin Labbe]

This patch fix an old information that mdio-mux-mmioreg can only handle
8bit registers.
This is not true anymore.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5afe6fdcc968..a9d16a3af514 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -85,7 +85,7 @@ config MDIO_BUS_MUX_MMIOREG
 	  parent bus.  Child bus selection is under the control of one of
 	  the FPGA's registers.
 
-	  Currently, only 8-bit registers are supported.
+	  Currently, only 8/16/32 bits registers are supported.
 
 config MDIO_CAVIUM
 	tristate
-- 
2.13.5

Re: [PATCH 4/5] net: mdio-mux-mmioreg: Can handle 8/16/32 bits registers

[Andrew Lunn]

On Wed, Aug 30, 2017 at 07:46:50PM +0200, Corentin Labbe wrote:
> This patch fix an old information that mdio-mux-mmioreg can only handle
> 8bit registers.
> This is not true anymore.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

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

    Andrew

[PATCH 5/5] net: mdio-mux: fix unbalanced put_device

[Corentin Labbe]

mdio_mux_uninit() call put_device (unconditionally) because of
of_mdio_find_bus() in mdio_mux_init.
But of_mdio_find_bus is only called if mux_bus is empty.
If mux_bus is set, mdio_mux_uninit will print a "refcount_t: underflow"
trace.

This patch add a get_device in the other branch of "if (mux_bus)".

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/phy/mdio-mux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 9a889a529b9c..0b7d3d30ae97 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -117,6 +117,7 @@ int mdio_mux_init(struct device *dev,
 	} else {
 		parent_bus_node = NULL;
 		parent_bus = mux_bus;
+		get_device(&parent_bus->dev);
 	}
 
 	pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
@@ -186,8 +187,7 @@ int mdio_mux_init(struct device *dev,
 	devm_kfree(dev, pb);
 err_pb_kz:
 	/* balance the reference of_mdio_find_bus() took */
-	if (!mux_bus)
-		put_device(&parent_bus->dev);
+	put_device(&parent_bus->dev);
 err_parent_bus:
 	of_node_put(parent_bus_node);
 	return ret_val;
-- 
2.13.5

Re: [PATCH 5/5] net: mdio-mux: fix unbalanced put_device

[Andrew Lunn]

>  err_pb_kz:
>  	/* balance the reference of_mdio_find_bus() took */

Hi Corentin

This comment should probably be updated as well. 

> -	if (!mux_bus)
> -		put_device(&parent_bus->dev);
> +	put_device(&parent_bus->dev);

  Thanks
     Andrew

Re: [PATCH 0/5] net: mdio-mux: Misc fix

[Andrew Lunn]

On Wed, Aug 30, 2017 at 07:46:46PM +0200, Corentin Labbe wrote:
> Hello
> 
> This patch series fix minor problems found when working on the
> dwmac-sun8i syscon mdio-mux.

Hi Corentin

In general, a nice patchset.

Looking at the code, there are a few calls to devm_kfree() which look
redundant. This one should probably stay:

	   	if (r) {
			mdiobus_free(cb->mii_bus);
			devm_kfree(dev, cb);
		} else {

but i think the others can go.

Just a suggestion, not a problem if you don't feel like doing this...

   Andrew

Re: [PATCH 0/5] net: mdio-mux: Misc fix

[Corentin Labbe]

On Wed, Aug 30, 2017 at 09:01:19PM +0200, Andrew Lunn wrote:
> On Wed, Aug 30, 2017 at 07:46:46PM +0200, Corentin Labbe wrote:
> > Hello
> > 
> > This patch series fix minor problems found when working on the
> > dwmac-sun8i syscon mdio-mux.
> 
> Hi Corentin
> 
> In geineral, a nice patchset.

Thanks, I will send a v2 with your suggestions.

> 
> Looking at the code, there are a few calls to devm_kfree() which look
> redundant. This one should probably stay:
> 
> 	   	if (r) {
> 			mdiobus_free(cb->mii_bus);
> 			devm_kfree(dev, cb);
> 		} else {
> 
> but i think the others can go.
> 
> Just a suggestion, not a problem if you don't feel like doing this...
> 

I will try but in another serie. For letting more time for test.

Regards