[PATCH v2 0/4] Make PL031 interrupt optional

[PATCH v2 0/4] Make PL031 interrupt optional

[Russell King - ARM Linux]

There are some boards out there which have a PL031 RTC, but its
interrupt is not wired up.  To support these, we need the PL031
to support the primecell without interrupts.

When no interrupt is present, there's little point exposing the
RTC's alarm capabilities, so we omit the alarm-related function
calls - the RTC merely becomes a source of time-of-day.

This patch series cleans up the pl031 driver a little, and adds
support for this configuration.

 drivers/rtc/rtc-pl031.c | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

v2: update patch 2 & 3 for review comment on v1.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH v2 0/4] Make PL031 interrupt optional

[Russell King - ARM Linux]

There are some boards out there which have a PL031 RTC, but its
interrupt is not wired up.  To support these, we need the PL031
to support the primecell without interrupts.

When no interrupt is present, there's little point exposing the
RTC's alarm capabilities, so we omit the alarm-related function
calls - the RTC merely becomes a source of time-of-day.

This patch series cleans up the pl031 driver a little, and adds
support for this configuration.

 drivers/rtc/rtc-pl031.c | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

v2: update patch 2 & 3 for review comment on v1.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH v2 1/4] rtc: pl031: constify amba_ids

[Russell King]

The AMBA device IDs should be marked const.  Make that so.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index e1687e19c59f..0d87b90b1903 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -446,7 +446,7 @@ static struct pl031_vendor_data stv2_pl031 = {
 	.irqflags = IRQF_SHARED | IRQF_COND_SUSPEND,
 };
 
-static struct amba_id pl031_ids[] = {
+static const struct amba_id pl031_ids[] = {
 	{
 		.id = 0x00041031,
 		.mask = 0x000fffff,
-- 
2.7.4

[PATCH v2 1/4] rtc: pl031: constify amba_ids

[Russell King]

The AMBA device IDs should be marked const.  Make that so.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index e1687e19c59f..0d87b90b1903 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -446,7 +446,7 @@ static struct pl031_vendor_data stv2_pl031 = {
 	.irqflags = IRQF_SHARED | IRQF_COND_SUSPEND,
 };
 
-static struct amba_id pl031_ids[] = {
+static const struct amba_id pl031_ids[] = {
 	{
 		.id = 0x00041031,
 		.mask = 0x000fffff,
-- 
2.7.4

[PATCH v2 2/4] rtc: pl031: use devm_* for allocating memory and mapping resource

[Russell King]

Use the devm_* APIs for allocating memory and mapping the memory in
the probe function to relieve the driver from having to deal with
this in the cleanup paths.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 0d87b90b1903..777da06d53ee 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -310,8 +310,6 @@ static int pl031_remove(struct amba_device *adev)
 	device_init_wakeup(&adev->dev, false);
 	free_irq(adev->irq[0], ldata);
 	rtc_device_unregister(ldata->rtc);
-	iounmap(ldata->base);
-	kfree(ldata);
 	amba_release_regions(adev);
 
 	return 0;
@@ -329,18 +327,19 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 	if (ret)
 		goto err_req;
 
-	ldata = kzalloc(sizeof(struct pl031_local), GFP_KERNEL);
+	ldata = devm_kzalloc(&adev->dev, sizeof(struct pl031_local),
+			     GFP_KERNEL);
 	if (!ldata) {
 		ret = -ENOMEM;
 		goto out;
 	}
 	ldata->vendor = vendor;
 
-	ldata->base = ioremap(adev->res.start, resource_size(&adev->res));
-
+	ldata->base = devm_ioremap(&adev->dev, adev->res.start,
+				   resource_size(&adev->res));
 	if (!ldata->base) {
 		ret = -ENOMEM;
-		goto out_no_remap;
+		goto out;
 	}
 
 	amba_set_drvdata(adev, ldata);
@@ -378,7 +377,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 					THIS_MODULE);
 	if (IS_ERR(ldata->rtc)) {
 		ret = PTR_ERR(ldata->rtc);
-		goto out_no_rtc;
+		goto out;
 	}
 
 	if (request_irq(adev->irq[0], pl031_interrupt,
@@ -391,10 +390,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 
 out_no_irq:
 	rtc_device_unregister(ldata->rtc);
-out_no_rtc:
-	iounmap(ldata->base);
-out_no_remap:
-	kfree(ldata);
 out:
 	amba_release_regions(adev);
 err_req:
-- 
2.7.4

[PATCH v2 2/4] rtc: pl031: use devm_* for allocating memory and mapping resource

[Russell King]

Use the devm_* APIs for allocating memory and mapping the memory in
the probe function to relieve the driver from having to deal with
this in the cleanup paths.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 0d87b90b1903..777da06d53ee 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -310,8 +310,6 @@ static int pl031_remove(struct amba_device *adev)
 	device_init_wakeup(&adev->dev, false);
 	free_irq(adev->irq[0], ldata);
 	rtc_device_unregister(ldata->rtc);
-	iounmap(ldata->base);
-	kfree(ldata);
 	amba_release_regions(adev);
 
 	return 0;
@@ -329,18 +327,19 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 	if (ret)
 		goto err_req;
 
-	ldata = kzalloc(sizeof(struct pl031_local), GFP_KERNEL);
+	ldata = devm_kzalloc(&adev->dev, sizeof(struct pl031_local),
+			     GFP_KERNEL);
 	if (!ldata) {
 		ret = -ENOMEM;
 		goto out;
 	}
 	ldata->vendor = vendor;
 
-	ldata->base = ioremap(adev->res.start, resource_size(&adev->res));
-
+	ldata->base = devm_ioremap(&adev->dev, adev->res.start,
+				   resource_size(&adev->res));
 	if (!ldata->base) {
 		ret = -ENOMEM;
-		goto out_no_remap;
+		goto out;
 	}
 
 	amba_set_drvdata(adev, ldata);
@@ -378,7 +377,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 					THIS_MODULE);
 	if (IS_ERR(ldata->rtc)) {
 		ret = PTR_ERR(ldata->rtc);
-		goto out_no_rtc;
+		goto out;
 	}
 
 	if (request_irq(adev->irq[0], pl031_interrupt,
@@ -391,10 +390,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 
 out_no_irq:
 	rtc_device_unregister(ldata->rtc);
-out_no_rtc:
-	iounmap(ldata->base);
-out_no_remap:
-	kfree(ldata);
 out:
 	amba_release_regions(adev);
 err_req:
-- 
2.7.4

[PATCH v2 3/4] rtc: pl031: avoid exposing alarm if no interrupt

[Russell King]

If the RTC has no interrupt, there is little point in exposing the RTC
alarm capabilities, as it can't be used as a wakeup source nor can it
deliver an event to userspace.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 777da06d53ee..64c77ec1b4ea 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -320,7 +320,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 	int ret;
 	struct pl031_local *ldata;
 	struct pl031_vendor_data *vendor = id->data;
-	struct rtc_class_ops *ops = &vendor->ops;
+	struct rtc_class_ops *ops;
 	unsigned long time, data;
 
 	ret = amba_request_regions(adev, NULL);
@@ -329,12 +329,14 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 
 	ldata = devm_kzalloc(&adev->dev, sizeof(struct pl031_local),
 			     GFP_KERNEL);
-	if (!ldata) {
+	ops = devm_kmemdup(&adev->dev, &vendor->ops, sizeof(vendor->ops),
+			   GFP_KERNEL);
+	if (!ldata || !ops) {
 		ret = -ENOMEM;
 		goto out;
 	}
-	ldata->vendor = vendor;
 
+	ldata->vendor = vendor;
 	ldata->base = devm_ioremap(&adev->dev, adev->res.start,
 				   resource_size(&adev->res));
 	if (!ldata->base) {
@@ -372,6 +374,13 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		}
 	}
 
+	if (!adev->irq[0]) {
+		/* When there's no interrupt, no point in exposing the alarm */
+		ops->read_alarm = NULL;
+		ops->set_alarm = NULL;
+		ops->alarm_irq_enable = NULL;
+	}
+
 	device_init_wakeup(&adev->dev, true);
 	ldata->rtc = rtc_device_register("pl031", &adev->dev, ops,
 					THIS_MODULE);
-- 
2.7.4

[PATCH v2 3/4] rtc: pl031: avoid exposing alarm if no interrupt

[Russell King]

If the RTC has no interrupt, there is little point in exposing the RTC
alarm capabilities, as it can't be used as a wakeup source nor can it
deliver an event to userspace.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 777da06d53ee..64c77ec1b4ea 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -320,7 +320,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 	int ret;
 	struct pl031_local *ldata;
 	struct pl031_vendor_data *vendor = id->data;
-	struct rtc_class_ops *ops = &vendor->ops;
+	struct rtc_class_ops *ops;
 	unsigned long time, data;
 
 	ret = amba_request_regions(adev, NULL);
@@ -329,12 +329,14 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 
 	ldata = devm_kzalloc(&adev->dev, sizeof(struct pl031_local),
 			     GFP_KERNEL);
-	if (!ldata) {
+	ops = devm_kmemdup(&adev->dev, &vendor->ops, sizeof(vendor->ops),
+			   GFP_KERNEL);
+	if (!ldata || !ops) {
 		ret = -ENOMEM;
 		goto out;
 	}
-	ldata->vendor = vendor;
 
+	ldata->vendor = vendor;
 	ldata->base = devm_ioremap(&adev->dev, adev->res.start,
 				   resource_size(&adev->res));
 	if (!ldata->base) {
@@ -372,6 +374,13 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		}
 	}
 
+	if (!adev->irq[0]) {
+		/* When there's no interrupt, no point in exposing the alarm */
+		ops->read_alarm = NULL;
+		ops->set_alarm = NULL;
+		ops->alarm_irq_enable = NULL;
+	}
+
 	device_init_wakeup(&adev->dev, true);
 	ldata->rtc = rtc_device_register("pl031", &adev->dev, ops,
 					THIS_MODULE);
-- 
2.7.4

[PATCH v2 4/4] rtc: pl031: make interrupt optional

[Russell King]

On some platforms, the interrupt for the PL031 is optional.  Avoid
trying to claim the interrupt if it's not specified.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 64c77ec1b4ea..82eb7da2c478 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -308,7 +308,8 @@ static int pl031_remove(struct amba_device *adev)
 
 	dev_pm_clear_wake_irq(&adev->dev);
 	device_init_wakeup(&adev->dev, false);
-	free_irq(adev->irq[0], ldata);
+	if (adev->irq[0])
+		free_irq(adev->irq[0], ldata);
 	rtc_device_unregister(ldata->rtc);
 	amba_release_regions(adev);
 
@@ -389,12 +390,13 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		goto out;
 	}
 
-	if (request_irq(adev->irq[0], pl031_interrupt,
-			vendor->irqflags, "rtc-pl031", ldata)) {
-		ret = -EIO;
-		goto out_no_irq;
+	if (adev->irq[0]) {
+		ret = request_irq(adev->irq[0], pl031_interrupt,
+				  vendor->irqflags, "rtc-pl031", ldata);
+		if (ret)
+			goto out_no_irq;
+		dev_pm_set_wake_irq(&adev->dev, adev->irq[0]);
 	}
-	dev_pm_set_wake_irq(&adev->dev, adev->irq[0]);
 	return 0;
 
 out_no_irq:
-- 
2.7.4

[PATCH v2 4/4] rtc: pl031: make interrupt optional

[Russell King]

On some platforms, the interrupt for the PL031 is optional.  Avoid
trying to claim the interrupt if it's not specified.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/rtc/rtc-pl031.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 64c77ec1b4ea..82eb7da2c478 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -308,7 +308,8 @@ static int pl031_remove(struct amba_device *adev)
 
 	dev_pm_clear_wake_irq(&adev->dev);
 	device_init_wakeup(&adev->dev, false);
-	free_irq(adev->irq[0], ldata);
+	if (adev->irq[0])
+		free_irq(adev->irq[0], ldata);
 	rtc_device_unregister(ldata->rtc);
 	amba_release_regions(adev);
 
@@ -389,12 +390,13 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		goto out;
 	}
 
-	if (request_irq(adev->irq[0], pl031_interrupt,
-			vendor->irqflags, "rtc-pl031", ldata)) {
-		ret = -EIO;
-		goto out_no_irq;
+	if (adev->irq[0]) {
+		ret = request_irq(adev->irq[0], pl031_interrupt,
+				  vendor->irqflags, "rtc-pl031", ldata);
+		if (ret)
+			goto out_no_irq;
+		dev_pm_set_wake_irq(&adev->dev, adev->irq[0]);
 	}
-	dev_pm_set_wake_irq(&adev->dev, adev->irq[0]);
 	return 0;
 
 out_no_irq:
-- 
2.7.4

Re: [PATCH v2 0/4] Make PL031 interrupt optional

[Linus Walleij]

On Fri, Sep 29, 2017 at 12:21 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> There are some boards out there which have a PL031 RTC, but its
> interrupt is not wired up.  To support these, we need the PL031
> to support the primecell without interrupts.
>
> When no interrupt is present, there's little point exposing the
> RTC's alarm capabilities, so we omit the alarm-related function
> calls - the RTC merely becomes a source of time-of-day.
>
> This patch series cleans up the pl031 driver a little, and adds
> support for this configuration.

This series:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 0/4] Make PL031 interrupt optional

[Linus Walleij]

On Fri, Sep 29, 2017 at 12:21 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> There are some boards out there which have a PL031 RTC, but its
> interrupt is not wired up.  To support these, we need the PL031
> to support the primecell without interrupts.
>
> When no interrupt is present, there's little point exposing the
> RTC's alarm capabilities, so we omit the alarm-related function
> calls - the RTC merely becomes a source of time-of-day.
>
> This patch series cleans up the pl031 driver a little, and adds
> support for this configuration.

This series:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 0/4] Make PL031 interrupt optional

[Alexandre Belloni]

On 29/09/2017 at 11:21:16 +0100, Russell King - ARM Linux wrote:
> There are some boards out there which have a PL031 RTC, but its
> interrupt is not wired up.  To support these, we need the PL031
> to support the primecell without interrupts.
> 
> When no interrupt is present, there's little point exposing the
> RTC's alarm capabilities, so we omit the alarm-related function
> calls - the RTC merely becomes a source of time-of-day.
> 
> This patch series cleans up the pl031 driver a little, and adds
> support for this configuration.
> 
>  drivers/rtc/rtc-pl031.c | 48 +++++++++++++++++++++++++++---------------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> v2: update patch 2 & 3 for review comment on v1.
> 

Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 0/4] Make PL031 interrupt optional

[Alexandre Belloni]

On 29/09/2017 at 11:21:16 +0100, Russell King - ARM Linux wrote:
> There are some boards out there which have a PL031 RTC, but its
> interrupt is not wired up.  To support these, we need the PL031
> to support the primecell without interrupts.
> 
> When no interrupt is present, there's little point exposing the
> RTC's alarm capabilities, so we omit the alarm-related function
> calls - the RTC merely becomes a source of time-of-day.
> 
> This patch series cleans up the pl031 driver a little, and adds
> support for this configuration.
> 
>  drivers/rtc/rtc-pl031.c | 48 +++++++++++++++++++++++++++---------------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> v2: update patch 2 & 3 for review comment on v1.
> 

Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com