[PATCH v2 0/4] introduce devm_mdiobus_alloc/free and clean up davinci mdio

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2 0/4] introduce devm_mdiobus_alloc/free and clean up davinci mdio

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.

Clean up Davinci MDIO driver and use new devm API.

Changes in v2:
- minor comments taken into account
- additional patches added for cleaning up Davinci MDIO driver

Cc: Florian Fainelli <[hidden email]>

Grygorii Strashko (4):
  mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
  net: davinci_mdio: use devm_* api
  net: davinci_mdio: drop pinctrl_pm_select_default_state from probe
  net: davinci_mdio: simplify IO memory mapping

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

--
1.7.9.5

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2 2/4] net: davinci_mdio: use devm_* api

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

Cc: Florian Fainelli <[hidden email]>
Signed-off-by: Grygorii Strashko <[hidden email]>
---
 drivers/net/ethernet/ti/davinci_mdio.c |   24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..eda4946 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;
 }
 
@@ -423,18 +415,12 @@ static int davinci_mdio_remove(struct platform_device *pdev)
 {
  struct davinci_mdio_data *data = platform_get_drvdata(pdev);
 
- if (data->bus) {
+ 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

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2 3/4] net: davinci_mdio: drop pinctrl_pm_select_default_state from probe

Grygorii Strashko
In reply to this post by Grygorii Strashko
The "default" pinctrl state is set by Drivers core now before
calling the driver's probe.
Hence, it's safe to drop pinctrl_pm_select_default_state() call
from Davinci mdio driver probe.

Cc: Florian Fainelli <[hidden email]>
Cs: Linus Walleij <[hidden email]>
Signed-off-by: Grygorii Strashko <[hidden email]>
---
 drivers/net/ethernet/ti/davinci_mdio.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index eda4946..98d53a3 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -348,9 +348,6 @@ static int davinci_mdio_probe(struct platform_device *pdev)
  data->bus->parent = dev;
  data->bus->priv = data;
 
- /* Select default pin state */
- pinctrl_pm_select_default_state(&pdev->dev);
-
  pm_runtime_enable(&pdev->dev);
  pm_runtime_get_sync(&pdev->dev);
  data->clk = devm_clk_get(dev, "fck");
--
1.7.9.5

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

Grygorii Strashko
In reply to this post by 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.

Cc: Florian Fainelli <[hidden email]>
Signed-off-by: Grygorii Strashko <[hidden email]>
---
 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..6412beb 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 (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

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 4/4] net: davinci_mdio: simplify IO memory mapping

Grygorii Strashko
In reply to this post by Grygorii Strashko
Simplify IO memory mapping by using devm_ioremap_resource()
which will do all errors handling and reporting for us.

Cc: Florian Fainelli <[hidden email]>
Signed-off-by: Grygorii Strashko <[hidden email]>
---
 drivers/net/ethernet/ti/davinci_mdio.c |   21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 98d53a3..87e9f20 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -363,24 +363,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
  spin_lock_init(&data->lock);
 
  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, "could not find register map resource\n");
- ret = -ENOENT;
- goto bail_out;
- }
-
- res = devm_request_mem_region(dev, res->start, resource_size(res),
-    dev_name(dev));
- if (!res) {
- dev_err(dev, "could not allocate register map resource\n");
- ret = -ENXIO;
- goto bail_out;
- }
-
- data->regs = devm_ioremap_nocache(dev, res->start, resource_size(res));
- if (!data->regs) {
- dev_err(dev, "could not map mdio registers\n");
- ret = -ENOMEM;
+ data->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data->regs)) {
+ ret = PTR_ERR(data->regs);
  goto bail_out;
  }
 
--
1.7.9.5

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

Sergei Shtylyov-3
In reply to this post by Grygorii Strashko
Hello.

On 04/18/2014 09:24 PM, 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.

> Cc: Florian Fainelli <[hidden email]>
> Signed-off-by: Grygorii Strashko <[hidden email]>
[...]

> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 76f54b3..6412beb 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)
[...]

> +/**
> + * 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 re-align this line, so that it starts right under 'dev' on the
previous line.

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

Sergei Shtylyov-3
In reply to this post by Grygorii Strashko
On 04/18/2014 09:24 PM, 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.

> Cc: Florian Fainelli <[hidden email]>
> Signed-off-by: Grygorii Strashko <[hidden email]>
[...]

> index 76f54b3..6412beb 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)
[...]

> +/**
> + * 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);

    Since the wrapped function is called mdiobus_alloc_size(), not
mdiobus_alloc(), perhaps it's better to call the wrapper
devm_mdiobus_alloc_size()?

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2 0/4] introduce devm_mdiobus_alloc/free and clean up davinci mdio

Lad, Prabhakar
In reply to this post by Grygorii Strashko
Hi Grygorii,

Thanks for the patches!


On Fri, Apr 18, 2014 at 10:54 PM, Grygorii Strashko
<[hidden email]> wrote:

> 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.
>
> Clean up Davinci MDIO driver and use new devm API.
>
> Changes in v2:
> - minor comments taken into account
> - additional patches added for cleaning up Davinci MDIO driver
>
> Cc: Florian Fainelli <[hidden email]>
>
> Grygorii Strashko (4):
>   mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
>   net: davinci_mdio: use devm_* api
>   net: davinci_mdio: drop pinctrl_pm_select_default_state from probe
>   net: davinci_mdio: simplify IO memory mapping
>
Tested on DA850EVM.

Acked-and-tested-by: Lad, Prabhakar <[hidden email]>

Regards,
--Prabhakar Lad

>  Documentation/driver-model/devres.txt  |    4 ++
>  drivers/net/ethernet/ti/davinci_mdio.c |   48 ++++------------------
>  drivers/net/phy/mdio_bus.c             |   68 ++++++++++++++++++++++++++++++++
>  include/linux/phy.h                    |    2 +
>  4 files changed, 82 insertions(+), 40 deletions(-)
>
> --
> 1.7.9.5
>
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

Grygorii Strashko
In reply to this post by Sergei Shtylyov-3
On 04/18/2014 08:52 PM, Sergei Shtylyov wrote:

> On 04/18/2014 09:24 PM, 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.
>
>> Cc: Florian Fainelli <[hidden email]>
>> Signed-off-by: Grygorii Strashko <[hidden email]>
> [...]
>
>> index 76f54b3..6412beb 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)
> [...]
>> +/**
>> + * 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);
>
>     Since the wrapped function is called mdiobus_alloc_size(), not
> mdiobus_alloc(), perhaps it's better to call the wrapper
> devm_mdiobus_alloc_size()?

Agree. I've just sent v3.

Regards,
-grygorii

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Loading...