[PATCH 0/2] powerpc: gpio_mdio: Simplify gpio_mdio_probe()

[PATCH 0/2] powerpc: gpio_mdio: Simplify gpio_mdio_probe()

[Christophe JAILLET]

While wondering if it was correct to call mdiobus_free() in the remove
function and only kfree() in the error handling of the probe, I
arrived at the conclusion that the code could be simpler here.

Patch 1 uses mdiobus_alloc_size() instead of a hand written
mdiobus_alloc() + kzalloc(). it also uses the devm_ version in order to
save some LoC (and answer my initial question)

Patch 2 uses devm_of_mdiobus_register() to completly remove the .remove()
function and save some more LoC.

Both patches are compile tested only.

Christophe JAILLET (2):
  powerpc: gpio_mdio: Use devm_mdiobus_alloc_size()
  powerpc: gpio_mdio: Use devm_of_mdiobus_register()

 arch/powerpc/platforms/pasemi/gpio_mdio.c | 41 ++++-------------------
 1 file changed, 6 insertions(+), 35 deletions(-)

-- 
2.48.1

[PATCH 1/2] powerpc: gpio_mdio: Use devm_mdiobus_alloc_size()

[Christophe JAILLET]

Use mdiobus_alloc_size() instead of a hand written mdiobus_alloc() +
kzalloc().

This is less verbose and more robust. It also reduces memory fragmentation
and saves a few bytes of memory.

While at it, switch to devm_mdiobus_alloc_size() for extra simplification.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is compile tested only.

Some memory is saved because pahole states, on a x86_64,  that:

struct mii_bus {
	...
	/* size: 3640, cachelines: 57, members: 23 */
	/* sum members: 3633, holes: 2, sum holes: 7 */
	/* member types with holes: 3, total: 4, bit paddings: 1, total: 1 bit */
	/* paddings: 1, sum paddings: 3 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 4 */
	/* last cacheline: 56 bytes */
}

Because of the way allocation works, 4096 bytes are allocated. When
mdiobus_alloc_size() is used, struct gpio_priv fits in this "wasted" space
and so is available "for free".
---
 arch/powerpc/platforms/pasemi/gpio_mdio.c | 26 +++++------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c b/arch/powerpc/platforms/pasemi/gpio_mdio.c
index e4538d471256..2c54f5f063b7 100644
--- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
+++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
@@ -213,15 +213,11 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
 	const unsigned int *prop;
 	int err;
 
-	err = -ENOMEM;
-	priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
-	if (!priv)
-		goto out;
-
-	new_bus = mdiobus_alloc();
-
+	new_bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
 	if (!new_bus)
-		goto out_free_priv;
+		return -ENOMEM;
+
+	priv = new_bus->priv;
 
 	new_bus->name = "pasemi gpio mdio bus";
 	new_bus->read = &gpio_mdio_read;
@@ -230,7 +226,6 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
 
 	prop = of_get_property(np, "reg", NULL);
 	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", *prop);
-	new_bus->priv = priv;
 
 	prop = of_get_property(np, "mdc-pin", NULL);
 	priv->mdc_pin = *prop;
@@ -246,17 +241,10 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
 	if (err != 0) {
 		pr_err("%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
-		goto out_free_irq;
+		return err;
 	}
 
 	return 0;
-
-out_free_irq:
-	kfree(new_bus);
-out_free_priv:
-	kfree(priv);
-out:
-	return err;
 }
 
 
@@ -267,10 +255,6 @@ static void gpio_mdio_remove(struct platform_device *dev)
 	mdiobus_unregister(bus);
 
 	dev_set_drvdata(&dev->dev, NULL);
-
-	kfree(bus->priv);
-	bus->priv = NULL;
-	mdiobus_free(bus);
 }
 
 static const struct of_device_id gpio_mdio_match[] =
-- 
2.48.1

[PATCH 2/2] powerpc: gpio_mdio: Use devm_of_mdiobus_register()

[Christophe JAILLET]

Use devm_of_mdiobus_register() in order to remove the now empty .remove()
function.

Doing so dev_set_drvdata() is now also unneeded. Remove it as well.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is compile tested only.
---
 arch/powerpc/platforms/pasemi/gpio_mdio.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c b/arch/powerpc/platforms/pasemi/gpio_mdio.c
index 2c54f5f063b7..6712ccb84c0a 100644
--- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
+++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
@@ -234,10 +234,8 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
 	priv->mdio_pin = *prop;
 
 	new_bus->parent = dev;
-	dev_set_drvdata(dev, new_bus);
-
-	err = of_mdiobus_register(new_bus, np);
 
+	err = devm_of_mdiobus_register(dev, new_bus, np);
 	if (err != 0) {
 		pr_err("%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
@@ -247,16 +245,6 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
 	return 0;
 }
 
-
-static void gpio_mdio_remove(struct platform_device *dev)
-{
-	struct mii_bus *bus = dev_get_drvdata(&dev->dev);
-
-	mdiobus_unregister(bus);
-
-	dev_set_drvdata(&dev->dev, NULL);
-}
-
 static const struct of_device_id gpio_mdio_match[] =
 {
 	{
@@ -269,7 +257,6 @@ MODULE_DEVICE_TABLE(of, gpio_mdio_match);
 static struct platform_driver gpio_mdio_driver =
 {
 	.probe		= gpio_mdio_probe,
-	.remove		= gpio_mdio_remove,
 	.driver = {
 		.name = "gpio-mdio-bitbang",
 		.of_match_table = gpio_mdio_match,
-- 
2.48.1