linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] add RTT support for SAM9X60
@ 2020-04-10 16:26 Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 1/5] irqchip/atmel-aic5: add support for sam9x60 rtt fixup Claudiu Beznea
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

This series adds RTT support for SAM9X60.

Claudiu Beznea (5):
  irqchip/atmel-aic5: add support for sam9x60 rtt fixup
  rtc: at91sam9: enable driver for sam9x60
  ARM: dts: sam9x60ek: enable gpbr
  ARM: dts: sam9x60: add rtt
  rtc: at91sam9: add microchip,sam9x60-rtt

 .../devicetree/bindings/rtc/atmel,at91sam9-rtc.txt |  2 +-
 arch/arm/boot/dts/at91-sam9x60ek.dts               |  9 ++++++
 arch/arm/boot/dts/sam9x60.dtsi                     |  7 +++++
 drivers/irqchip/irq-atmel-aic-common.c             | 36 +++++++++++++---------
 drivers/irqchip/irq-atmel-aic5.c                   |  8 ++++-
 drivers/rtc/rtc-at91sam9.c                         |  1 +
 6 files changed, 47 insertions(+), 16 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/5] irqchip/atmel-aic5: add support for sam9x60 rtt fixup
  2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
@ 2020-04-10 16:26 ` Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 2/5] rtc: at91sam9: enable driver for sam9x60 Claudiu Beznea
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

Add support for SAM9X60 RTT fixup.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/irqchip/irq-atmel-aic-common.c | 36 +++++++++++++++++++++-------------
 drivers/irqchip/irq-atmel-aic5.c       |  8 +++++++-
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 072bd227b6c6..e8681133162b 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -169,21 +169,29 @@ void __init aic_common_rtt_irq_fixup(void)
 {
 	struct device_node *np;
 	void __iomem *regs;
+	static const  struct of_device_id aic_rtt_fixup_ids[] = {
+		{ .compatible = "atmel,at91sam9260-rtt", },
+		{ .compatible = "microchip,sam9x60-rtt", },
+	};
+	int i;
 
-	/*
-	 * The at91sam9263 SoC has 2 instances of the RTT block, hence we
-	 * iterate over the DT to find each occurrence.
-	 */
-	for_each_compatible_node(np, NULL, "atmel,at91sam9260-rtt") {
-		regs = of_iomap(np, 0);
-		if (!regs)
-			continue;
-
-		writel(readl(regs + AT91_RTT_MR) &
-		       ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN),
-		       regs + AT91_RTT_MR);
-
-		iounmap(regs);
+	for (i = 0; i < ARRAY_SIZE(aic_rtt_fixup_ids); i++) {
+		/*
+		 * The at91sam9263 SoC has 2 instances of the RTT block, hence
+		 * we iterate over the DT to find each occurrence.
+		 */
+		for_each_compatible_node(np, NULL,
+					 aic_rtt_fixup_ids[i].compatible) {
+			regs = of_iomap(np, 0);
+			if (!regs)
+				continue;
+
+			writel(readl(regs + AT91_RTT_MR) &
+			       ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN),
+			       regs + AT91_RTT_MR);
+
+			iounmap(regs);
+		}
 	}
 }
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 29333497ba10..74a66cd05553 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -310,10 +310,16 @@ static void __init sama5d3_aic_irq_fixup(void)
 	aic_common_rtc_irq_fixup();
 }
 
+static void __init sam9x60_aic_irq_fixup(void)
+{
+	aic_common_rtc_irq_fixup();
+	aic_common_rtt_irq_fixup();
+}
+
 static const struct of_device_id aic5_irq_fixups[] __initconst = {
 	{ .compatible = "atmel,sama5d3", .data = sama5d3_aic_irq_fixup },
 	{ .compatible = "atmel,sama5d4", .data = sama5d3_aic_irq_fixup },
-	{ .compatible = "microchip,sam9x60", .data = sama5d3_aic_irq_fixup },
+	{ .compatible = "microchip,sam9x60", .data = sam9x60_aic_irq_fixup },
 	{ /* sentinel */ },
 };
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/5] rtc: at91sam9: enable driver for sam9x60
  2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 1/5] irqchip/atmel-aic5: add support for sam9x60 rtt fixup Claudiu Beznea
@ 2020-04-10 16:26 ` Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 3/5] ARM: dts: sam9x60ek: enable gpbr Claudiu Beznea
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

Enable driver for SAM9X60.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/rtc/rtc-at91sam9.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index e39e89867d29..cb0a8f4a73a4 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -525,6 +525,7 @@ static SIMPLE_DEV_PM_OPS(at91_rtc_pm_ops, at91_rtc_suspend, at91_rtc_resume);
 
 static const struct of_device_id at91_rtc_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9260-rtt" },
+	{ .compatible = "microchip,sam9x60-rtt" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/5] ARM: dts: sam9x60ek: enable gpbr
  2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 1/5] irqchip/atmel-aic5: add support for sam9x60 rtt fixup Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 2/5] rtc: at91sam9: enable driver for sam9x60 Claudiu Beznea
@ 2020-04-10 16:26 ` Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 4/5] ARM: dts: sam9x60: add rtt Claudiu Beznea
  2020-04-10 16:26 ` [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt Claudiu Beznea
  4 siblings, 0 replies; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

Enable gpbr.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91-sam9x60ek.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
index 9f30132d7d7b..ab3d2d9a420a 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -309,6 +309,10 @@
 	};
 };
 
+&gpbr {
+	status = "okay";
+};
+
 &i2s {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2s_default>;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
                   ` (2 preceding siblings ...)
  2020-04-10 16:26 ` [PATCH 3/5] ARM: dts: sam9x60ek: enable gpbr Claudiu Beznea
@ 2020-04-10 16:26 ` Claudiu Beznea
  2020-04-10 22:26   ` Alexandre Belloni
  2020-04-10 16:26 ` [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt Claudiu Beznea
  4 siblings, 1 reply; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

Add RTT.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91-sam9x60ek.dts | 5 +++++
 arch/arm/boot/dts/sam9x60.dtsi       | 7 +++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
index ab3d2d9a420a..4020e79a958e 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -617,6 +617,11 @@
 	};
 };
 
+&rtt {
+	atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
+	status = "okay";
+};
+
 &shutdown_controller {
 	atmel,shdwc-debouncer = <976>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index 326b39328b58..e1d8e3a4cb0b 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -661,6 +661,13 @@
 				status = "disabled";
 			};
 
+			rtt: rtt@fffffe20 {
+				compatible = "microchip,sam9x60-rtt";
+				reg = <0xfffffe20 0x20>;
+				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
+				clocks = <&clk32k 0>;
+			};
+
 			pit: timer@fffffe40 {
 				compatible = "atmel,at91sam9260-pit";
 				reg = <0xfffffe40 0x10>;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt
  2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
                   ` (3 preceding siblings ...)
  2020-04-10 16:26 ` [PATCH 4/5] ARM: dts: sam9x60: add rtt Claudiu Beznea
@ 2020-04-10 16:26 ` Claudiu Beznea
  2020-04-10 17:41   ` Alexandre Belloni
  4 siblings, 1 reply; 19+ messages in thread
From: Claudiu Beznea @ 2020-04-10 16:26 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, mark.rutland, nicolas.ferre,
	ludovic.desroches, tglx, jason, maz
  Cc: linux-rtc, devicetree, linux-arm-kernel, linux-kernel, Claudiu Beznea

Add microchip,sam9x60-rtt to compatible list.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
index 6ae79d1843f3..b2f913ff6c69 100644
--- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
@@ -1,7 +1,7 @@
 Atmel AT91SAM9260 Real Time Timer
 
 Required properties:
-- compatible: should be: "atmel,at91sam9260-rtt"
+- compatible: should be: "atmel,at91sam9260-rtt" or "microchip,sam9x60-rtt"
 - reg: should encode the memory region of the RTT controller
 - interrupts: rtt alarm/event interrupt
 - clocks: should contain the 32 KHz slow clk that will drive the RTT block.
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt
  2020-04-10 16:26 ` [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt Claudiu Beznea
@ 2020-04-10 17:41   ` Alexandre Belloni
  2020-04-13  8:50     ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-10 17:41 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: a.zummo, robh+dt, mark.rutland, nicolas.ferre, ludovic.desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

Hi,

This patch must come first. Also, the correct prefix is
dt-bindings: rtc:

On 10/04/2020 19:26:59+0300, Claudiu Beznea wrote:
> Add microchip,sam9x60-rtt to compatible list.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> index 6ae79d1843f3..b2f913ff6c69 100644
> --- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> @@ -1,7 +1,7 @@
>  Atmel AT91SAM9260 Real Time Timer
>  
>  Required properties:
> -- compatible: should be: "atmel,at91sam9260-rtt"
> +- compatible: should be: "atmel,at91sam9260-rtt" or "microchip,sam9x60-rtt"

If you had a fallback on atmel,at91sam9260-rtt, this would make merging
this series easier as this removes the need for 1/5 and 2/5.

I think 2/5 may be useful in the future but as far as the aic fixup
is concerned, both IPs are identical.

>  - reg: should encode the memory region of the RTT controller
>  - interrupts: rtt alarm/event interrupt
>  - clocks: should contain the 32 KHz slow clk that will drive the RTT block.
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-10 16:26 ` [PATCH 4/5] ARM: dts: sam9x60: add rtt Claudiu Beznea
@ 2020-04-10 22:26   ` Alexandre Belloni
  2020-04-13  8:51     ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-10 22:26 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: a.zummo, robh+dt, mark.rutland, nicolas.ferre, ludovic.desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

On 10/04/2020 19:26:58+0300, Claudiu Beznea wrote:
> Add RTT.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  arch/arm/boot/dts/at91-sam9x60ek.dts | 5 +++++
>  arch/arm/boot/dts/sam9x60.dtsi       | 7 +++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
> index ab3d2d9a420a..4020e79a958e 100644
> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
> @@ -617,6 +617,11 @@
>  	};
>  };
>  
> +&rtt {
> +	atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> +	status = "okay";

Is there any point using a gpbr register while there is already a much
better RTC in the system?

In any case, this diff should be merge with the other at91-sam9x60ek.dts
change instead of being with the dtsi change.

> +};
> +
>  &shutdown_controller {
>  	atmel,shdwc-debouncer = <976>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
> index 326b39328b58..e1d8e3a4cb0b 100644
> --- a/arch/arm/boot/dts/sam9x60.dtsi
> +++ b/arch/arm/boot/dts/sam9x60.dtsi
> @@ -661,6 +661,13 @@
>  				status = "disabled";
>  			};
>  
> +			rtt: rtt@fffffe20 {
> +				compatible = "microchip,sam9x60-rtt";
> +				reg = <0xfffffe20 0x20>;
> +				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
> +				clocks = <&clk32k 0>;
> +			};
> +
>  			pit: timer@fffffe40 {
>  				compatible = "atmel,at91sam9260-pit";
>  				reg = <0xfffffe40 0x10>;
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt
  2020-04-10 17:41   ` Alexandre Belloni
@ 2020-04-13  8:50     ` Claudiu.Beznea
  2020-04-13 10:41       ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-13  8:50 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: mark.rutland, a.zummo, jason, devicetree, maz, linux-kernel,
	Ludovic.Desroches, robh+dt, tglx, linux-arm-kernel, linux-rtc



On 10.04.2020 20:41, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> This patch must come first.

OK!

 Also, the correct prefix is
> dt-bindings: rtc:

OK! I used the prefix that was previously used.

> 
> On 10/04/2020 19:26:59+0300, Claudiu Beznea wrote:
>> Add microchip,sam9x60-rtt to compatible list.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> index 6ae79d1843f3..b2f913ff6c69 100644
>> --- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> @@ -1,7 +1,7 @@
>>  Atmel AT91SAM9260 Real Time Timer
>>
>>  Required properties:
>> -- compatible: should be: "atmel,at91sam9260-rtt"
>> +- compatible: should be: "atmel,at91sam9260-rtt" or "microchip,sam9x60-rtt"
> 
> If you had a fallback on atmel,at91sam9260-rtt, this would make merging
> this series easier as this removes the need for 1/5 and 2/5.

The:

diff --git a/drivers/irqchip/irq-atmel-aic5.c
b/drivers/irqchip/irq-atmel-aic5.c
index 29333497ba10..74a66cd05553 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -310,10 +310,16 @@ static void __init sama5d3_aic_irq_fixup(void)
 	aic_common_rtc_irq_fixup();
 }

+static void __init sam9x60_aic_irq_fixup(void)
+{
+	aic_common_rtc_irq_fixup();
+	aic_common_rtt_irq_fixup();
+}
+
 static const struct of_device_id aic5_irq_fixups[] __initconst = {
 	{ .compatible = "atmel,sama5d3", .data = sama5d3_aic_irq_fixup },
 	{ .compatible = "atmel,sama5d4", .data = sama5d3_aic_irq_fixup },
-	{ .compatible = "microchip,sam9x60", .data = sama5d3_aic_irq_fixup },
+	{ .compatible = "microchip,sam9x60", .data = sam9x60_aic_irq_fixup },
 	{ /* sentinel */ },
 };

part of 1/5 is still necessary.

Regarding the fallback to "atmel,at91sam9260-rtt" I am aware of that. I
chose this approach because this IP is a bit different than the one with
"atmel,at91sam9260-rtt" compatible, meaning it has a features that the old
one has not. I'm talking about [1] which I cannot see on a SAM9G45 [2]
where RTT IP uses "atmel,at91sam9260-rtt" as compatible.

Is true it may be necessary in the future when new features may be
implemented. Taking this into account, would you like to get rid of the new
compatible in code and keep it only in device tree?

[1]
http://ww1.microchip.com/downloads/en/DeviceDoc/SAM9X60-Data-Sheet-DS60001579B.pdf#_OPENTOPIC_TOC_PROCESSING_d137e64502
[2]
http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6438-32-bit-ARM926-Embedded-Microprocessor-SAM9G45_Datasheet.pdf

> 
> I think 2/5 may be useful in the future but as far as the aic fixup
> is concerned, both IPs are identical.
> 
>>  - reg: should encode the memory region of the RTT controller
>>  - interrupts: rtt alarm/event interrupt
>>  - clocks: should contain the 32 KHz slow clk that will drive the RTT block.
>> --
>> 2.7.4
>>
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-10 22:26   ` Alexandre Belloni
@ 2020-04-13  8:51     ` Claudiu.Beznea
  2020-04-13 10:46       ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-13  8:51 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel



On 11.04.2020 01:26, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/04/2020 19:26:58+0300, Claudiu Beznea wrote:
>> Add RTT.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  arch/arm/boot/dts/at91-sam9x60ek.dts | 5 +++++
>>  arch/arm/boot/dts/sam9x60.dtsi       | 7 +++++++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
>> index ab3d2d9a420a..4020e79a958e 100644
>> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
>> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
>> @@ -617,6 +617,11 @@
>>       };
>>  };
>>
>> +&rtt {
>> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>> +     status = "okay";
> 
> Is there any point using a gpbr register while there is already a much
> better RTC in the system?

Only to have it also enabled.

> 
> In any case, this diff should be merge with the other at91-sam9x60ek.dts
> change instead of being with the dtsi change.

The changes in this patch are related to enabling the RTT. The other dts
change is related to enabling gpbr. The RTT uses that enabled gpbr -> one
change per patch.

If you still want to merge then, I'll do it, but then it becomes mixed.

> 
>> +};
>> +
>>  &shutdown_controller {
>>       atmel,shdwc-debouncer = <976>;
>>       status = "okay";
>> diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
>> index 326b39328b58..e1d8e3a4cb0b 100644
>> --- a/arch/arm/boot/dts/sam9x60.dtsi
>> +++ b/arch/arm/boot/dts/sam9x60.dtsi
>> @@ -661,6 +661,13 @@
>>                               status = "disabled";
>>                       };
>>
>> +                     rtt: rtt@fffffe20 {
>> +                             compatible = "microchip,sam9x60-rtt";
>> +                             reg = <0xfffffe20 0x20>;
>> +                             interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
>> +                             clocks = <&clk32k 0>;
>> +                     };
>> +
>>                       pit: timer@fffffe40 {
>>                               compatible = "atmel,at91sam9260-pit";
>>                               reg = <0xfffffe40 0x10>;
>> --
>> 2.7.4
>>
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt
  2020-04-13  8:50     ` Claudiu.Beznea
@ 2020-04-13 10:41       ` Alexandre Belloni
  0 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-13 10:41 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: mark.rutland, a.zummo, jason, devicetree, maz, linux-kernel,
	Ludovic.Desroches, robh+dt, tglx, linux-arm-kernel, linux-rtc

On 13/04/2020 08:50:02+0000, Claudiu.Beznea@microchip.com wrote:
> part of 1/5 is still necessary.
> 

indeed.

> Regarding the fallback to "atmel,at91sam9260-rtt" I am aware of that. I
> chose this approach because this IP is a bit different than the one with
> "atmel,at91sam9260-rtt" compatible, meaning it has a features that the old
> one has not. I'm talking about [1] which I cannot see on a SAM9G45 [2]
> where RTT IP uses "atmel,at91sam9260-rtt" as compatible.
> 
> Is true it may be necessary in the future when new features may be
> implemented. Taking this into account, would you like to get rid of the new
> compatible in code and keep it only in device tree?
> 

What I said is not that the new compatible is not necessary at all but
that it can be avoided in the aic code.

> [1]
> http://ww1.microchip.com/downloads/en/DeviceDoc/SAM9X60-Data-Sheet-DS60001579B.pdf#_OPENTOPIC_TOC_PROCESSING_d137e64502
> [2]
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6438-32-bit-ARM926-Embedded-Microprocessor-SAM9G45_Datasheet.pdf
> 
> > 
> > I think 2/5 may be useful in the future but as far as the aic fixup
> > is concerned, both IPs are identical.
> > 
> >>  - reg: should encode the memory region of the RTT controller
> >>  - interrupts: rtt alarm/event interrupt
> >>  - clocks: should contain the 32 KHz slow clk that will drive the RTT block.
> >> --
> >> 2.7.4
> >>
> > 
> > --
> > Alexandre Belloni, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-13  8:51     ` Claudiu.Beznea
@ 2020-04-13 10:46       ` Alexandre Belloni
  2020-04-14  8:42         ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-13 10:46 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

On 13/04/2020 08:51:12+0000, Claudiu.Beznea@microchip.com wrote:
> 
> 
> On 11.04.2020 01:26, Alexandre Belloni wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 10/04/2020 19:26:58+0300, Claudiu Beznea wrote:
> >> Add RTT.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >> ---
> >>  arch/arm/boot/dts/at91-sam9x60ek.dts | 5 +++++
> >>  arch/arm/boot/dts/sam9x60.dtsi       | 7 +++++++
> >>  2 files changed, 12 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
> >> index ab3d2d9a420a..4020e79a958e 100644
> >> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
> >> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
> >> @@ -617,6 +617,11 @@
> >>       };
> >>  };
> >>
> >> +&rtt {
> >> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> >> +     status = "okay";
> > 
> > Is there any point using a gpbr register while there is already a much
> > better RTC in the system?
> 
> Only to have it also enabled.
> 

Why would one use the RTT while the RTC is far superior?

> > 
> > In any case, this diff should be merge with the other at91-sam9x60ek.dts
> > change instead of being with the dtsi change.
> 
> The changes in this patch are related to enabling the RTT. The other dts
> change is related to enabling gpbr. The RTT uses that enabled gpbr -> one
> change per patch.
> 
> If you still want to merge then, I'll do it, but then it becomes mixed.
> 

This patch is already mixing add the gpbr in sam9x60ek and add the node
in sam9x60.dtsi which is worse.

Just have one patch adding the rtt node to the sam9x60.dtsi and then a
patch adding the RTT to sam9x60ek. Because the RTT uses the gpbr, it is
a good time to add enable the gpbr, this is a single functionnal change.

Let's say that for some reason, the RTT patch on sam9x60ek has to be
reverted, then the RTT node is still defined which is good for all the
other eventual users.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-13 10:46       ` Alexandre Belloni
@ 2020-04-14  8:42         ` Claudiu.Beznea
  2020-04-14 11:16           ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-14  8:42 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel



On 13.04.2020 13:46, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 13/04/2020 08:51:12+0000, Claudiu.Beznea@microchip.com wrote:
>>
>>
>> On 11.04.2020 01:26, Alexandre Belloni wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/04/2020 19:26:58+0300, Claudiu Beznea wrote:
>>>> Add RTT.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>> ---
>>>>  arch/arm/boot/dts/at91-sam9x60ek.dts | 5 +++++
>>>>  arch/arm/boot/dts/sam9x60.dtsi       | 7 +++++++
>>>>  2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
>>>> index ab3d2d9a420a..4020e79a958e 100644
>>>> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
>>>> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
>>>> @@ -617,6 +617,11 @@
>>>>       };
>>>>  };
>>>>
>>>> +&rtt {
>>>> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>>>> +     status = "okay";
>>>
>>> Is there any point using a gpbr register while there is already a much
>>> better RTC in the system?
>>
>> Only to have it also enabled.
>>
> 
> Why would one use the RTT while the RTC is far superior?

I didn't enabled this for a particular use case, but: couldn't this be used
by some user that wants to generate multiple alarms? from multiple RTCs?

Moreover, this IP's counter has the possibility of being clocked at 1Hz.
Couldn't this minimize the power consumption while being in a power saving
mode?

> 
>>>
>>> In any case, this diff should be merge with the other at91-sam9x60ek.dts
>>> change instead of being with the dtsi change.
>>
>> The changes in this patch are related to enabling the RTT. The other dts
>> change is related to enabling gpbr. The RTT uses that enabled gpbr -> one
>> change per patch.
>>
>> If you still want to merge then, I'll do it, but then it becomes mixed.
>>
> 
> This patch is already mixing add the gpbr in sam9x60ek and add the node
> in sam9x60.dtsi which is worse.

This patch is like this:

diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts
b/arch/arm/boot/dts/at91-sam9x60ek.dts
index ab3d2d9a420a..4020e79a958e 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -617,6 +617,11 @@
 	};
 };

+&rtt {
+	atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
+	status = "okay";
+};
+
 &shutdown_controller {
 	atmel,shdwc-debouncer = <976>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index 326b39328b58..e1d8e3a4cb0b 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -661,6 +661,13 @@
 				status = "disabled";
 			};

+			rtt: rtt@fffffe20 {
+				compatible = "microchip,sam9x60-rtt";
+				reg = <0xfffffe20 0x20>;
+				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
+				clocks = <&clk32k 0>;
+			};
+

It doesn't adds the GPBR in sam9x60ek, it adds rtt in sam9x60ek which uses
GPBR.

> 
> Just have one patch adding the rtt node to the sam9x60.dtsi and then a
> patch adding the RTT to sam9x60ek.

Ok, I understand this.

> Because the RTT uses the gpbr, it is
> a good time to add enable the gpbr, this is a single functionnal change.
> 
> Let's say that for some reason, the RTT patch on sam9x60ek has to be
> reverted, then the RTT node is still defined which is good for all the
> other eventual users.

RTT node would still be defined but GPBR node will not be enabled.

If RTT patch contains this change that I understand you want me to merge here:

+&gpbr {
+	status = "okay";
+};
+

then, theoretically, some other IPs using the GPBR (RTC have the
possibility of doing this), may be broken by reverting the RTT patch that
includes the GPBR enabling patch.

> 
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14  8:42         ` Claudiu.Beznea
@ 2020-04-14 11:16           ` Alexandre Belloni
  2020-04-14 12:13             ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-14 11:16 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

On 14/04/2020 08:42:08+0000, Claudiu.Beznea@microchip.com wrote:
> > Why would one use the RTT while the RTC is far superior?
> 
> I didn't enabled this for a particular use case, but: couldn't this be used
> by some user that wants to generate multiple alarms? from multiple RTCs?
> 

I very much doubt that as Linux is able to properly multiplex alarms and
basically, the only one we are interested in is actually wakeup.

> Moreover, this IP's counter has the possibility of being clocked at 1Hz.
> Couldn't this minimize the power consumption while being in a power saving
> mode?
> 

And that 1Hz clock is coming from the RTC so using the RTC is
definitively consuming less power.

> > 
> >>>
> >>> In any case, this diff should be merge with the other at91-sam9x60ek.dts
> >>> change instead of being with the dtsi change.
> >>
> >> The changes in this patch are related to enabling the RTT. The other dts
> >> change is related to enabling gpbr. The RTT uses that enabled gpbr -> one
> >> change per patch.
> >>
> >> If you still want to merge then, I'll do it, but then it becomes mixed.
> >>
> > 
> > This patch is already mixing add the gpbr in sam9x60ek and add the node
> > in sam9x60.dtsi which is worse.
> 
> This patch is like this:
> 
> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts
> b/arch/arm/boot/dts/at91-sam9x60ek.dts
> index ab3d2d9a420a..4020e79a958e 100644
> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
> @@ -617,6 +617,11 @@
>  	};
>  };
> 
> +&rtt {
> +	atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> +	status = "okay";
> +};
> +
>  &shutdown_controller {
>  	atmel,shdwc-debouncer = <976>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
> index 326b39328b58..e1d8e3a4cb0b 100644
> --- a/arch/arm/boot/dts/sam9x60.dtsi
> +++ b/arch/arm/boot/dts/sam9x60.dtsi
> @@ -661,6 +661,13 @@
>  				status = "disabled";
>  			};
> 
> +			rtt: rtt@fffffe20 {
> +				compatible = "microchip,sam9x60-rtt";
> +				reg = <0xfffffe20 0x20>;
> +				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
> +				clocks = <&clk32k 0>;
> +			};
> +
> 
> It doesn't adds the GPBR in sam9x60ek, it adds rtt in sam9x60ek which uses
> GPBR.
> 
> > 
> > Just have one patch adding the rtt node to the sam9x60.dtsi and then a
> > patch adding the RTT to sam9x60ek.
> 
> Ok, I understand this.
> 
> > Because the RTT uses the gpbr, it is
> > a good time to add enable the gpbr, this is a single functionnal change.
> > 
> > Let's say that for some reason, the RTT patch on sam9x60ek has to be
> > reverted, then the RTT node is still defined which is good for all the
> > other eventual users.
> 
> RTT node would still be defined but GPBR node will not be enabled.
> 
> If RTT patch contains this change that I understand you want me to merge here:
> 
> +&gpbr {
> +	status = "okay";
> +};
> +
> 
> then, theoretically, some other IPs using the GPBR (RTC have the
> possibility of doing this), may be broken by reverting the RTT patch that
> includes the GPBR enabling patch.
> 

But this is very unlikely to happen because this would be limited to a
single board device tree instead of impact every sam9x60 based boards.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14 11:16           ` Alexandre Belloni
@ 2020-04-14 12:13             ` Claudiu.Beznea
  2020-04-14 12:47               ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-14 12:13 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel



On 14.04.2020 14:16, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 14/04/2020 08:42:08+0000, Claudiu.Beznea@microchip.com wrote:
>>> Why would one use the RTT while the RTC is far superior?
>>
>> I didn't enabled this for a particular use case, but: couldn't this be used
>> by some user that wants to generate multiple alarms? from multiple RTCs?
>>
> 
> I very much doubt that as Linux is able to properly multiplex alarms and
> basically, the only one we are interested in is actually wakeup.

I think you can use the wakealarm sysfs exported file to prepare an alarm
and take user space actions based on that without being suspended.

> 
>> Moreover, this IP's counter has the possibility of being clocked at 1Hz.
>> Couldn't this minimize the power consumption while being in a power saving
>> mode?
>>
> 
> And that 1Hz clock is coming from the RTC so using the RTC is
> definitively consuming less power.

Datasheet specifies this: "Configuring the RTPRES field value to 0x8000
(default value) corresponds to feeding the real-time counter with a

1Hz signal (if the slow clock is 32.768 kHz)."

So, it is not the RTC, it is the slow clock divided by 32768.

> 
>>>
>>>>>
>>>>> In any case, this diff should be merge with the other at91-sam9x60ek.dts
>>>>> change instead of being with the dtsi change.
>>>>
>>>> The changes in this patch are related to enabling the RTT. The other dts
>>>> change is related to enabling gpbr. The RTT uses that enabled gpbr -> one
>>>> change per patch.
>>>>
>>>> If you still want to merge then, I'll do it, but then it becomes mixed.
>>>>
>>>
>>> This patch is already mixing add the gpbr in sam9x60ek and add the node
>>> in sam9x60.dtsi which is worse.
>>
>> This patch is like this:
>>
>> diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts
>> b/arch/arm/boot/dts/at91-sam9x60ek.dts
>> index ab3d2d9a420a..4020e79a958e 100644
>> --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
>> +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
>> @@ -617,6 +617,11 @@
>>       };
>>  };
>>
>> +&rtt {
>> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>> +     status = "okay";
>> +};
>> +
>>  &shutdown_controller {
>>       atmel,shdwc-debouncer = <976>;
>>       status = "okay";
>> diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
>> index 326b39328b58..e1d8e3a4cb0b 100644
>> --- a/arch/arm/boot/dts/sam9x60.dtsi
>> +++ b/arch/arm/boot/dts/sam9x60.dtsi
>> @@ -661,6 +661,13 @@
>>                               status = "disabled";
>>                       };
>>
>> +                     rtt: rtt@fffffe20 {
>> +                             compatible = "microchip,sam9x60-rtt";
>> +                             reg = <0xfffffe20 0x20>;
>> +                             interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
>> +                             clocks = <&clk32k 0>;
>> +                     };
>> +
>>
>> It doesn't adds the GPBR in sam9x60ek, it adds rtt in sam9x60ek which uses
>> GPBR.
>>
>>>
>>> Just have one patch adding the rtt node to the sam9x60.dtsi and then a
>>> patch adding the RTT to sam9x60ek.
>>
>> Ok, I understand this.
>>
>>> Because the RTT uses the gpbr, it is
>>> a good time to add enable the gpbr, this is a single functionnal change.
>>>
>>> Let's say that for some reason, the RTT patch on sam9x60ek has to be
>>> reverted, then the RTT node is still defined which is good for all the
>>> other eventual users.
>>
>> RTT node would still be defined but GPBR node will not be enabled.
>>
>> If RTT patch contains this change that I understand you want me to merge here:
>>
>> +&gpbr {
>> +     status = "okay";
>> +};
>> +
>>
>> then, theoretically, some other IPs using the GPBR (RTC have the
>> possibility of doing this), may be broken by reverting the RTT patch that
>> includes the GPBR enabling patch.
>>
> 
> But this is very unlikely to happen because this would be limited to a
> single board device tree instead of impact every sam9x60 based boards.

Very unlikely but a having a patch with diff like this:

+&gpbr {
+     status = "okay";
+};
+
+&rtt {
+     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
+     status = "okay";
+};
+

and reverting it may affect the other users of gpbr in sam9x60ek.dts.

> 
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14 12:13             ` Claudiu.Beznea
@ 2020-04-14 12:47               ` Alexandre Belloni
  2020-04-14 13:05                 ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-14 12:47 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

On 14/04/2020 12:13:46+0000, Claudiu.Beznea@microchip.com wrote:
> 
> 
> On 14.04.2020 14:16, Alexandre Belloni wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 14/04/2020 08:42:08+0000, Claudiu.Beznea@microchip.com wrote:
> >>> Why would one use the RTT while the RTC is far superior?
> >>
> >> I didn't enabled this for a particular use case, but: couldn't this be used
> >> by some user that wants to generate multiple alarms? from multiple RTCs?
> >>
> > 
> > I very much doubt that as Linux is able to properly multiplex alarms and
> > basically, the only one we are interested in is actually wakeup.
> 
> I think you can use the wakealarm sysfs exported file to prepare an alarm
> and take user space actions based on that without being suspended.
> 
> > 
> >> Moreover, this IP's counter has the possibility of being clocked at 1Hz.
> >> Couldn't this minimize the power consumption while being in a power saving
> >> mode?
> >>
> > 
> > And that 1Hz clock is coming from the RTC so using the RTC is
> > definitively consuming less power.
> 
> Datasheet specifies this: "Configuring the RTPRES field value to 0x8000
> (default value) corresponds to feeding the real-time counter with a
> 
> 1Hz signal (if the slow clock is 32.768 kHz)."
> 
> So, it is not the RTC, it is the slow clock divided by 32768.

This is not what you described previously, using RTPRES means running
the RTT at 32kHz. This is exactly what happens with the RTC but you get
the added clock calibration circuitry that is probably not drawing to
much power but the added consumption of the configurable prescaler
versus the static prescaler of the RTC is probably similar.

Using RTC1HZ would be driving the RTT at 1Hz.

> > But this is very unlikely to happen because this would be limited to a
> > single board device tree instead of impact every sam9x60 based boards.
> 
> Very unlikely but a having a patch with diff like this:
> 
> +&gpbr {
> +     status = "okay";
> +};
> +
> +&rtt {
> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> +     status = "okay";
> +};
> +
> 
> and reverting it may affect the other users of gpbr in sam9x60ek.dts.
> 

Again, this affects only sam9x60ek.dts instead of possibly multiple DTs
that may be out of tree. So the risk of doing that is null.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14 12:47               ` Alexandre Belloni
@ 2020-04-14 13:05                 ` Claudiu.Beznea
  2020-04-14 13:12                   ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-14 13:05 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel



On 14.04.2020 15:47, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 14/04/2020 12:13:46+0000, Claudiu.Beznea@microchip.com wrote:
>>
>>
>> On 14.04.2020 14:16, Alexandre Belloni wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 14/04/2020 08:42:08+0000, Claudiu.Beznea@microchip.com wrote:
>>>>> Why would one use the RTT while the RTC is far superior?
>>>>
>>>> I didn't enabled this for a particular use case, but: couldn't this be used
>>>> by some user that wants to generate multiple alarms? from multiple RTCs?
>>>>
>>>
>>> I very much doubt that as Linux is able to properly multiplex alarms and
>>> basically, the only one we are interested in is actually wakeup.
>>
>> I think you can use the wakealarm sysfs exported file to prepare an alarm
>> and take user space actions based on that without being suspended.
>>
>>>
>>>> Moreover, this IP's counter has the possibility of being clocked at 1Hz.
>>>> Couldn't this minimize the power consumption while being in a power saving
>>>> mode?
>>>>
>>>
>>> And that 1Hz clock is coming from the RTC so using the RTC is
>>> definitively consuming less power.
>>
>> Datasheet specifies this: "Configuring the RTPRES field value to 0x8000
>> (default value) corresponds to feeding the real-time counter with a
>>
>> 1Hz signal (if the slow clock is 32.768 kHz)."
>>
>> So, it is not the RTC, it is the slow clock divided by 32768.
> 
> This is not what you described previously,

I said this way: "this *IP's counter* has the possibility of being clocked at 1Hz"

> using RTPRES means running
> the RTT at 32kHz. This is exactly what happens with the RTC but you get
> the added clock calibration circuitry that is probably not drawing to
> much power but the added consumption of the configurable prescaler
> versus the static prescaler of the RTC is probably similar.
> 
> Using RTC1HZ would be driving the RTT at 1Hz.
> 
>>> But this is very unlikely to happen because this would be limited to a
>>> single board device tree instead of impact every sam9x60 based boards.
>>
>> Very unlikely but a having a patch with diff like this:
>>
>> +&gpbr {
>> +     status = "okay";
>> +};
>> +
>> +&rtt {
>> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>> +     status = "okay";
>> +};
>> +
>>
>> and reverting it may affect the other users of gpbr in sam9x60ek.dts.
>>
> 
> Again, this affects only sam9x60ek.dts instead of possibly multiple DTs
> that may be out of tree. So the risk of doing that is null.

Anyway... I'll merge it although I don't consider is the right way.

> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14 13:05                 ` Claudiu.Beznea
@ 2020-04-14 13:12                   ` Alexandre Belloni
  2020-04-14 13:59                     ` Claudiu.Beznea
  0 siblings, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2020-04-14 13:12 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: a.zummo, robh+dt, mark.rutland, Nicolas.Ferre, Ludovic.Desroches,
	tglx, jason, maz, linux-rtc, devicetree, linux-arm-kernel,
	linux-kernel

On 14/04/2020 13:05:03+0000, Claudiu.Beznea@microchip.com wrote:
> >>> But this is very unlikely to happen because this would be limited to a
> >>> single board device tree instead of impact every sam9x60 based boards.
> >>
> >> Very unlikely but a having a patch with diff like this:
> >>
> >> +&gpbr {
> >> +     status = "okay";
> >> +};
> >> +
> >> +&rtt {
> >> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> >> +     status = "okay";
> >> +};
> >> +
> >>
> >> and reverting it may affect the other users of gpbr in sam9x60ek.dts.
> >>
> > 
> > Again, this affects only sam9x60ek.dts instead of possibly multiple DTs
> > that may be out of tree. So the risk of doing that is null.
> 
> Anyway... I'll merge it although I don't consider is the right way.
> 

Do as you wish but a board DT change mixed with a dtsi is a no go.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] ARM: dts: sam9x60: add rtt
  2020-04-14 13:12                   ` Alexandre Belloni
@ 2020-04-14 13:59                     ` Claudiu.Beznea
  0 siblings, 0 replies; 19+ messages in thread
From: Claudiu.Beznea @ 2020-04-14 13:59 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: mark.rutland, a.zummo, jason, devicetree, maz, linux-kernel,
	Ludovic.Desroches, robh+dt, tglx, linux-arm-kernel, linux-rtc



On 14.04.2020 16:12, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 14/04/2020 13:05:03+0000, Claudiu.Beznea@microchip.com wrote:
>>>>> But this is very unlikely to happen because this would be limited to a
>>>>> single board device tree instead of impact every sam9x60 based boards.
>>>>
>>>> Very unlikely but a having a patch with diff like this:
>>>>
>>>> +&gpbr {
>>>> +     status = "okay";
>>>> +};
>>>> +
>>>> +&rtt {
>>>> +     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>>>> +     status = "okay";
>>>> +};
>>>> +
>>>>
>>>> and reverting it may affect the other users of gpbr in sam9x60ek.dts.
>>>>
>>>
>>> Again, this affects only sam9x60ek.dts instead of possibly multiple DTs
>>> that may be out of tree. So the risk of doing that is null.
>>
>> Anyway... I'll merge it although I don't consider is the right way.
>>
> 
> Do as you wish but a board DT change mixed with a dtsi is a no go.

I was talking about mixing these:

+&gpbr {
+     status = "okay";
+};
+
+&rtt {
+     atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
+     status = "okay";
+};
+

> 
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-04-14 14:00 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-10 16:26 [PATCH 0/5] add RTT support for SAM9X60 Claudiu Beznea
2020-04-10 16:26 ` [PATCH 1/5] irqchip/atmel-aic5: add support for sam9x60 rtt fixup Claudiu Beznea
2020-04-10 16:26 ` [PATCH 2/5] rtc: at91sam9: enable driver for sam9x60 Claudiu Beznea
2020-04-10 16:26 ` [PATCH 3/5] ARM: dts: sam9x60ek: enable gpbr Claudiu Beznea
2020-04-10 16:26 ` [PATCH 4/5] ARM: dts: sam9x60: add rtt Claudiu Beznea
2020-04-10 22:26   ` Alexandre Belloni
2020-04-13  8:51     ` Claudiu.Beznea
2020-04-13 10:46       ` Alexandre Belloni
2020-04-14  8:42         ` Claudiu.Beznea
2020-04-14 11:16           ` Alexandre Belloni
2020-04-14 12:13             ` Claudiu.Beznea
2020-04-14 12:47               ` Alexandre Belloni
2020-04-14 13:05                 ` Claudiu.Beznea
2020-04-14 13:12                   ` Alexandre Belloni
2020-04-14 13:59                     ` Claudiu.Beznea
2020-04-10 16:26 ` [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt Claudiu Beznea
2020-04-10 17:41   ` Alexandre Belloni
2020-04-13  8:50     ` Claudiu.Beznea
2020-04-13 10:41       ` Alexandre Belloni

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).