On Fri, 28 Feb 2025 at 16:45, Eugen Hristev <eugen.hris...@linaro.org> wrote: > > > > On 2/28/25 12:58, Sumit Garg wrote: > > On Fri, 28 Feb 2025 at 15:20, <manikanda...@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, manikanda...@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 > >>>>> <manikanda...@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 <manikanda...@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 ?
The way it works currently is you can have a single clock/at91.h available where preference is currently given to local U-Boot copy (for backwards compatibility) over what is available in upstream DT. So can't have different versions of clock/at91.h available. > > >> > >> 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 Since the drivers are also common for both new and older SoCs, it's rather better to adapt them to common bindings rather than supporting 2 different versions of clock/at91.h. If you want to do that as part of this series or later will be based on your preference. -Sumit > > > > > -Sumit > > > >>> > >>>>>> #ifndef _DT_BINDINGS_CLK_AT91_H > >>>>>> #define _DT_BINDINGS_CLK_AT91_H > >>>>>> > >>>>>> -- > >>>>>> 2.25.1 > >>>>>> > >>>> > >>> > >> > >> -- > >> Thanks and Regards, > >> Manikandan M. > >> >
