Hi Krzysztof, On Thu, 9 Jun 2022 at 16:22, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 09/06/2022 15:17, Tomer Maimon wrote: > > Hi Krzysztof, > > > > Thanks for your comments. > > > > On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > >> > >> On 08/06/2022 11:56, Tomer Maimon wrote: > >>> Add binding for the Arbel BMC NPCM8XX Clock controller. > >>> > >>> Signed-off-by: Tomer Maimon <tmaimon77@xxxxxxxxx> > >>> --- > >>> .../bindings/clock/nuvoton,npcm845-clk.yaml | 63 +++++++++++++++++++ > >>> .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++ > >>> 2 files changed, 113 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml > >>> create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h > >>> > >>> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml > >>> new file mode 100644 > >>> index 000000000000..e1f375716bc5 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml > >>> @@ -0,0 +1,63 @@ > >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Nuvoton NPCM8XX Clock Controller Binding > >>> + > >>> +maintainers: > >>> + - Tomer Maimon <tmaimon77@xxxxxxxxx> > >>> + > >>> +description: | > >>> + Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which > >>> + generates and supplies clocks to all modules within the BMC. > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - nuvoton,npcm845-clk > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + clocks: > >>> + items: > >>> + - description: 25M reference clock > >>> + - description: CPU reference clock > >>> + - description: MC reference clock > >>> + > >>> + clock-names: > >>> + items: > >>> + - const: refclk > >>> + - const: sysbypck > >>> + - const: mcbypck > >>> + > >> > >> I asked what is the suffix about and you replied "ck"... ok, so let's > >> make clear. This should be: > >> > >> items: > >> - const: ref > >> - const: sysbyp > >> - const: mcbyp > >> > >> or something similar, without the same suffix all over. > > The clock names are the same clock name in our spec, this why we > > prefer to leave the clock names as is. > > The naming with useless suffixes does not help. If your spec had > "refclk_really_clock_this_is_a_clock" you also would insist on that? It > does not make sense. Sorry but I don't understand why the clock name cause an issue, we prefer it will be the same as in our spec-clock diagram BTW, the same naming found in NPCM7XX https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt#L36 > > >> > >>> diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h > >>> new file mode 100644 > >>> index 000000000000..229915a254a5 > >>> --- /dev/null > >>> +++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h > >> > >> Same comment as before. No changes here... > >> > > about the comments from V1:: > > - Krzysztof: Filename - same as bindings, so nuvoton,npcm845-clk.h > > In NPCM7XX we use the same include file and clock source > > dt-binding > > https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt > > dt-binding include > > https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/nuvoton,npcm7xx-clock.h > > we prefer to be align with our older BMC version > > Older has incorrect name, so do not align to incorrect one. What is the > logic behind having header not matching the bindings file? It makes it > only more difficult to connect these two. Will modify the file name in V3 to be the same as dt-binding > > > > > - Krzysztof: Dual license, same as bindings. > > modified in the file * SPDX-License-Identifier: (GPL-2.0-only OR > > BSD-2-Clause) */ > > the same license approved in en7523-clk include file and pushed to > > Linux kernel 5.19 : > > https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/en7523-clk.h > > I don't understand this comment at all. I am not commenting about > en7523-clk.h. I am commenting about the header here - it should have > dual license. What en7523-clk.h has to do with it? > > Best regards, > Krzysztof Best regards, Tomer
