* [PATCH net-next 0/2] net/davinci_emac updates
@ 2013-03-25 9:25 Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 1/2] net/davinci_emac: use devres APIs Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare} Sekhar Nori
0 siblings, 2 replies; 6+ messages in thread
From: Sekhar Nori @ 2013-03-25 9:25 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, davinci-linux-open-source, Sekhar Nori
This patch series converts davinci_emac driver to use
clk_prepare/unprepare in preparation towards common
clock migration for DaVinci. While at it, also introduce
a patch to start using devres APIs to simplify error
handling during probe.
Sekhar Nori (2):
net/davinci_emac: use devres APIs
net/davinci_emac: use clk_{prepare|unprepare}
drivers/net/ethernet/ti/davinci_emac.c | 55 ++++++++++++++++----------------
1 file changed, 28 insertions(+), 27 deletions(-)
--
1.7.10.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] net/davinci_emac: use devres APIs
2013-03-25 9:25 [PATCH net-next 0/2] net/davinci_emac updates Sekhar Nori
@ 2013-03-25 9:25 ` Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare} Sekhar Nori
1 sibling, 0 replies; 6+ messages in thread
From: Sekhar Nori @ 2013-03-25 9:25 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, davinci-linux-open-source, Sekhar Nori
Use devres APIs where possible to simplify error handling
in driver probe.
While at it, also rename the goto targets in error path to
introduce some consistency in how they are named.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 46 +++++++++++---------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index ae1b77a..b1349b5 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1865,21 +1865,18 @@ static int davinci_emac_probe(struct platform_device *pdev)
/* obtain emac clock from kernel */
- emac_clk = clk_get(&pdev->dev, NULL);
+ emac_clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(emac_clk)) {
dev_err(&pdev->dev, "failed to get EMAC clock\n");
return -EBUSY;
}
emac_bus_frequency = clk_get_rate(emac_clk);
- clk_put(emac_clk);
/* TODO: Probe PHY here if possible */
ndev = alloc_etherdev(sizeof(struct emac_priv));
- if (!ndev) {
- rc = -ENOMEM;
- goto no_ndev;
- }
+ if (!ndev)
+ return -ENOMEM;
platform_set_drvdata(pdev, ndev);
priv = netdev_priv(ndev);
@@ -1893,7 +1890,7 @@ static int davinci_emac_probe(struct platform_device *pdev)
if (!pdata) {
dev_err(&pdev->dev, "no platform data\n");
rc = -ENODEV;
- goto probe_quit;
+ goto no_pdata;
}
/* MAC addr and PHY mask , RMII enable info from platform_data */
@@ -1913,23 +1910,23 @@ static int davinci_emac_probe(struct platform_device *pdev)
if (!res) {
dev_err(&pdev->dev,"error getting res\n");
rc = -ENOENT;
- goto probe_quit;
+ goto no_pdata;
}
priv->emac_base_phys = res->start + pdata->ctrl_reg_offset;
size = resource_size(res);
- if (!request_mem_region(res->start, size, ndev->name)) {
+ if (!devm_request_mem_region(&pdev->dev, res->start,
+ size, ndev->name)) {
dev_err(&pdev->dev, "failed request_mem_region() for regs\n");
rc = -ENXIO;
- goto probe_quit;
+ goto no_pdata;
}
- priv->remap_addr = ioremap(res->start, size);
+ priv->remap_addr = devm_ioremap(&pdev->dev, res->start, size);
if (!priv->remap_addr) {
dev_err(&pdev->dev, "unable to map IO\n");
rc = -ENOMEM;
- release_mem_region(res->start, size);
- goto probe_quit;
+ goto no_pdata;
}
priv->emac_base = priv->remap_addr + pdata->ctrl_reg_offset;
ndev->base_addr = (unsigned long)priv->remap_addr;
@@ -1962,7 +1959,7 @@ static int davinci_emac_probe(struct platform_device *pdev)
if (!priv->dma) {
dev_err(&pdev->dev, "error initializing DMA\n");
rc = -ENOMEM;
- goto no_dma;
+ goto no_pdata;
}
priv->txchan = cpdma_chan_create(priv->dma, tx_chan_num(EMAC_DEF_TX_CH),
@@ -1971,14 +1968,14 @@ static int davinci_emac_probe(struct platform_device *pdev)
emac_rx_handler);
if (WARN_ON(!priv->txchan || !priv->rxchan)) {
rc = -ENOMEM;
- goto no_irq_res;
+ goto no_cpdma_chan;
}
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res) {
dev_err(&pdev->dev, "error getting irq res\n");
rc = -ENOENT;
- goto no_irq_res;
+ goto no_cpdma_chan;
}
ndev->irq = res->start;
@@ -2000,7 +1997,7 @@ static int davinci_emac_probe(struct platform_device *pdev)
if (rc) {
dev_err(&pdev->dev, "error in register_netdev\n");
rc = -ENODEV;
- goto no_irq_res;
+ goto no_cpdma_chan;
}
@@ -2015,20 +2012,14 @@ static int davinci_emac_probe(struct platform_device *pdev)
return 0;
-no_irq_res:
+no_cpdma_chan:
if (priv->txchan)
cpdma_chan_destroy(priv->txchan);
if (priv->rxchan)
cpdma_chan_destroy(priv->rxchan);
cpdma_ctlr_destroy(priv->dma);
-no_dma:
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
- iounmap(priv->remap_addr);
-
-probe_quit:
+no_pdata:
free_netdev(ndev);
-no_ndev:
return rc;
}
@@ -2041,14 +2032,12 @@ no_ndev:
*/
static int davinci_emac_remove(struct platform_device *pdev)
{
- struct resource *res;
struct net_device *ndev = platform_get_drvdata(pdev);
struct emac_priv *priv = netdev_priv(ndev);
dev_notice(&ndev->dev, "DaVinci EMAC: davinci_emac_remove()\n");
platform_set_drvdata(pdev, NULL);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (priv->txchan)
cpdma_chan_destroy(priv->txchan);
@@ -2056,10 +2045,7 @@ static int davinci_emac_remove(struct platform_device *pdev)
cpdma_chan_destroy(priv->rxchan);
cpdma_ctlr_destroy(priv->dma);
- release_mem_region(res->start, resource_size(res));
-
unregister_netdev(ndev);
- iounmap(priv->remap_addr);
free_netdev(ndev);
return 0;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare}
2013-03-25 9:25 [PATCH net-next 0/2] net/davinci_emac updates Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 1/2] net/davinci_emac: use devres APIs Sekhar Nori
@ 2013-03-25 9:25 ` Sekhar Nori
2013-03-25 16:16 ` Mike Turquette
1 sibling, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2013-03-25 9:25 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, davinci-linux-open-source, Sekhar Nori, Mike Turquette
Use clk_prepare()/clk_unprepare() in the driver since common
clock framework needs these to be called before clock is enabled.
This is in preparation of common clock framework migration
for DaVinci.
Cc: Mike Turquette <mturquette@linaro.org>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index b1349b5..436296c 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -350,6 +350,7 @@ struct emac_priv {
/*platform specific members*/
void (*int_enable) (void);
void (*int_disable) (void);
+ struct clk *clk;
};
/* EMAC TX Host Error description strings */
@@ -1870,19 +1871,29 @@ static int davinci_emac_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to get EMAC clock\n");
return -EBUSY;
}
+
+ rc = clk_prepare(emac_clk);
+ if (rc) {
+ dev_err(&pdev->dev, "emac clock prepare failed.\n");
+ return rc;
+ }
+
emac_bus_frequency = clk_get_rate(emac_clk);
/* TODO: Probe PHY here if possible */
ndev = alloc_etherdev(sizeof(struct emac_priv));
- if (!ndev)
- return -ENOMEM;
+ if (!ndev) {
+ rc = -ENOMEM;
+ goto no_etherdev;
+ }
platform_set_drvdata(pdev, ndev);
priv = netdev_priv(ndev);
priv->pdev = pdev;
priv->ndev = ndev;
priv->msg_enable = netif_msg_init(debug_level, DAVINCI_EMAC_DEBUG);
+ priv->clk = emac_clk;
spin_lock_init(&priv->lock);
@@ -2020,6 +2031,8 @@ no_cpdma_chan:
cpdma_ctlr_destroy(priv->dma);
no_pdata:
free_netdev(ndev);
+no_etherdev:
+ clk_unprepare(emac_clk);
return rc;
}
@@ -2048,6 +2061,8 @@ static int davinci_emac_remove(struct platform_device *pdev)
unregister_netdev(ndev);
free_netdev(ndev);
+ clk_unprepare(priv->clk);
+
return 0;
}
--
1.7.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare}
2013-03-25 9:25 ` [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare} Sekhar Nori
@ 2013-03-25 16:16 ` Mike Turquette
2013-03-26 10:05 ` Sekhar Nori
0 siblings, 1 reply; 6+ messages in thread
From: Mike Turquette @ 2013-03-25 16:16 UTC (permalink / raw)
To: Sekhar Nori, David S. Miller
Cc: netdev, davinci-linux-open-source, Sekhar Nori
Quoting Sekhar Nori (2013-03-25 02:25:47)
> Use clk_prepare()/clk_unprepare() in the driver since common
> clock framework needs these to be called before clock is enabled.
>
> This is in preparation of common clock framework migration
> for DaVinci.
>
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
> drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index b1349b5..436296c 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -350,6 +350,7 @@ struct emac_priv {
> /*platform specific members*/
> void (*int_enable) (void);
> void (*int_disable) (void);
> + struct clk *clk;
> };
>
> /* EMAC TX Host Error description strings */
> @@ -1870,19 +1871,29 @@ static int davinci_emac_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "failed to get EMAC clock\n");
> return -EBUSY;
> }
> +
> + rc = clk_prepare(emac_clk);
> + if (rc) {
> + dev_err(&pdev->dev, "emac clock prepare failed.\n");
> + return rc;
> + }
> +
Is clk_enable ever called for emac_clk here? I don't see it in the
diff. clk_prepare and clk_enable should usually be paired, even if
simply calling clk_prepare generates the clock signal that your device
needs.
Also this change only calls clk_prepare at probe-time and clk_unprepare
at remove-time. Preparing a clock can result in power consumption.
With that in mind should your implementation be more aggressive about
calling clk_{un}prepare?
Regards,
Mike
> emac_bus_frequency = clk_get_rate(emac_clk);
>
> /* TODO: Probe PHY here if possible */
>
> ndev = alloc_etherdev(sizeof(struct emac_priv));
> - if (!ndev)
> - return -ENOMEM;
> + if (!ndev) {
> + rc = -ENOMEM;
> + goto no_etherdev;
> + }
>
> platform_set_drvdata(pdev, ndev);
> priv = netdev_priv(ndev);
> priv->pdev = pdev;
> priv->ndev = ndev;
> priv->msg_enable = netif_msg_init(debug_level, DAVINCI_EMAC_DEBUG);
> + priv->clk = emac_clk;
>
> spin_lock_init(&priv->lock);
>
> @@ -2020,6 +2031,8 @@ no_cpdma_chan:
> cpdma_ctlr_destroy(priv->dma);
> no_pdata:
> free_netdev(ndev);
> +no_etherdev:
> + clk_unprepare(emac_clk);
> return rc;
> }
>
> @@ -2048,6 +2061,8 @@ static int davinci_emac_remove(struct platform_device *pdev)
> unregister_netdev(ndev);
> free_netdev(ndev);
>
> + clk_unprepare(priv->clk);
> +
> return 0;
> }
>
> --
> 1.7.10.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare}
2013-03-25 16:16 ` Mike Turquette
@ 2013-03-26 10:05 ` Sekhar Nori
2013-03-26 16:29 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2013-03-26 10:05 UTC (permalink / raw)
To: Mike Turquette
Cc: David S. Miller, netdev, davinci-linux-open-source,
Rafael J. Wysocki
On 3/25/2013 9:46 PM, Mike Turquette wrote:
> Quoting Sekhar Nori (2013-03-25 02:25:47)
>> Use clk_prepare()/clk_unprepare() in the driver since common
>> clock framework needs these to be called before clock is enabled.
>>
>> This is in preparation of common clock framework migration
>> for DaVinci.
>>
>> Cc: Mike Turquette <mturquette@linaro.org>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>> drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++--
>> 1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
>> index b1349b5..436296c 100644
>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>> @@ -350,6 +350,7 @@ struct emac_priv {
>> /*platform specific members*/
>> void (*int_enable) (void);
>> void (*int_disable) (void);
>> + struct clk *clk;
>> };
>>
>> /* EMAC TX Host Error description strings */
>> @@ -1870,19 +1871,29 @@ static int davinci_emac_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "failed to get EMAC clock\n");
>> return -EBUSY;
>> }
>> +
>> + rc = clk_prepare(emac_clk);
>> + if (rc) {
>> + dev_err(&pdev->dev, "emac clock prepare failed.\n");
>> + return rc;
>> + }
>> +
>
> Is clk_enable ever called for emac_clk here? I don't see it in the
> diff. clk_prepare and clk_enable should usually be paired, even if
> simply calling clk_prepare generates the clock signal that your device
> needs.
clk_enable() is called by pm_runtime framework. Without this patch, the
clk_enable() call from pm_clk_resume() will result in a warning when
using common clock framework. This issue can actually be fixed by patch
below which fixes this in drivers/base/power/clock_ops.c so
individual drivers don't have to do this, but we need to careful since
will break for callers who intend to use the pm_runtime apis from atomic
context.
Anyway, its probably much better to fix this in pm_runtime framework so I will
work with Rafael to see if I can come up with something acceptable.
This patch can then be dropped for now, but 1/2 can still be applied.
That one is pretty harmless!
Thanks,
Sekhar
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 9d8fde7..60d389a 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -230,7 +230,7 @@ int pm_clk_suspend(struct device *dev)
list_for_each_entry_reverse(ce, &psd->clock_list, node) {
if (ce->status < PCE_STATUS_ERROR) {
if (ce->status == PCE_STATUS_ENABLED)
- clk_disable(ce->clk);
+ clk_disable_unprepare(ce->clk);
ce->status = PCE_STATUS_ACQUIRED;
}
}
@@ -259,7 +259,7 @@ int pm_clk_resume(struct device *dev)
list_for_each_entry(ce, &psd->clock_list, node) {
if (ce->status < PCE_STATUS_ERROR) {
- clk_enable(ce->clk);
+ clk_prepare_enable(ce->clk);
ce->status = PCE_STATUS_ENABLED;
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare}
2013-03-26 10:05 ` Sekhar Nori
@ 2013-03-26 16:29 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-03-26 16:29 UTC (permalink / raw)
To: nsekhar; +Cc: mturquette, netdev, davinci-linux-open-source, rjw
From: Sekhar Nori <nsekhar@ti.com>
Date: Tue, 26 Mar 2013 15:35:43 +0530
> This patch can then be dropped for now, but 1/2 can still be applied.
> That one is pretty harmless!
I've applied it.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-26 16:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-25 9:25 [PATCH net-next 0/2] net/davinci_emac updates Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 1/2] net/davinci_emac: use devres APIs Sekhar Nori
2013-03-25 9:25 ` [PATCH net-next 2/2] net/davinci_emac: use clk_{prepare|unprepare} Sekhar Nori
2013-03-25 16:16 ` Mike Turquette
2013-03-26 10:05 ` Sekhar Nori
2013-03-26 16:29 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).