This series makes some corrections on STM32 RTC:
- update copyright and adopt SPDX identifier
- fix alarm interrupt flags
- rework DBP (Disable Backup domain Protection) management.
This series is a preamble to the serie which will introduce next STM32 RTC
version.
Amelie Delaunay (4):
rtc: stm32: fix copyright and adopt SPDX identifier
rtc: stm32: fix alarm interrupt flags by removing IRQF_TRIGGER_RISING
dt-bindings: rtc: update stm32-rtc documentation for st,syscfg
property
rtc: stm32: get DBP register and mask from DT st,syscfg property
.../devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++--
drivers/rtc/rtc-stm32.c | 68 +++++++++++++---------
2 files changed, 47 insertions(+), 31 deletions(-)
--
2.7.4
RTC driver should not be aware of the PWR registers offset and bits
position. Furthermore, we can imagine that DBP relative register and bit
mask could change depending on the SoC.
So this patch introduces 2 parameters, dbp_reg and dbp_mask, allowing to
get PWR_CR and PWR_CR_DBP from device tree. And it prepares next RTC
version, backup domain write protection is disabled only if needed.
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/rtc/rtc-stm32.c | 59 +++++++++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 6a06078..de49b5b 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -85,23 +85,17 @@
#define RTC_WPR_2ND_KEY 0x53
#define RTC_WPR_WRONG_KEY 0xFF
-/*
- * RTC registers are protected against parasitic write access.
- * PWR_CR_DBP bit must be set to enable write access to RTC registers.
- */
-/* STM32_PWR_CR */
-#define PWR_CR 0x00
-/* STM32_PWR_CR bit field */
-#define PWR_CR_DBP BIT(8)
-
struct stm32_rtc_data {
bool has_pclk;
+ bool need_dbp;
};
struct stm32_rtc {
struct rtc_device *rtc_dev;
void __iomem *base;
struct regmap *dbp;
+ unsigned int dbp_reg;
+ unsigned int dbp_mask;
struct stm32_rtc_data *data;
struct clk *pclk;
struct clk *rtc_ck;
@@ -498,10 +492,12 @@ static const struct rtc_class_ops stm32_rtc_ops = {
static const struct stm32_rtc_data stm32_rtc_data = {
.has_pclk = false,
+ .need_dbp = true,
};
static const struct stm32_rtc_data stm32h7_rtc_data = {
.has_pclk = true,
+ .need_dbp = true,
};
static const struct of_device_id stm32_rtc_of_match[] = {
@@ -576,7 +572,6 @@ static int stm32_rtc_probe(struct platform_device *pdev)
{
struct stm32_rtc *rtc;
struct resource *res;
- const struct of_device_id *match;
int ret;
rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
@@ -588,15 +583,31 @@ static int stm32_rtc_probe(struct platform_device *pdev)
if (IS_ERR(rtc->base))
return PTR_ERR(rtc->base);
- rtc->dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
- "st,syscfg");
- if (IS_ERR(rtc->dbp)) {
- dev_err(&pdev->dev, "no st,syscfg\n");
- return PTR_ERR(rtc->dbp);
- }
+ rtc->data = (struct stm32_rtc_data *)
+ of_device_get_match_data(&pdev->dev);
+
+ if (rtc->data->need_dbp) {
+ rtc->dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "st,syscfg");
+ if (IS_ERR(rtc->dbp)) {
+ dev_err(&pdev->dev, "no st,syscfg\n");
+ return PTR_ERR(rtc->dbp);
+ }
- match = of_match_device(stm32_rtc_of_match, &pdev->dev);
- rtc->data = (struct stm32_rtc_data *)match->data;
+ ret = of_property_read_u32_index(pdev->dev.of_node, "st,syscfg",
+ 1, &rtc->dbp_reg);
+ if (ret) {
+ dev_err(&pdev->dev, "can't read DBP register offset\n");
+ return ret;
+ }
+
+ ret = of_property_read_u32_index(pdev->dev.of_node, "st,syscfg",
+ 2, &rtc->dbp_mask);
+ if (ret) {
+ dev_err(&pdev->dev, "can't read DBP register mask\n");
+ return ret;
+ }
+ }
if (!rtc->data->has_pclk) {
rtc->pclk = NULL;
@@ -624,7 +635,9 @@ static int stm32_rtc_probe(struct platform_device *pdev)
if (ret)
goto err;
- regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
+ if (rtc->data->need_dbp)
+ regmap_update_bits(rtc->dbp, rtc->dbp_reg,
+ rtc->dbp_mask, rtc->dbp_mask);
/*
* After a system reset, RTC_ISR.INITS flag can be read to check if
@@ -684,7 +697,8 @@ static int stm32_rtc_probe(struct platform_device *pdev)
clk_disable_unprepare(rtc->pclk);
clk_disable_unprepare(rtc->rtc_ck);
- regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, 0);
+ if (rtc->data->need_dbp)
+ regmap_update_bits(rtc->dbp, rtc->dbp_reg, rtc->dbp_mask, 0);
device_init_wakeup(&pdev->dev, false);
@@ -707,8 +721,9 @@ static int stm32_rtc_remove(struct platform_device *pdev)
if (rtc->data->has_pclk)
clk_disable_unprepare(rtc->pclk);
- /* Enable backup domain write protection */
- regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, 0);
+ /* Enable backup domain write protection if needed */
+ if (rtc->data->need_dbp)
+ regmap_update_bits(rtc->dbp, rtc->dbp_reg, rtc->dbp_mask, 0);
device_init_wakeup(&pdev->dev, false);
--
2.7.4
RTC driver should not be aware of the PWR registers offset and bits
position. Furthermore, we can imagine that Disable Backup Protection (DBP)
relative register and bit mask could change depending on the SoC. So this
patch moves st,syscfg property from single pwrcfg phandle to pwrcfg
phandle/offset/mask triplet.
Signed-off-by: Amelie Delaunay <[email protected]>
---
Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
index a66692a..00f8b5d 100644
--- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
@@ -14,8 +14,10 @@ Required properties:
It is required only on stm32h7.
- interrupt-parent: phandle for the interrupt controller.
- interrupts: rtc alarm interrupt.
-- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
- (RTC registers) write protection.
+- st,syscfg: phandle/offset/mask triplet. The phandle to pwrcfg used to
+ access control register at offset, and change the dbp (Disable Backup
+ Protection) bit represented by the mask, mandatory to disable/enable backup
+ domain (RTC registers) write protection.
Optional properties (to override default rtc_ck parent clock):
- assigned-clocks: reference to the rtc_ck clock entry.
@@ -31,7 +33,7 @@ Example:
assigned-clock-parents = <&rcc 1 CLK_LSE>;
interrupt-parent = <&exti>;
interrupts = <17 1>;
- st,syscfg = <&pwrcfg>;
+ st,syscfg = <&pwrcfg 0x00 0x100>;
};
rtc: rtc@58004000 {
@@ -44,5 +46,5 @@ Example:
interrupt-parent = <&exti>;
interrupts = <17 1>;
interrupt-names = "alarm";
- st,syscfg = <&pwrcfg>;
+ st,syscfg = <&pwrcfg 0x00 0x100>;
};
--
2.7.4
Fix copyright by removing "SA" and "for STMicroelectronics", not required.
Adopt SPDX identifier.
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/rtc/rtc-stm32.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index f25dabe..79d9e86 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -1,7 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
- * Copyright (C) STMicroelectronics SA 2017
- * Author: Amelie Delaunay <[email protected]> for STMicroelectronics.
- * License terms: GNU General Public License (GPL), version 2
+ * Copyright (C) STMicroelectronics 2017
+ * Author: Amelie Delaunay <[email protected]>
*/
#include <linux/bcd.h>
--
2.7.4
RTC alarm interrupt is active high and already configured by device tree.
So remove IRQF_TRIGGER_RISING from driver.
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/rtc/rtc-stm32.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 79d9e86..6a06078 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -663,8 +663,7 @@ static int stm32_rtc_probe(struct platform_device *pdev)
/* Handle RTC alarm interrupts */
ret = devm_request_threaded_irq(&pdev->dev, rtc->irq_alarm, NULL,
- stm32_rtc_alarm_irq,
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ stm32_rtc_alarm_irq, IRQF_ONESHOT,
pdev->name, rtc);
if (ret) {
dev_err(&pdev->dev, "IRQ%d (alarm interrupt) already claimed\n",
--
2.7.4
Hi,
Both series were not sent to the correct email address. I would suggest
rebasing on top of v4.17-rc1 ;). No need to resend just for that.
On 19/04/2018 15:21:39+0200, Amelie Delaunay wrote:
> This series makes some corrections on STM32 RTC:
> - update copyright and adopt SPDX identifier
> - fix alarm interrupt flags
> - rework DBP (Disable Backup domain Protection) management.
>
> This series is a preamble to the serie which will introduce next STM32 RTC
> version.
>
> Amelie Delaunay (4):
> rtc: stm32: fix copyright and adopt SPDX identifier
> rtc: stm32: fix alarm interrupt flags by removing IRQF_TRIGGER_RISING
> dt-bindings: rtc: update stm32-rtc documentation for st,syscfg
> property
> rtc: stm32: get DBP register and mask from DT st,syscfg property
>
> .../devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++--
> drivers/rtc/rtc-stm32.c | 68 +++++++++++++---------
> 2 files changed, 47 insertions(+), 31 deletions(-)
>
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hi,
Sorry for that! I take note of your new email address. Thanks for your highlight.
Regards,
Amelie
> -----Original Message-----
> From: Alexandre Belloni [mailto:[email protected]]
> Sent: jeudi 19 avril 2018 15:43
> To: Amelie DELAUNAY <[email protected]>
> Cc: Alessandro Zummo <[email protected]>; Rob Herring
> <[email protected]>; Mark Rutland <[email protected]>; Maxime
> Coquelin <[email protected]>; Alexandre TORGUE
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH 0/4] Various fixes on STM32 RTC
>
> Hi,
>
> Both series were not sent to the correct email address. I would suggest
> rebasing on top of v4.17-rc1 ;). No need to resend just for that.
>
> On 19/04/2018 15:21:39+0200, Amelie Delaunay wrote:
> > This series makes some corrections on STM32 RTC:
> > - update copyright and adopt SPDX identifier
> > - fix alarm interrupt flags
> > - rework DBP (Disable Backup domain Protection) management.
> >
> > This series is a preamble to the serie which will introduce next STM32
> > RTC version.
> >
> > Amelie Delaunay (4):
> > rtc: stm32: fix copyright and adopt SPDX identifier
> > rtc: stm32: fix alarm interrupt flags by removing IRQF_TRIGGER_RISING
> > dt-bindings: rtc: update stm32-rtc documentation for st,syscfg
> > property
> > rtc: stm32: get DBP register and mask from DT st,syscfg property
> >
> > .../devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++--
> > drivers/rtc/rtc-stm32.c | 68 +++++++++++++---------
> > 2 files changed, 47 insertions(+), 31 deletions(-)
> >
> > --
> > 2.7.4
> >
>
> --
> Alexandre Belloni, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
On Thu, Apr 19, 2018 at 03:21:42PM +0200, Amelie Delaunay wrote:
> RTC driver should not be aware of the PWR registers offset and bits
> position. Furthermore, we can imagine that Disable Backup Protection (DBP)
> relative register and bit mask could change depending on the SoC. So this
> patch moves st,syscfg property from single pwrcfg phandle to pwrcfg
> phandle/offset/mask triplet.
>
> Signed-off-by: Amelie Delaunay <[email protected]>
> ---
> Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> index a66692a..00f8b5d 100644
> --- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> +++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> @@ -14,8 +14,10 @@ Required properties:
> It is required only on stm32h7.
> - interrupt-parent: phandle for the interrupt controller.
> - interrupts: rtc alarm interrupt.
> -- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
> - (RTC registers) write protection.
> +- st,syscfg: phandle/offset/mask triplet. The phandle to pwrcfg used to
> + access control register at offset, and change the dbp (Disable Backup
> + Protection) bit represented by the mask, mandatory to disable/enable backup
> + domain (RTC registers) write protection.
It's fine to add this, but you are breaking compatibility in the driver
with existing DTBs by requiring these new fields.
Rob
Amelie,
On 26/04/2018 21:58:03-0500, Rob Herring wrote:
> On Thu, Apr 19, 2018 at 03:21:42PM +0200, Amelie Delaunay wrote:
> > RTC driver should not be aware of the PWR registers offset and bits
> > position. Furthermore, we can imagine that Disable Backup Protection (DBP)
> > relative register and bit mask could change depending on the SoC. So this
> > patch moves st,syscfg property from single pwrcfg phandle to pwrcfg
> > phandle/offset/mask triplet.
> >
> > Signed-off-by: Amelie Delaunay <[email protected]>
> > ---
> > Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> > index a66692a..00f8b5d 100644
> > --- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> > +++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
> > @@ -14,8 +14,10 @@ Required properties:
> > It is required only on stm32h7.
> > - interrupt-parent: phandle for the interrupt controller.
> > - interrupts: rtc alarm interrupt.
> > -- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
> > - (RTC registers) write protection.
> > +- st,syscfg: phandle/offset/mask triplet. The phandle to pwrcfg used to
> > + access control register at offset, and change the dbp (Disable Backup
> > + Protection) bit represented by the mask, mandatory to disable/enable backup
> > + domain (RTC registers) write protection.
>
> It's fine to add this, but you are breaking compatibility in the driver
> with existing DTBs by requiring these new fields.
>
I'm fine with that change but I would like confirmation that this has
been well thought. Maybe Maxime or Alexandre could give their ack.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hi Alexandre,
On 05/03/2018 10:53 PM, Alexandre Belloni wrote:
> Amelie,
>
> On 26/04/2018 21:58:03-0500, Rob Herring wrote:
>> On Thu, Apr 19, 2018 at 03:21:42PM +0200, Amelie Delaunay wrote:
>>> RTC driver should not be aware of the PWR registers offset and bits
>>> position. Furthermore, we can imagine that Disable Backup Protection (DBP)
>>> relative register and bit mask could change depending on the SoC. So this
>>> patch moves st,syscfg property from single pwrcfg phandle to pwrcfg
>>> phandle/offset/mask triplet.
>>>
>>> Signed-off-by: Amelie Delaunay <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>> index a66692a..00f8b5d 100644
>>> --- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>> +++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>> @@ -14,8 +14,10 @@ Required properties:
>>> It is required only on stm32h7.
>>> - interrupt-parent: phandle for the interrupt controller.
>>> - interrupts: rtc alarm interrupt.
>>> -- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>>> - (RTC registers) write protection.
>>> +- st,syscfg: phandle/offset/mask triplet. The phandle to pwrcfg used to
>>> + access control register at offset, and change the dbp (Disable Backup
>>> + Protection) bit represented by the mask, mandatory to disable/enable backup
>>> + domain (RTC registers) write protection.
>>
>> It's fine to add this, but you are breaking compatibility in the driver
>> with existing DTBs by requiring these new fields.
>>
>
> I'm fine with that change but I would like confirmation that this has
> been well thought. Maybe Maxime or Alexandre could give their ack.
>
It's a good thing to remove PWR registers information from RTC driver.
My only concern was the compatibility with old DT but we can accept it.
Indeed, Kernel will continue to boot fine, only RTC will not probe if we
use old DT.
Acked-by: Alexandre TORGUE <[email protected]>
Regards
alex
On 19/04/2018 15:21:39+0200, Amelie Delaunay wrote:
> This series makes some corrections on STM32 RTC:
> - update copyright and adopt SPDX identifier
> - fix alarm interrupt flags
> - rework DBP (Disable Backup domain Protection) management.
>
> This series is a preamble to the serie which will introduce next STM32 RTC
> version.
>
> Amelie Delaunay (4):
> rtc: stm32: fix copyright and adopt SPDX identifier
> rtc: stm32: fix alarm interrupt flags by removing IRQF_TRIGGER_RISING
> dt-bindings: rtc: update stm32-rtc documentation for st,syscfg
> property
> rtc: stm32: get DBP register and mask from DT st,syscfg property
>
> .../devicetree/bindings/rtc/st,stm32-rtc.txt | 10 ++--
> drivers/rtc/rtc-stm32.c | 68 +++++++++++++---------
> 2 files changed, 47 insertions(+), 31 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hi,
On 05/04/2018 09:40 AM, Alexandre Torgue wrote:
> Hi Alexandre,
>
> On 05/03/2018 10:53 PM, Alexandre Belloni wrote:
>> Amelie,
>>
>> On 26/04/2018 21:58:03-0500, Rob Herring wrote:
>>> On Thu, Apr 19, 2018 at 03:21:42PM +0200, Amelie Delaunay wrote:
>>>> RTC driver should not be aware of the PWR registers offset and bits
>>>> position. Furthermore, we can imagine that Disable Backup Protection
>>>> (DBP)
>>>> relative register and bit mask could change depending on the SoC. So
>>>> this
>>>> patch moves st,syscfg property from single pwrcfg phandle to pwrcfg
>>>> phandle/offset/mask triplet.
>>>>
>>>> Signed-off-by: Amelie Delaunay <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt | 10
>>>> ++++++----
>>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>>> b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>>> index a66692a..00f8b5d 100644
>>>> --- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>>> +++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
>>>> @@ -14,8 +14,10 @@ Required properties:
>>>> It is required only on stm32h7.
>>>> - interrupt-parent: phandle for the interrupt controller.
>>>> - interrupts: rtc alarm interrupt.
>>>> -- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup
>>>> domain
>>>> - (RTC registers) write protection.
>>>> +- st,syscfg: phandle/offset/mask triplet. The phandle to pwrcfg
>>>> used to
>>>> + access control register at offset, and change the dbp (Disable
>>>> Backup
>>>> + Protection) bit represented by the mask, mandatory to
>>>> disable/enable backup
>>>> + domain (RTC registers) write protection.
>>>
>>> It's fine to add this, but you are breaking compatibility in the driver
>>> with existing DTBs by requiring these new fields.
>>>
>>
>> I'm fine with that change but I would like confirmation that this has
>> been well thought. Maybe Maxime or Alexandre could give their ack.
>>
>
> It's a good thing to remove PWR registers information from RTC driver.
> My only concern was the compatibility with old DT but we can accept it.
> Indeed, Kernel will continue to boot fine, only RTC will not probe if we
> use old DT.
>
> Acked-by: Alexandre TORGUE <[email protected]>
>
> Regards
> alex
I am going to send a series to update st,syscfg property in
stm32f429/stm32f746/stm32h743 RTC node.
Thanks,
Amelie