Use devm_ioremap_resource() in order to make the code simpler,
because devm_ioremap_resource() includes devm_request_mem_region()
and devm_ioremap(). Also, it checks the return value of
platform_get_resource().
Jingoo Han (4):
rtc: rtc-coh901331: Use devm_ioremap_resource()
rtc: rtc-davinci: Use devm_ioremap_resource()
rtc: rtc-jz4740: Use devm_ioremap_resource()
rtc: rtc-vt8500: Use devm_ioremap_resource()
---
drivers/rtc/rtc-coh901331.c | 18 +++---------------
drivers/rtc/rtc-davinci.c | 29 ++++-------------------------
drivers/rtc/rtc-jz4740.c | 24 +++++++-----------------
drivers/rtc/rtc-vt8500.c | 28 +++++-----------------------
4 files changed, 19 insertions(+), 80 deletions(-)
Use devm_ioremap_resource() in order to make the code simpler,
and remove redundant return value check of platform_get_resource()
because the value is checked by devm_ioremap_resource().
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/rtc/rtc-coh901331.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/rtc/rtc-coh901331.c b/drivers/rtc/rtc-coh901331.c
index 73f1575..869cae2 100644
--- a/drivers/rtc/rtc-coh901331.c
+++ b/drivers/rtc/rtc-coh901331.c
@@ -43,8 +43,6 @@
struct coh901331_port {
struct rtc_device *rtc;
struct clk *clk;
- u32 phybase;
- u32 physize;
void __iomem *virtbase;
int irq;
#ifdef CONFIG_PM_SLEEP
@@ -173,19 +171,9 @@ static int __init coh901331_probe(struct platform_device *pdev)
return -ENOMEM;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -ENOENT;
-
- rtap->phybase = res->start;
- rtap->physize = resource_size(res);
-
- if (devm_request_mem_region(&pdev->dev, rtap->phybase, rtap->physize,
- "rtc-coh901331") == NULL)
- return -EBUSY;
-
- rtap->virtbase = devm_ioremap(&pdev->dev, rtap->phybase, rtap->physize);
- if (!rtap->virtbase)
- return -ENOMEM;
+ rtap->virtbase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(rtap->virtbase))
+ return PTR_ERR(rtap->virtbase);
rtap->irq = platform_get_irq(pdev, 0);
if (devm_request_irq(&pdev->dev, rtap->irq, coh901331_interrupt, 0,
--
1.7.10.4
Use devm_ioremap_resource() in order to make the code simpler,
and remove redundant return value check of platform_get_resource()
because the value is checked by devm_ioremap_resource().
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/rtc/rtc-davinci.c | 29 ++++-------------------------
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/drivers/rtc/rtc-davinci.c b/drivers/rtc/rtc-davinci.c
index 24677ef8..d3e70f3 100644
--- a/drivers/rtc/rtc-davinci.c
+++ b/drivers/rtc/rtc-davinci.c
@@ -119,8 +119,6 @@ static DEFINE_SPINLOCK(davinci_rtc_lock);
struct davinci_rtc {
struct rtc_device *rtc;
void __iomem *base;
- resource_size_t pbase;
- size_t base_size;
int irq;
};
@@ -482,7 +480,7 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct davinci_rtc *davinci_rtc;
- struct resource *res, *mem;
+ struct resource *res;
int ret = 0;
davinci_rtc = devm_kzalloc(&pdev->dev, sizeof(struct davinci_rtc), GFP_KERNEL);
@@ -498,28 +496,9 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, "no mem resource\n");
- return -EINVAL;
- }
-
- davinci_rtc->pbase = res->start;
- davinci_rtc->base_size = resource_size(res);
-
- mem = devm_request_mem_region(dev, davinci_rtc->pbase,
- davinci_rtc->base_size, pdev->name);
- if (!mem) {
- dev_err(dev, "RTC registers at %08x are not free\n",
- davinci_rtc->pbase);
- return -EBUSY;
- }
-
- davinci_rtc->base = devm_ioremap(dev, davinci_rtc->pbase,
- davinci_rtc->base_size);
- if (!davinci_rtc->base) {
- dev_err(dev, "unable to ioremap MEM resource\n");
- return -ENOMEM;
- }
+ davinci_rtc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(davinci_rtc->base))
+ return PTR_ERR(davinci_rtc->base);
platform_set_drvdata(pdev, davinci_rtc);
--
1.7.10.4
Use devm_ioremap_resource() in order to make the code simpler,
and move 'struct resource *mem' from 'struct jz4740_rtc' to
jz4740_rtc_probe() because the 'mem' variable is used only in
jz4740_rtc_probe().
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/rtc/rtc-jz4740.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 1b126d2..717abb3 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -38,7 +38,6 @@
#define JZ_RTC_CTRL_ENABLE BIT(0)
struct jz4740_rtc {
- struct resource *mem;
void __iomem *base;
struct rtc_device *rtc;
@@ -216,6 +215,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
int ret;
struct jz4740_rtc *rtc;
uint32_t scratchpad;
+ struct resource *mem;
rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
if (!rtc)
@@ -227,25 +227,15 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
return -ENOENT;
}
- rtc->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!rtc->mem) {
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem) {
dev_err(&pdev->dev, "Failed to get platform mmio memory\n");
return -ENOENT;
}
-
- rtc->mem = devm_request_mem_region(&pdev->dev, rtc->mem->start,
- resource_size(rtc->mem), pdev->name);
- if (!rtc->mem) {
- dev_err(&pdev->dev, "Failed to request mmio memory region\n");
- return -EBUSY;
- }
-
- rtc->base = devm_ioremap_nocache(&pdev->dev, rtc->mem->start,
- resource_size(rtc->mem));
- if (!rtc->base) {
- dev_err(&pdev->dev, "Failed to ioremap mmio memory\n");
- return -EBUSY;
- }
+ mem->flags &= ~IORESOURCE_CACHEABLE;
+ rtc->base = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(rtc->base))
+ return PTR_ERR(rtc->base);
spin_lock_init(&rtc->lock);
--
1.7.10.4
Use devm_ioremap_resource() in order to make the code simpler,
and move 'struct resource *res' from 'struct vt8500_rtc' to
vt8500_rtc_probe() because the 'res' variable is used only in
vt8500_rtc_probe().
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/rtc/rtc-vt8500.c | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/rtc/rtc-vt8500.c b/drivers/rtc/rtc-vt8500.c
index df2ef3e..051da96 100644
--- a/drivers/rtc/rtc-vt8500.c
+++ b/drivers/rtc/rtc-vt8500.c
@@ -79,7 +79,6 @@
struct vt8500_rtc {
void __iomem *regbase;
- struct resource *res;
int irq_alarm;
struct rtc_device *rtc;
spinlock_t lock; /* Protects this structure */
@@ -209,6 +208,7 @@ static const struct rtc_class_ops vt8500_rtc_ops = {
static int vt8500_rtc_probe(struct platform_device *pdev)
{
struct vt8500_rtc *vt8500_rtc;
+ struct resource *res;
int ret;
vt8500_rtc = devm_kzalloc(&pdev->dev,
@@ -219,34 +219,16 @@ static int vt8500_rtc_probe(struct platform_device *pdev)
spin_lock_init(&vt8500_rtc->lock);
platform_set_drvdata(pdev, vt8500_rtc);
- vt8500_rtc->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!vt8500_rtc->res) {
- dev_err(&pdev->dev, "No I/O memory resource defined\n");
- return -ENXIO;
- }
-
vt8500_rtc->irq_alarm = platform_get_irq(pdev, 0);
if (vt8500_rtc->irq_alarm < 0) {
dev_err(&pdev->dev, "No alarm IRQ resource defined\n");
return vt8500_rtc->irq_alarm;
}
- vt8500_rtc->res = devm_request_mem_region(&pdev->dev,
- vt8500_rtc->res->start,
- resource_size(vt8500_rtc->res),
- "vt8500-rtc");
- if (vt8500_rtc->res == NULL) {
- dev_err(&pdev->dev, "failed to request I/O memory\n");
- return -EBUSY;
- }
-
- vt8500_rtc->regbase = devm_ioremap(&pdev->dev, vt8500_rtc->res->start,
- resource_size(vt8500_rtc->res));
- if (!vt8500_rtc->regbase) {
- dev_err(&pdev->dev, "Unable to map RTC I/O memory\n");
- ret = -EBUSY;
- goto err_return;
- }
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ vt8500_rtc->regbase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vt8500_rtc->regbase))
+ return PTR_ERR(vt8500_rtc->regbase);
/* Enable RTC and set it to 24-hour mode */
writel(VT8500_RTC_CR_ENABLE,
--
1.7.10.4
On 02/07/2014 08:58 AM, Jingoo Han wrote:
[...]
> - rtc->base = devm_ioremap_nocache(&pdev->dev, rtc->mem->start,
> - resource_size(rtc->mem));
> - if (!rtc->base) {
> - dev_err(&pdev->dev, "Failed to ioremap mmio memory\n");
> - return -EBUSY;
> - }
> + mem->flags &= ~IORESOURCE_CACHEABLE;
You shouldn't be modifying the resource, strictly speaking it is not owned
by the device. And IORESOURCE_CACHEABLE is never set for this device anyway.
> + rtc->base = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(rtc->base))
> + return PTR_ERR(rtc->base);
>
> spin_lock_init(&rtc->lock);
>
>
On Friday, February 07, 2014 5:13 PM, Lars-Peter Clausen wrote:
> On 02/07/2014 08:58 AM, Jingoo Han wrote:
> [...]
> > - rtc->base = devm_ioremap_nocache(&pdev->dev, rtc->mem->start,
> > - resource_size(rtc->mem));
> > - if (!rtc->base) {
> > - dev_err(&pdev->dev, "Failed to ioremap mmio memory\n");
> > - return -EBUSY;
> > - }
> > + mem->flags &= ~IORESOURCE_CACHEABLE;
>
> You shouldn't be modifying the resource, strictly speaking it is not owned
> by the device. And IORESOURCE_CACHEABLE is never set for this device anyway.
(+cc Thierry Reding)
Hi Lars-Peter Clausen,
Do you mean that resource's flags should NOT be modified by the
device driver, right?
Then, without 'mem->flags &= ~IORESOURCE_CACHEABLE', is it possible
that devm_ioremap_nocache() can be called at devm_ioremap_resource()?
Thierry,
Do you have any comments on this?
Thank you.
Best regards,
Jingoo Han
>
> > + rtc->base = devm_ioremap_resource(&pdev->dev, mem);
> > + if (IS_ERR(rtc->base))
> > + return PTR_ERR(rtc->base);
> >
> > spin_lock_init(&rtc->lock);
> >
> >
On 02/07/2014 09:31 AM, Jingoo Han wrote:
> On Friday, February 07, 2014 5:13 PM, Lars-Peter Clausen wrote:
>> On 02/07/2014 08:58 AM, Jingoo Han wrote:
>> [...]
>>> - rtc->base = devm_ioremap_nocache(&pdev->dev, rtc->mem->start,
>>> - resource_size(rtc->mem));
>>> - if (!rtc->base) {
>>> - dev_err(&pdev->dev, "Failed to ioremap mmio memory\n");
>>> - return -EBUSY;
>>> - }
>>> + mem->flags &= ~IORESOURCE_CACHEABLE;
>>
>> You shouldn't be modifying the resource, strictly speaking it is not owned
>> by the device. And IORESOURCE_CACHEABLE is never set for this device anyway.
>
> (+cc Thierry Reding)
>
> Hi Lars-Peter Clausen,
>
> Do you mean that resource's flags should NOT be modified by the
> device driver, right?
> Then, without 'mem->flags &= ~IORESOURCE_CACHEABLE', is it possible
> that devm_ioremap_nocache() can be called at devm_ioremap_resource()?
>
Yes. ioremap_nocache() will be called, when the flag is not set,
ioremap_cached() will be called when the flag is set. If the flag is set the
intention is probably that the region should be mapped cached.
> Thierry,
> Do you have any comments on this?
>
> Thank you.
>
> Best regards,
> Jingoo Han
>
>>
>>> + rtc->base = devm_ioremap_resource(&pdev->dev, mem);
>>> + if (IS_ERR(rtc->base))
>>> + return PTR_ERR(rtc->base);
>>>
>>> spin_lock_init(&rtc->lock);
>>>
>>>
>
On Fri, Feb 7, 2014 at 8:55 AM, Jingoo Han <[email protected]> wrote:
> Use devm_ioremap_resource() in order to make the code simpler,
> and remove redundant return value check of platform_get_resource()
> because the value is checked by devm_ioremap_resource().
>
> Signed-off-by: Jingoo Han <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij