Re: [PATCH v2 1/3] clk: nuvoton: ma35d1: Add missing WDT/WWDT parent clocks
From: Krzysztof Kozlowski
Date: Fri Sep 25 2026 - 09:02:45 EST
On Fri, Sep 25, 2026 at 12:18:43PM +0200, Miquel Raynal wrote:
> Hello Krzysztof,
>
> >> Reported-by: Jacky Huang <ychuang570808@xxxxxxxxx>
> >> Closes: https://lore.kernel.org/linux-clk/b8a8a65b-e302-4747-bc90-4920fc48795a@xxxxxxxxx/
> >> Suggested-by: Jacky Huang <ychuang570808@xxxxxxxxx>
> >> Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> >> ---
> >> drivers/clk/nuvoton/clk-ma35d1.c | 2 ++
> >> include/dt-bindings/clock/nuvoton,ma35d1-clk.h | 4 +++-
> >
> > Please do not combine patches.
>
> I thought splitting these changes would not make sense in this (= a fix)
> case, but fine.
>
> [...]
>
> >> @@ -248,6 +248,8 @@
> >> #define ADC_GATE 233
> >> #define EADC_DIV 234
> >> #define EADC_GATE 235
> >> -#define CLK_MAX_IDX 236
> >> +#define PCLK3_DIV4096 236
> >> +#define PCLK4_DIV4096 237
> >> +#define CLK_MAX_IDX 238
> >
> > This cannot be updated. If it has to, then not a binding and should be
> > first dropped from the bindings like we did for most of the platforms
> > having similar issue.
>
> When you say "This cannot be updated", do we agree that you mean
> "CLK_MAX_IDX cannot be updated"?
Yes, the MAX ID.
>
> I'm not sure I fully understand the reasoning behind, because the whole
> point of defining this CLK_MAX_IDX value is to let drivers know the
> number of clocks, so it must obviously follow the updates in this
> file. I cannot just drop it, the driver does not work without it. If we
It's not a binding if it has to change, because bindings should not
change - otherwise they break its users, because since it is ABI there
is an outside user expecting EXACTLY this number of clocks and it cannot
grow. Obviously, this is not a binding just like FOR EVERY other case so
can be safely dropped.
> enlarge that number, what is the problem as long as we keep the other
> numbers identical?
>
> Maybe one way to fix that mess is to:
> 1- Add the 2 new values (238 and 239) + a new define:
> NUVOTON_MA35D1_MAX_IDX 240
> 238 would then have two definitions for a short period of time.
> 2- Replace CLK_MAX_IDX by NUVOTON_MA35D1_MAX_IDX in the driver
> 3- Drop CLK_MAX_IDX in the bindings.
Why we can't fix it like we did everywhere else?
git lg -- include/dt-bindings/clock/
and search for "number" which leads to recent commits like
d10f26a7abbd3dd5d59bac1acdca117385b54ea9
but there was same work for Samsung and many others (Renesas, maybe
NXP?). You add define to the driver, then drop it from the binding. Can
be a few commits like we did for many cases, but recent change from
Linus I think went acked while combined binding and driver.
Best regards,
Krzysztof