[PATCH v2 1/8] dt-bindings: clk: define additional PMC clocks

[Eugen Hristev]

On 2/28/25 12:58, Sumit Garg wrote:
> On Fri, 28 Feb 2025 at 15:20, <Manikandan.M at microchip.com> wrote:
>>
>> Hi Eugen,
>>
>> On 27/02/25 7:48 pm, Eugen Hristev wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 2/27/25 12:37, Manikandan.M at microchip.com wrote:
>>>> Hi Sumit,
>>>>
>>>> On 27/02/25 3:14 pm, Sumit Garg wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On Thu, 27 Feb 2025 at 15:06, Manikandan Muralidharan
>>>>> <manikandan.m at microchip.com> wrote:
>>>>>>
>>>>>> Add PMC clock definitions for MCK and UTMI which will be required
>>>>>> for the sam9x7 OF_upstream DT since the clock framework is not in
>>>>>> sync with Linux and also include this header in 'clock/at91.h' file
>>>>>
>>>>> You should rather drop these local DT bindings headers which will
>>>>> allow dts/upstream/include/dt-bindings/clock/at91.h to be included
>>>>> automatically.
>>>> Other SoC DTs where OF_UPSTREAM migration is not added yet, depends on
>>>> the local DT bindings header, dropping this will lead to issues with
>>>> compilation.
>>>> We can drop this altogether when we sync the u-boot clock framework with
>>>> Linux.
>>>>>
>>>>> -Sumit
>>>>>
>>>>>>
>>>>>> Signed-off-by: Manikandan Muralidharan <manikandan.m at microchip.com>
>>>>>> ---
>>>>>>    include/dt-bindings/clk/at91.h   | 3 +++
>>>>>>    include/dt-bindings/clock/at91.h | 2 ++
>>>>>>    2 files changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/include/dt-bindings/clk/at91.h b/include/dt-bindings/clk/at91.h
>>>>>> index a178b94157b..016c6e0c620 100644
>>>>>> --- a/include/dt-bindings/clk/at91.h
>>>>>> +++ b/include/dt-bindings/clk/at91.h
>>>>>> @@ -24,4 +24,7 @@
>>>>>>    #define USB_UTMI2              1
>>>>>>    #define USB_UTMI3              2
>>>>>>
>>>>>> +#define PMC_MCK                        1
>>>>>> +#define PMC_UTMI               2
>>>
>>> Where in the patch series do you need these defines ?
>>>
>>>>>> +
>>>>>>    #endif
>>>>>> diff --git a/include/dt-bindings/clock/at91.h b/include/dt-bindings/clock/at91.h
>>>>>> index ab3ee241d10..7235b3ba01e 100644
>>>>>> --- a/include/dt-bindings/clock/at91.h
>>>>>> +++ b/include/dt-bindings/clock/at91.h
>>>>>> @@ -6,6 +6,8 @@
>>>>>>     * Licensed under GPLv2 or later.
>>>>>>     */
>>>>>>
>>>>>> +#include <dt-bindings/clk/at91.h>
>>>>>> +
>>>
>>> I find this odd to include one at91.h in another at91.h
>>>
>>> Let's consider to remove one of these files in the future, and have just
>>> one that is identical with the bindings one from Linux
>>>
>>> Meanwhile, let's see where do you need the PMC_*
>> The PMC_MCK and PMC_UTMI are defined in the dts/upstream sam9x7 SoC DT.
>> since during compilation the clock/at91.h from u-boot is used and to
>> resolve the syntax issues I had to declare them in clk/at91.h and
>> include the header in clock/at91.h

I don't understand this. So , compiling sam9x7 upstream DT with upstream
at91.h should work.
Do you have issues with sam9x7-u-boot.dtsi that fails build ?

>>
>> if we drop the clock/at91.h from u-boot, the sam9x75 will pass using the
>> includes from upstream Linux but will break other SoC that has not
>> migrated to OF_UPSTREAM yet.

So you need to include clock/at91.h from upstream for sam9x75 and the
other at91.h for older SoC.
Does that work ?

>>
>> Or we can align u-boot's clock/at91.h with Linux and drop clk/at91.h,
>> replace it with clock/at91.h in drivers and DT.
>> Please let me know if that works.
> 
> I would rather favor the U-Boot drivers and DT to directly use
> dts/upstream/include/dt-bindings/clock/at91.h instead and drop local
> DT bindings import. It then becomes easier for other SoCs to migrate
> to OF_UPSTREAM too.

Sumit,

The problem is that there are two at91.h with different definitions for
the same macros. (I know... legacy reasons..)
The drivers have to be reworked to cope with the new values. Meanwhile I
would say that at least the new SoCs should use the right macros/bindings

> 
> -Sumit
> 
>>>
>>>>>>    #ifndef _DT_BINDINGS_CLK_AT91_H
>>>>>>    #define _DT_BINDINGS_CLK_AT91_H
>>>>>>
>>>>>> --
>>>>>> 2.25.1
>>>>>>
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Manikandan M.
>>