[PATCH v2 2/3] dts: sama5d2: add TCB node

[Eugen.Hristev at microchip.com]

On 2/4/22 10:37 AM, Eugen Hristev - M18282 wrote:
> On 2/4/22 10:04 AM, Clément Léger wrote:
>> Le Fri, 4 Feb 2022 07:52:26 +0000,
>> <Eugen.Hristev at microchip.com> a écrit :
>>
>>> Hello Clement,
>>>
>>> Subject should be ARM: dts: [at91:] sama5d2: ...
>>>
>>> On 2/2/22 4:43 PM, Clément Léger wrote:
>>>> Add the device-tree node to describe the TCB timer.
>>>>
>>>> Signed-off-by: Clément Léger <clement.leger at bootlin.com>
>>>> ---
>>>>     arch/arm/dts/sama5d2.dtsi | 15 +++++++++++++++
>>>>     1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi
>>>> index 038cd73c03..fc6a4fbe4d 100644
>>>> --- a/arch/arm/dts/sama5d2.dtsi
>>>> +++ b/arch/arm/dts/sama5d2.dtsi
>>>> @@ -700,6 +700,21 @@
>>>>                                    clocks = <&h32ck>;
>>>>                            };
>>>>
>>>> +                       tcb0: timer at f800c000 {
>>>> +                               compatible = "atmel,sama5d2-tcb", "simple-mfd";
>>>
>>> syscon ?
>>
>> Hi Eugen,
>>
>> Yes I might add it if needed but in my case, there is no need for it.
> 
> True, but the binding says that the compatible *must* be the three
> stated... so we must comply to that, regardless if it's a need for it or
> not in U-boot.
> 
>>
>>>
>>>> +                               reg = <0xf800c000 0x100>;
>>>> +
>>>> +                               clocks = <&tcb0_clk>, <&tcb0_gclk>, <&clk32k>;
>>>> +                               clock-names = "t0_clk", "gclk", "slow_clk";
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               timer0: timer at 0 {
>>>> +                                       compatible = "atmel,tcb-timer";
>>>> +                                       reg = <0>, <1>;
>>>> +                               };
>>>> +                       };
>>>> +
>>>
>>> I am not happy that the original binding has the interrupts as
>>> 'mandatory'. Maybe the binding author did not have more use cases in mind.
>>> Anyway I think that it can go to u-boot without this interrupts property
>>> as it's surely unused and there is no interrupt controller at the moment
>>> in the DT.
>>
>> I checked the other nodes and indeed, I did not found any 'interrupts'
>> property so i guess it 'could' be left out... But I can add it for sure.
> 
> I sent a patch in Linux to remove interrupts from mandatory. Let's have
> the node without it for now.
> 
> Your series will have to wait a little bit anyway for the next merge
> window, and a review on the driver.

Hello Clement,

According to this :

https://lore.kernel.org/linux-arm-kernel/Yf0Bkh4pXKORmNkG@piout.net/

... the interrupt is mandatory.

Eugen

> 
> Thanks again for your patches,
> 
> Eugen
> 
>>
>>> If nobody has another opinion that is...
>>>
>>> Eugen
>>>
>>>>                            watchdog at f8048040 {
>>>>                                    compatible = "atmel,sama5d4-wdt";
>>>>                                    reg = <0xf8048040 0x10>;
>>>> --
>>>> 2.34.1
>>>>
>>>
>>
>>
>>
>> --
>> Clément Léger,
>> Embedded Linux and Kernel engineer at Bootlin
>> https://bootlin.com
>>
>