On 11/02/2025 08:16, Alexander Dahl wrote: > Hello everyone, > > Am Mon, Feb 10, 2025 at 06:05:31PM +0100 schrieb Krzysztof Kozlowski: >> On 10/02/2025 17: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,sam9x60-pmc.h b/include/dt-bindings/clock/microchip,sam9x60-pmc.h >>> new file mode 100644 >>> index 0000000000000..e01e867e8c4da >>> --- /dev/null >>> +++ b/include/dt-bindings/clock/microchip,sam9x60-pmc.h >>> @@ -0,0 +1,19 @@ >>> +/* 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 sam9x60 clock driver. >>> + */ >>> + >>> +#ifndef _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H >>> +#define _DT_BINDINGS_CLOCK_MICROCHIP_SAM9X60_PMC_H >>> + >>> +#include <dt-bindings/clock/at91.h> >>> + >>> +/* old from before bindings splitup */ >>> +#define SAM9X60_PMC_MCK PMC_MCK /* 1 */ >>> +#define SAM9X60_PMC_UTMI PMC_UTMI /* 2 */ >>> +#define SAM9X60_PMC_MAIN PMC_MAIN /* 3 */ >>> + >>> +#define SAM9X60_PMC_PLLACK PMC_PLLACK /* 7 */ >> >> IIUC, you want to have bindings per SoC, so why not adding proper >> constants here instead of including entire old binding header? The >> binding header should be entirely abandoned later. > > Which binding header should be abandoned entirely? The one you claim to split. I assume it is the same one included here. > > The bindings per SoC idea was proposed in series v1 feedback. I'm > neither a binding nor a clock expeert. As far as I understood it's > important to keep the exact same values as before to not change any Yes, I did not propose to change any IDs. > ABI. The non SoC specific values are still used on older SoCs of the > at91 family, so this is why I used the old constants for now. > > These PMC indexes are not the only definitions in > dt-bindings/clock/at91.h however, there are more which are not SoC > specific. > > I'd like some thoughts from the Microchip maintainers here, > what's their idea on how to proceed with the at91 clock stuff? > > This works for me, but the current state is more or less still an idea > as base for discussion. Please don't make it overly complicated, this > is not the primary focus of my work. You made this binding more complicated than it should be - instead of simple list of clocks you include some other file and split between old and new. Best regards, Krzysztof
