On 2020/12/17 19:17, Stephen Boyd wrote: > Quoting Damien Le Moal (2020-12-17 00:13:57) >> On 2020/12/17 17:10, Stephen Boyd wrote: >>> Quoting Damien Le Moal (2020-12-13 05:50:42) >>>> diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h >>>> index 5a2fd64d1a49..b2de702cbf75 100644 >>>> --- a/include/dt-bindings/clock/k210-clk.h >>>> +++ b/include/dt-bindings/clock/k210-clk.h >>>> @@ -3,18 +3,51 @@ >>>> * Copyright (C) 2019-20 Sean Anderson <seanga2@xxxxxxxxx> >>>> * Copyright (c) 2020 Western Digital Corporation or its affiliates. >>>> */ >>>> -#ifndef K210_CLK_H >>>> -#define K210_CLK_H >>>> +#ifndef CLOCK_K210_CLK_H >>>> +#define CLOCK_K210_CLK_H >>>> >>>> /* >>>> - * Arbitrary identifiers for clocks. >>>> - * The structure is: in0 -> pll0 -> aclk -> cpu >>>> - * >>>> - * Since we use the hardware defaults for now, set all these to the same clock. >>>> + * Kendryte K210 SoC clock identifiers (arbitrary values). >>>> */ >>>> -#define K210_CLK_PLL0 0 >>>> -#define K210_CLK_PLL1 0 >>>> -#define K210_CLK_ACLK 0 >>>> -#define K210_CLK_CPU 0 >>> >>> This seems to open a bisection hole. I see that ACLK is used in the >>> existing dtsi file, and that is the same as CLK_CPU, but after this >>> patch it will change to not exist anymore. Can we leave ACLK around >>> defined to be 0? I imagine it won't be used in the future so we can >>> remove it later. I can then apply this for v5.11-rc1 and then merge the >>> clk driver patch in clk tree. >>> >>>> +#define K210_CLK_CPU 0 >>>> +#define K210_CLK_SRAM0 1 >>>> +#define K210_CLK_SRAM1 2 >>> >> >> Patch 6 of the series removes the use of K210_CLK_CPU and K210_CLK_ACLK from the >> device trees. I added that patch as the DT modification proper comes only at >> patch 16. Maybe I should squash patch 6 into this one ? >> > > Preferably the defines are just left alone forever and then forgotten. > The dt-bindings directory is almost ABI and so changing numbers or > removing defines is hard to do. Usually patches in this directory are an > additive thing. I just did that. It works. Ideally, patches 7, 8 and 9 need to go in together with the clk driver patch. Since the builtin DTB patch precedes the clock driver patch that touches the sysctl driver, I need to rework it a little, keeping the SOC_DECLARE_BUILTIN_DTB() for now. And finally, a small DTS update patch needs to be added too for the sysctl & sysclk nodes update. That would make it a 5 patch series for the clock driver addition. Would this work ? Or, you just take patch 9 (clk doc) and patch 13 (clk driver), slightly modified to move the sysctl register definitions into a common header (currently part of patch 7). 2 patches only, without any other change, resulting in the clock driver not being used until the rest of the series goes into 5.12. Do you prefer that solution ? -- Damien Le Moal Western Digital Research
