Re: [PATCH v2 01/16] dt-bindings: clock: at91: Split up per SoC partially

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Claudiu,

Am Mon, Feb 17, 2025 at 11:11:44AM +0200 schrieb Claudiu Beznea:
> Hi, Alexander,
> 
> On 10.02.2025 18:44, Alexander Dahl wrote:
> > Before adding even more new indexes creating more holes in the
> > clk at91 drivers pmc_data->chws arrays, split this up.
> > 
> > This is a partial split up only for SoCs affected by upcoming changes
> > and by that PMC_MAIN + x hack, others could follow by the same scheme.
> > 
> > Binding splitup was proposed for several reasons:
> > 
> > 1) keep the driver code simple, readable, and efficient
> > 2) avoid accidental array index duplication
> > 3) avoid memory waste by creating more and more unused array members.
> > 
> > Old values are kept to not break dts, and to maintain dt ABI.
> > 
> > Link: https://lore.kernel.org/linux-devicetree/20250207-jailbird-circus-bcc04ee90e05@xxxxxxxxxxx/T/#u
> > Signed-off-by: Alexander Dahl <ada@xxxxxxxxxxx>
> > ---
> > 
> > Notes:
> >     v2:
> >     - new patch, not present in v1
> > 
> >  .../dt-bindings/clock/microchip,sam9x60-pmc.h | 19 +++++++++++
> >  .../dt-bindings/clock/microchip,sam9x7-pmc.h  | 25 +++++++++++++++
> >  .../clock/microchip,sama7d65-pmc.h            | 32 +++++++++++++++++++
> >  .../dt-bindings/clock/microchip,sama7g5-pmc.h | 24 ++++++++++++++
> >  4 files changed, 100 insertions(+)
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x60-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sam9x7-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7d65-pmc.h
> >  create mode 100644 include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > 
> 
> [ ...]
> 
> > diff --git a/include/dt-bindings/clock/microchip,sama7g5-pmc.h b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > new file mode 100644
> > index 0000000000000..ad69ccdf9dc78
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/microchip,sama7g5-pmc.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > +/*
> > + * The constants defined in this header are being used in dts and in
> > + * at91 sama7g5 clock driver.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> > +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H
> > +
> > +#include <dt-bindings/clock/at91.h>
> > +
> > +/* old from before bindings splitup */
> > +#define SAMA7G5_PMC_MCK0	PMC_MCK		/* 1 */
> > +#define SAMA7G5_PMC_UTMI	PMC_UTMI	/* 2 */
> > +#define SAMA7G5_PMC_MAIN	PMC_MAIN	/* 3 */
> > +#define SAMA7G5_PMC_CPUPLL	PMC_CPUPLL	/* 4 */
> > +#define SAMA7G5_PMC_SYSPLL	PMC_SYSPLL	/* 5 */
> > +
> > +#define SAMA7G5_PMC_AUDIOPMCPLL	PMC_AUDIOPMCPLL	/* 9 */
> > +#define SAMA7G5_PMC_AUDIOIOPLL	PMC_AUDIOIOPLL	/* 10 */
> > +
> > +#define SAMA7G5_PMC_MCK1	PMC_MCK1	/* 13 */
> > +
> > +#endif
> 
> I would have expected this to be something like:
> 
> #ifndef __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> #define __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__
> 
> /* Core clocks. */
> #define SAMA7G5_MCK0			1
> #define SAMA7G5_UTMI			2
> #define SAMA7G5_MAIN			3
> #define SAMA7G5_CPUPLL			4
> #define SAMA7G5_SYSPLL			5
> #define SAMA7G5_DDRPLL			6
> #define SAMA7G5_IMGPLL			7
> #define SAMA7G5_BAUDPLL			8

Okay no reference to the old header, but numbers.  Got that.

I'm not sure where you got the 7 and 8 from here, according to my
analysis, sama7g5 does not use those.

> 
> // ...
> 
> #define SAMA7G5_MCK1			13
> 
> #endif /* __DT_BINDINGS_CLOCK_MICROCHIP_SAMA7G5_PMC_H__ */
> 
> Same for the other affected SoCs.
> 
> The content of include/dt-bindings/clock/at91.h would be limited eventually
> only to the PMC clock types.

What does this mean?  The clocks split out are no PMC clocks?  Then
the old PMC_MAIN etc. definitions were named wrong?  All or only some
of them?  Or is this different between older and newer SoC variants of
the at91 family?


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux