diff mbox series

[v2,06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

Message ID 20220608095623.22327-7-tmaimon77@gmail.com
State New
Headers show
Series Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand

Commit Message

Tomer Maimon June 8, 2022, 9:56 a.m. UTC
Add binding for the Arbel BMC NPCM8XX Clock controller.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../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

Comments

Krzysztof Kozlowski June 8, 2022, 10:03 a.m. UTC | #1
On 08/06/2022 11:56, Tomer Maimon wrote:
> Add binding for the Arbel BMC NPCM8XX Clock controller.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../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@gmail.com>
> +
> +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.

> 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...



Best regards,
Krzysztof
Tomer Maimon June 9, 2022, 1:17 p.m. UTC | #2
Hi Krzysztof,

Thanks for your comments.

On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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@gmail.com>
> > ---
> >  .../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@gmail.com>
> > +
> > +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.
>
> > 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

- 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

>
>
> Best regards,
> Krzysztof

Best regards,

Tomer
Krzysztof Kozlowski June 9, 2022, 1:22 p.m. UTC | #3
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@linaro.org> 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@gmail.com>
>>> ---
>>>  .../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@gmail.com>
>>> +
>>> +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.

>>
>>> 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.

> 
> - 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
J. Neuschäfer June 9, 2022, 5:44 p.m. UTC | #4
Hello Tomer and Krzysztof,

On Wed, Jun 08, 2022 at 12:03:00PM +0200, Krzysztof Kozlowski 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@gmail.com>
> > ---
[...]
> > +  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.

A bit of a side note on these names:

To make the binding as easy to understand as possible, I think it would
help to have every part of the clock-names reflected in corresponding
clock description:

- sysbypck:  presumably means system bypass clock
- mcbypck:   presumably means memory controller bypass clock


As it currently is in the patch, the "byp" part stays unexplained and
unmentioned in the descriptions.



Thanks,
Jonathan
Tomer Maimon June 9, 2022, 9:21 p.m. UTC | #5
Hi Krzysztof,


On Thu, 9 Jun 2022 at 16:22, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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@linaro.org> 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@gmail.com>
> >>> ---
> >>>  .../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@gmail.com>
> >>> +
> >>> +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
Krzysztof Kozlowski June 10, 2022, 9:49 a.m. UTC | #6
On 09/06/2022 23:21, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> 
> On Thu, 9 Jun 2022 at 16:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> 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@linaro.org> 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@gmail.com>
>>>>> ---
>>>>>  .../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@gmail.com>
>>>>> +
>>>>> +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

Because the names should not have irrelevant information.
interrupt-names should have "txirq". dma-names should not have "txdma".
clock-names should not have "refclock" or "refclk" because it is
irrelevant duplication. These are bindings, not DTS, so whatever you
have in your spec matters less. DTS is the representation of hardware
and there you can name clocks closer to the spec so it is easier for
you, if that's your preference.

And if your spec has "refclk_really_clock_this_is_a_clock" you still
should not use it.

Anyway, you should discuss it last time when I pointed it out. Instead
my comments were ignored and you decided to send v2. That's not how
discussion works and it will not bring you closer to your point.


Best regards,
Krzysztof
diff mbox series

Patch

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@gmail.com>
+
+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
+
+  '#clock-cells':
+    const: 1
+    description:
+      See include/dt-bindings/clock/nuvoton,npcm8xx-clock.h for the full
+      list of NPCM8XX clock IDs.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    ahb {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clock-controller@f0801000 {
+            compatible = "nuvoton,npcm845-clk";
+            reg = <0x0 0xf0801000 0x0 0x1000>;
+            clocks = <&clk_refclk>, <&clk_sysbypck>, <&clk_mcbypck>;
+            #clock-cells = <1>;
+        };
+    };
+...
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
@@ -0,0 +1,50 @@ 
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Nuvoton NPCM8xx Clock Generator binding
+ * clock binding number for all clocks supportted by nuvoton,npcm8xx-clk
+ *
+ * Copyright (C) 2021 Nuvoton Technologies.
+ * Author: Tomer Maimon <tomer.maimon@nuvoton.com>
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_NPCM8XX_H
+#define __DT_BINDINGS_CLOCK_NPCM8XX_H
+
+#define NPCM8XX_CLK_CPU		0
+#define NPCM8XX_CLK_GFX_PIXEL	1
+#define NPCM8XX_CLK_MC		2
+#define NPCM8XX_CLK_ADC		3
+#define NPCM8XX_CLK_AHB		4
+#define NPCM8XX_CLK_TIMER	5
+#define NPCM8XX_CLK_UART	6
+#define NPCM8XX_CLK_UART2	7
+#define NPCM8XX_CLK_MMC		8
+#define NPCM8XX_CLK_SPI3	9
+#define NPCM8XX_CLK_PCI		10
+#define NPCM8XX_CLK_AXI		11
+#define NPCM8XX_CLK_APB4	12
+#define NPCM8XX_CLK_APB3	13
+#define NPCM8XX_CLK_APB2	14
+#define NPCM8XX_CLK_APB1	15
+#define NPCM8XX_CLK_APB5	16
+#define NPCM8XX_CLK_CLKOUT	17
+#define NPCM8XX_CLK_GFX		18
+#define NPCM8XX_CLK_SU		19
+#define NPCM8XX_CLK_SU48	20
+#define NPCM8XX_CLK_SDHC	21
+#define NPCM8XX_CLK_SPI0	22
+#define NPCM8XX_CLK_SPI1	23
+#define NPCM8XX_CLK_SPIX	24
+#define NPCM8XX_CLK_RG		25
+#define NPCM8XX_CLK_RCP		26
+#define NPCM8XX_CLK_PRE_ADC	27
+#define NPCM8XX_CLK_ATB		28
+#define NPCM8XX_CLK_PRE_CLK	29
+#define NPCM8XX_CLK_TH		30
+#define NPCM8XX_CLK_REFCLK	31
+#define NPCM8XX_CLK_SYSBYPCK	32
+#define NPCM8XX_CLK_MCBYPCK	33
+
+#define NPCM8XX_NUM_CLOCKS	(NPCM8XX_CLK_MCBYPCK + 1)
+
+#endif