Re: [PATCH 5/5] rtc: at91sam9: add microchip,sam9x60-rtt

[<Claudiu.Beznea@microchip.com>]

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
>