All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
  2014-04-04 13:40   ` Grygorii Strashko
  (?)
@ 2014-04-04 13:15   ` Sergei Shtylyov
  2014-04-08  9:52       ` Grygorii Strashko
  -1 siblings, 1 reply; 13+ messages in thread
From: Sergei Shtylyov @ 2014-04-04 13:15 UTC (permalink / raw)
  To: Grygorii Strashko, Florian Fainelli, netdev
  Cc: davinci-linux-open-source, linux-doc, Randy Dunlap, linux-kernel,
	santosh.shilimkar, David S. Miller, Jonathan Cameron

Hello.

On 04-04-2014 17:40, Grygorii Strashko wrote:

> Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
> to automatically clean up MDIO bus alocations made by MDIO drivers,
> thus leading to simplified MDIO drivers code.

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[...]

> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 76f54b3..fdcd6d1 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
>   }
>   EXPORT_SYMBOL(mdiobus_alloc_size);
>
> +static void _devm_mdiobus_free(struct device *dev, void *res)
> +{
> +	mdiobus_free(*(struct mii_bus **)res);
> +}
> +
> +static int devm_mdiobus_match(struct device *dev, void *res, void *data)
> +{
> +	struct mii_bus **r = res;

    Please insert an empty line after declaration.

> +	if (!r || !*r) {
> +		WARN_ON(!r || !*r);

    Hm, why we need the duplicate check This condition is always true.

[...]
> +/**
> + * devm_mdiobus_free - Resource-managed mdiobus_free()
> + * @dev:		Device this mii_bus belongs to
> + * @bus:		the mii_bus associated with the device
> + *
> + * Free mii_bus allocated with devm_mdiobus_alloc().
> + */
> +void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
> +{
> +	int rc;
> +
> +	rc = devres_release(dev, _devm_mdiobus_free,
> +		devm_mdiobus_match, bus);

    Please make this line start under 'dev', according to the networking 
coding style.

[...]

WBR, Sergei


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] net: davinci_mdio: use devm_* api
  2014-04-04 13:40   ` Grygorii Strashko
  (?)
@ 2014-04-04 13:18   ` Sergei Shtylyov
  -1 siblings, 0 replies; 13+ messages in thread
From: Sergei Shtylyov @ 2014-04-04 13:18 UTC (permalink / raw)
  To: Grygorii Strashko, Florian Fainelli, netdev
  Cc: davinci-linux-open-source, linux-doc, Randy Dunlap, linux-kernel,
	santosh.shilimkar, David S. Miller, Jonathan Cameron

On 04-04-2014 17:40, Grygorii Strashko wrote:

> Use devm_* API for memory allocation and to get device's clock
> to simplify driver's code.

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   drivers/net/ethernet/ti/davinci_mdio.c |   21 ++++-----------------
>   1 file changed, 4 insertions(+), 17 deletions(-)

> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 0cca9de..f0f7128 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
[...]
> @@ -425,16 +417,11 @@ static int davinci_mdio_remove(struct platform_device *pdev)
>
>   	if (data->bus) {
>   		mdiobus_unregister(data->bus);
> -		mdiobus_free(data->bus);
>   	}

    Remove {} please, it's not needed anymore, according to 
Documentation/CodingStyle.

WBR, Sergei


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-04 13:40 ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: Randy Dunlap, Jonathan Cameron, David S. Miller,
	prabhakar.csengg, santosh.shilimkar, Sekhar Nori, linux-doc,
	linux-kernel, davinci-linux-open-source, Grygorii Strashko

Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.

Update Davinci MDIO driver ss example of new devm APIs usage.

Grygorii Strashko (2):
  mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
  net: davinci_mdio: use devm_* api

 Documentation/driver-model/devres.txt  |    4 ++
 drivers/net/ethernet/ti/davinci_mdio.c |   21 ++--------
 drivers/net/phy/mdio_bus.c             |   68 ++++++++++++++++++++++++++++++++
 include/linux/phy.h                    |    2 +
 4 files changed, 78 insertions(+), 17 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-04 13:40 ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	santosh.shilimkar-l0cyMroinI0, David S. Miller, Jonathan Cameron

Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.

Update Davinci MDIO driver ss example of new devm APIs usage.

Grygorii Strashko (2):
  mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
  net: davinci_mdio: use devm_* api

 Documentation/driver-model/devres.txt  |    4 ++
 drivers/net/ethernet/ti/davinci_mdio.c |   21 ++--------
 drivers/net/phy/mdio_bus.c             |   68 ++++++++++++++++++++++++++++++++
 include/linux/phy.h                    |    2 +
 4 files changed, 78 insertions(+), 17 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-04 13:40   ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: Randy Dunlap, Jonathan Cameron, David S. Miller,
	prabhakar.csengg, santosh.shilimkar, Sekhar Nori, linux-doc,
	linux-kernel, davinci-linux-open-source, Grygorii Strashko

Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/driver-model/devres.txt |    4 ++
 drivers/net/phy/mdio_bus.c            |   68 +++++++++++++++++++++++++++++++++
 include/linux/phy.h                   |    2 +
 3 files changed, 74 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 4f7897e..31aa066 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -308,3 +308,7 @@ SLAVE DMA ENGINE
 
 SPI
   devm_spi_register_master()
+
+MDIO
+  devm_mdiobus_alloc()
+  devm_mdiobus_free()
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..fdcd6d1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
 }
 EXPORT_SYMBOL(mdiobus_alloc_size);
 
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+	mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+	struct mii_bus **r = res;
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+	return *r == data;
+}
+
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev:		Device to allocate mii_bus for
+ * @sizeof_priv:	Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+	struct mii_bus **ptr, *bus;
+
+	ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	/* use raw alloc_dr for kmalloc caller tracing */
+	bus = mdiobus_alloc_size(sizeof_priv);
+	if (bus) {
+		*ptr = bus;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
+
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev:		Device this mii_bus belongs to
+ * @bus:		the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+	int rc;
+
+	rc = devres_release(dev, _devm_mdiobus_free,
+		devm_mdiobus_match, bus);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_free);
+
 /**
  * mdiobus_release - mii_bus device release callback
  * @d: the target struct device that contains the mii_bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24126c4..2238bea 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -195,6 +195,8 @@ static inline struct mii_bus *mdiobus_alloc(void)
 int mdiobus_register(struct mii_bus *bus);
 void mdiobus_unregister(struct mii_bus *bus);
 void mdiobus_free(struct mii_bus *bus);
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv);
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
 struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
 int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
 int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-04 13:40   ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	santosh.shilimkar-l0cyMroinI0, David S. Miller, Jonathan Cameron

Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.

Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 Documentation/driver-model/devres.txt |    4 ++
 drivers/net/phy/mdio_bus.c            |   68 +++++++++++++++++++++++++++++++++
 include/linux/phy.h                   |    2 +
 3 files changed, 74 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 4f7897e..31aa066 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -308,3 +308,7 @@ SLAVE DMA ENGINE
 
 SPI
   devm_spi_register_master()
+
+MDIO
+  devm_mdiobus_alloc()
+  devm_mdiobus_free()
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..fdcd6d1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
 }
 EXPORT_SYMBOL(mdiobus_alloc_size);
 
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+	mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+	struct mii_bus **r = res;
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+	return *r == data;
+}
+
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev:		Device to allocate mii_bus for
+ * @sizeof_priv:	Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+	struct mii_bus **ptr, *bus;
+
+	ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	/* use raw alloc_dr for kmalloc caller tracing */
+	bus = mdiobus_alloc_size(sizeof_priv);
+	if (bus) {
+		*ptr = bus;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return bus;
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
+
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev:		Device this mii_bus belongs to
+ * @bus:		the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+	int rc;
+
+	rc = devres_release(dev, _devm_mdiobus_free,
+		devm_mdiobus_match, bus);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_mdiobus_free);
+
 /**
  * mdiobus_release - mii_bus device release callback
  * @d: the target struct device that contains the mii_bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24126c4..2238bea 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -195,6 +195,8 @@ static inline struct mii_bus *mdiobus_alloc(void)
 int mdiobus_register(struct mii_bus *bus);
 void mdiobus_unregister(struct mii_bus *bus);
 void mdiobus_free(struct mii_bus *bus);
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv);
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
 struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
 int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
 int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] net: davinci_mdio: use devm_* api
@ 2014-04-04 13:40   ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: Randy Dunlap, Jonathan Cameron, David S. Miller,
	prabhakar.csengg, santosh.shilimkar, Sekhar Nori, linux-doc,
	linux-kernel, davinci-linux-open-source, Grygorii Strashko

Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/davinci_mdio.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..f0f7128 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -321,15 +321,14 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	struct phy_device *phy;
 	int ret, addr;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->bus = mdiobus_alloc();
+	data->bus = devm_mdiobus_alloc(dev, 0);
 	if (!data->bus) {
 		dev_err(dev, "failed to alloc mii bus\n");
-		ret = -ENOMEM;
-		goto bail_out;
+		return -ENOMEM;
 	}
 
 	if (dev->of_node) {
@@ -354,7 +353,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
-	data->clk = clk_get(&pdev->dev, "fck");
+	data->clk = devm_clk_get(dev, "fck");
 	if (IS_ERR(data->clk)) {
 		dev_err(dev, "failed to get device clock\n");
 		ret = PTR_ERR(data->clk);
@@ -406,16 +405,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	return 0;
 
 bail_out:
-	if (data->bus)
-		mdiobus_free(data->bus);
-
-	if (data->clk)
-		clk_put(data->clk);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	kfree(data);
-
 	return ret;
 }
 
@@ -425,16 +417,11 @@ static int davinci_mdio_remove(struct platform_device *pdev)
 
 	if (data->bus) {
 		mdiobus_unregister(data->bus);
-		mdiobus_free(data->bus);
 	}
 
-	if (data->clk)
-		clk_put(data->clk);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	kfree(data);
-
 	return 0;
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] net: davinci_mdio: use devm_* api
@ 2014-04-04 13:40   ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-04 13:40 UTC (permalink / raw)
  To: Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	santosh.shilimkar-l0cyMroinI0, David S. Miller, Jonathan Cameron

Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.

Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 drivers/net/ethernet/ti/davinci_mdio.c |   21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..f0f7128 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -321,15 +321,14 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	struct phy_device *phy;
 	int ret, addr;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->bus = mdiobus_alloc();
+	data->bus = devm_mdiobus_alloc(dev, 0);
 	if (!data->bus) {
 		dev_err(dev, "failed to alloc mii bus\n");
-		ret = -ENOMEM;
-		goto bail_out;
+		return -ENOMEM;
 	}
 
 	if (dev->of_node) {
@@ -354,7 +353,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
-	data->clk = clk_get(&pdev->dev, "fck");
+	data->clk = devm_clk_get(dev, "fck");
 	if (IS_ERR(data->clk)) {
 		dev_err(dev, "failed to get device clock\n");
 		ret = PTR_ERR(data->clk);
@@ -406,16 +405,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	return 0;
 
 bail_out:
-	if (data->bus)
-		mdiobus_free(data->bus);
-
-	if (data->clk)
-		clk_put(data->clk);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	kfree(data);
-
 	return ret;
 }
 
@@ -425,16 +417,11 @@ static int davinci_mdio_remove(struct platform_device *pdev)
 
 	if (data->bus) {
 		mdiobus_unregister(data->bus);
-		mdiobus_free(data->bus);
 	}
 
-	if (data->clk)
-		clk_put(data->clk);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	kfree(data);
-
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-08  9:52       ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-08  9:52 UTC (permalink / raw)
  To: Sergei Shtylyov, Florian Fainelli, netdev
  Cc: davinci-linux-open-source, linux-doc, Randy Dunlap, linux-kernel,
	santosh.shilimkar, David S. Miller, Jonathan Cameron

On 04/04/2014 04:15 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 04-04-2014 17:40, Grygorii Strashko wrote:
>
>> Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
>> to automatically clean up MDIO bus alocations made by MDIO drivers,
>> thus leading to simplified MDIO drivers code.
>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> [...]
>
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 76f54b3..fdcd6d1 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
>>   }
>>   EXPORT_SYMBOL(mdiobus_alloc_size);
>>
>> +static void _devm_mdiobus_free(struct device *dev, void *res)
>> +{
>> +    mdiobus_free(*(struct mii_bus **)res);
>> +}
>> +
>> +static int devm_mdiobus_match(struct device *dev, void *res, void *data)
>> +{
>> +    struct mii_bus **r = res;
>
>     Please insert an empty line after declaration.
>
>> +    if (!r || !*r) {
>> +        WARN_ON(!r || !*r);
>
>     Hm, why we need the duplicate check This condition is always true.

It can be replaced with:
	if (WARN_ON(!r || !*r))
		return 0;


I will wait for additional comments, then resend.

Regards,
- grygorii


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-08  9:52       ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-08  9:52 UTC (permalink / raw)
  To: Sergei Shtylyov, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	santosh.shilimkar-l0cyMroinI0, David S. Miller, Jonathan Cameron

On 04/04/2014 04:15 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 04-04-2014 17:40, Grygorii Strashko wrote:
>
>> Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
>> to automatically clean up MDIO bus alocations made by MDIO drivers,
>> thus leading to simplified MDIO drivers code.
>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> [...]
>
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 76f54b3..fdcd6d1 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
>>   }
>>   EXPORT_SYMBOL(mdiobus_alloc_size);
>>
>> +static void _devm_mdiobus_free(struct device *dev, void *res)
>> +{
>> +    mdiobus_free(*(struct mii_bus **)res);
>> +}
>> +
>> +static int devm_mdiobus_match(struct device *dev, void *res, void *data)
>> +{
>> +    struct mii_bus **r = res;
>
>     Please insert an empty line after declaration.
>
>> +    if (!r || !*r) {
>> +        WARN_ON(!r || !*r);
>
>     Hm, why we need the duplicate check This condition is always true.

It can be replaced with:
	if (WARN_ON(!r || !*r))
		return 0;


I will wait for additional comments, then resend.

Regards,
- grygorii

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
  2014-04-04 13:40 ` Grygorii Strashko
                   ` (2 preceding siblings ...)
  (?)
@ 2014-04-11  4:55 ` Florian Fainelli
  2014-04-11 11:05     ` Grygorii Strashko
  -1 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2014-04-11  4:55 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: netdev, Randy Dunlap, Jonathan Cameron, David S. Miller,
	Prabhakar Lad, santosh.shilimkar, Sekhar Nori, linux-doc,
	linux-kernel, dlos

Hi Grygorii,

2014-04-04 6:40 GMT-07:00 Grygorii Strashko <grygorii.strashko@ti.com>:
> Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
> to automatically clean up MDIO bus alocations made by MDIO drivers,
> thus leading to simplified MDIO drivers code.
>
> Update Davinci MDIO driver ss example of new devm APIs usage.

This does look good at first glance. net-next is currently closed at
the moment, so this will have to be merged later.

At some point, we might also want to handle the mdio_bus irq array, as
that one is also usually dynamically allocated. Maybe we could just do
a static irq[PHY_MAX_ADDR] allocation, 32 times the size of an integer
might not be worth a potential leak.

>
> Grygorii Strashko (2):
>   mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
>   net: davinci_mdio: use devm_* api
>
>  Documentation/driver-model/devres.txt  |    4 ++
>  drivers/net/ethernet/ti/davinci_mdio.c |   21 ++--------
>  drivers/net/phy/mdio_bus.c             |   68 ++++++++++++++++++++++++++++++++
>  include/linux/phy.h                    |    2 +
>  4 files changed, 78 insertions(+), 17 deletions(-)
>
> --
> 1.7.9.5
>



-- 
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-11 11:05     ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-11 11:05 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Randy Dunlap, Jonathan Cameron, David S. Miller,
	Prabhakar Lad, santosh.shilimkar, Sekhar Nori, linux-doc,
	linux-kernel, dlos

Hi Florian,
On 04/11/2014 07:55 AM, Florian Fainelli wrote:
> Hi Grygorii,
> 
> 2014-04-04 6:40 GMT-07:00 Grygorii Strashko <grygorii.strashko@ti.com>:
>> Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
>> to automatically clean up MDIO bus alocations made by MDIO drivers,
>> thus leading to simplified MDIO drivers code.
>>
>> Update Davinci MDIO driver ss example of new devm APIs usage.
> 
> This does look good at first glance. net-next is currently closed at
> the moment, so this will have to be merged later.

Thanks. It can wait for 3.16, so I'll update & resend after rc1.
I have few more patches for davinci_mdio.c, so
my intention here was to check if I can base them on top of new API or not :)

> 
> At some point, we might also want to handle the mdio_bus irq array, as
> that one is also usually dynamically allocated. Maybe we could just do
> a static irq[PHY_MAX_ADDR] allocation, 32 times the size of an integer
> might not be worth a potential leak.

It sounds good, but first of all irq field of mii_bus structure has to be made private. 
And drivers have to use getter/setters to access it - then its type can be changes 
simply and safely.

By the way, mdiobus_register() can be handled using DEVM approach too, but it 
will a bit more complex.

[...]
> 

Regards,
- grygorii


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free
@ 2014-04-11 11:05     ` Grygorii Strashko
  0 siblings, 0 replies; 13+ messages in thread
From: Grygorii Strashko @ 2014-04-11 11:05 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: dlos, linux-doc-u79uwXL29TY76Z2rM5mHXA, netdev, Randy Dunlap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	santosh.shilimkar-l0cyMroinI0, David S. Miller, Jonathan Cameron

Hi Florian,
On 04/11/2014 07:55 AM, Florian Fainelli wrote:
> Hi Grygorii,
> 
> 2014-04-04 6:40 GMT-07:00 Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>:
>> Introduce a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
>> to automatically clean up MDIO bus alocations made by MDIO drivers,
>> thus leading to simplified MDIO drivers code.
>>
>> Update Davinci MDIO driver ss example of new devm APIs usage.
> 
> This does look good at first glance. net-next is currently closed at
> the moment, so this will have to be merged later.

Thanks. It can wait for 3.16, so I'll update & resend after rc1.
I have few more patches for davinci_mdio.c, so
my intention here was to check if I can base them on top of new API or not :)

> 
> At some point, we might also want to handle the mdio_bus irq array, as
> that one is also usually dynamically allocated. Maybe we could just do
> a static irq[PHY_MAX_ADDR] allocation, 32 times the size of an integer
> might not be worth a potential leak.

It sounds good, but first of all irq field of mii_bus structure has to be made private. 
And drivers have to use getter/setters to access it - then its type can be changes 
simply and safely.

By the way, mdiobus_register() can be handled using DEVM approach too, but it 
will a bit more complex.

[...]
> 

Regards,
- grygorii

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-04-11 11:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-04 13:40 [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free Grygorii Strashko
2014-04-04 13:40 ` Grygorii Strashko
2014-04-04 13:40 ` [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free Grygorii Strashko
2014-04-04 13:40   ` Grygorii Strashko
2014-04-04 13:15   ` Sergei Shtylyov
2014-04-08  9:52     ` Grygorii Strashko
2014-04-08  9:52       ` Grygorii Strashko
2014-04-04 13:40 ` [PATCH 2/2] net: davinci_mdio: use devm_* api Grygorii Strashko
2014-04-04 13:40   ` Grygorii Strashko
2014-04-04 13:18   ` Sergei Shtylyov
2014-04-11  4:55 ` [PATCH 0/2] introduce devm_mdiobus_alloc/devm_mdiobus_free Florian Fainelli
2014-04-11 11:05   ` Grygorii Strashko
2014-04-11 11:05     ` Grygorii Strashko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.