On 08/06/2022 11:56, Tomer Maimon wrote:
> Add binding document and device tree binding
> constants for Nuvoton BMC NPCM8XX reset controller.
>
> Signed-off-by: Tomer Maimon <[email protected]>
> ---
> .../bindings/reset/nuvoton,npcm-reset.yaml | 13 +-
> .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 ++++++++++++++++++
> 2 files changed, 140 insertions(+), 1 deletion(-)
> create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
>
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> index c6bbc1589ab9..93ea81686f58 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> @@ -9,9 +9,20 @@ title: Nuvoton NPCM Reset controller
> maintainers:
> - Tomer Maimon <[email protected]>
>
> +description: |
> + The NPCM reset controller used to reset various set of peripherals.
> + Please refer to reset.txt in this directory for common reset
> + controller binding usage.
> +
> + For list of all valid reset indices see
> + <dt-bindings/reset/nuvoton,npcm7xx-reset.h> for Poleg NPCM7XX SoC,
> + <dt-bindings/reset/nuvoton,npcm8xx-reset.h> for Arbel NPCM8XX SoC.
> +
> properties:
> compatible:
> - const: nuvoton,npcm750-reset
> + enum:
> + - nuvoton,npcm750-reset # Poleg NPCM7XX SoC
> + - nuvoton,npcm845-reset # Arbel NPCM8XX SoC
>
> reg:
> maxItems: 1
> diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> new file mode 100644
> index 000000000000..5b3b74534b50
> --- /dev/null
> +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
Again - ignored comment from v1.
> +/*
> + * Copyright (c) 2022 Nuvoton Technology corporation.
> + * Author: Tomer Maimon <[email protected]>
> + */
> +
> +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
> +#define _DT_BINDINGS_NPCM8XX_RESET_H
> +
> +/* represent reset register offset */
> +#define NPCM8XX_RESET_IPSRST1 0x20
> +#define NPCM8XX_RESET_IPSRST2 0x24
> +#define NPCM8XX_RESET_IPSRST3 0x34
> +#define NPCM8XX_RESET_IPSRST4 0x74
> +
> +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
Again - ignored comment from v1. My last message was quite clear, wasn't it?
https://lore.kernel.org/all/[email protected]/
You ignored several of previous comments, so:
NAK.
Best regards,
Krzysztof
Hi Krzysztof,
Sorry, but I thought the fix is only to add an explanation to the
dt-binding file as was done in V2.
The NPCM8XX binding is done in the same way as the NPCM7XX and both
use the same reset driver and use the same reset method in upstreamed
NPCM reset driver.
Can you please explain again what you suggest to do?
On Wed, 8 Jun 2022 at 13:11, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Add binding document and device tree binding
> > constants for Nuvoton BMC NPCM8XX reset controller.
> >
> > Signed-off-by: Tomer Maimon <[email protected]>
> > ---
> > .../bindings/reset/nuvoton,npcm-reset.yaml | 13 +-
> > .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 ++++++++++++++++++
> > 2 files changed, 140 insertions(+), 1 deletion(-)
> > create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> >
> > diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > index c6bbc1589ab9..93ea81686f58 100644
> > --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > @@ -9,9 +9,20 @@ title: Nuvoton NPCM Reset controller
> > maintainers:
> > - Tomer Maimon <[email protected]>
> >
> > +description: |
> > + The NPCM reset controller used to reset various set of peripherals.
> > + Please refer to reset.txt in this directory for common reset
> > + controller binding usage.
> > +
> > + For list of all valid reset indices see
> > + <dt-bindings/reset/nuvoton,npcm7xx-reset.h> for Poleg NPCM7XX SoC,
> > + <dt-bindings/reset/nuvoton,npcm8xx-reset.h> for Arbel NPCM8XX SoC.
> > +
> > properties:
> > compatible:
> > - const: nuvoton,npcm750-reset
> > + enum:
> > + - nuvoton,npcm750-reset # Poleg NPCM7XX SoC
> > + - nuvoton,npcm845-reset # Arbel NPCM8XX SoC
> >
> > reg:
> > maxItems: 1
> > diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> > new file mode 100644
> > index 000000000000..5b3b74534b50
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> > @@ -0,0 +1,128 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> Again - ignored comment from v1.
>
> > +/*
> > + * Copyright (c) 2022 Nuvoton Technology corporation.
> > + * Author: Tomer Maimon <[email protected]>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
> > +#define _DT_BINDINGS_NPCM8XX_RESET_H
> > +
> > +/* represent reset register offset */
> > +#define NPCM8XX_RESET_IPSRST1 0x20
> > +#define NPCM8XX_RESET_IPSRST2 0x24
> > +#define NPCM8XX_RESET_IPSRST3 0x34
> > +#define NPCM8XX_RESET_IPSRST4 0x74
> > +
> > +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
>
> Again - ignored comment from v1. My last message was quite clear, wasn't it?
>
> https://lore.kernel.org/all/[email protected]/
>
> You ignored several of previous comments, so:
>
> NAK.
>
> Best regards,
> Krzysztof
Appreciate your time and comments.
Best regards,
Tomer
On 10/06/2022 00:05, Tomer Maimon wrote:
> Hi Krzysztof,
>
> Sorry, but I thought the fix is only to add an explanation to the
> dt-binding file as was done in V2.
>
> The NPCM8XX binding is done in the same way as the NPCM7XX and both
> use the same reset driver and use the same reset method in upstreamed
> NPCM reset driver.
>
> Can you please explain again what you suggest to do?
If you want abstract IDs, they must be abstract, so not representing
hardware registers. Then they start at 1 and are incremented by 1.
Other option is to skip such IDs entirely and use register
offsets/addresses directly, like Arnd suggested in linked documents. I
think he expressed it clearly, so please read his answers which I linked
in previous discussion.
There is no single reason to store register addresses/values/offsets as
binding headers. These are not bindings.
Best regards,
Krzysztof
Hi Krzysztof,
Thanks for your clarification.
We can remove the dt-binding file and use numbers in the DTS,
appreciate if you can answer few additional questions:
1. Do you suggest adding all NPCM reset values to the NPCM reset
document or the reset values should describe in the module
documentation that uses it?
2. Some of the NPCM7XX document modules describe the reset value they
use from the dt-binding for example:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
If we remove the NPCM8XX dt-binding file should we describe the
NPCM8XX values in the NPCM-ADC document file?
Best regards,
Tomer
On Fri, 10 Jun 2022 at 12:55, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 10/06/2022 00:05, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Sorry, but I thought the fix is only to add an explanation to the
> > dt-binding file as was done in V2.
> >
> > The NPCM8XX binding is done in the same way as the NPCM7XX and both
> > use the same reset driver and use the same reset method in upstreamed
> > NPCM reset driver.
> >
> > Can you please explain again what you suggest to do?
>
> If you want abstract IDs, they must be abstract, so not representing
> hardware registers. Then they start at 1 and are incremented by 1.
>
> Other option is to skip such IDs entirely and use register
> offsets/addresses directly, like Arnd suggested in linked documents. I
> think he expressed it clearly, so please read his answers which I linked
> in previous discussion.
>
> There is no single reason to store register addresses/values/offsets as
> binding headers. These are not bindings.
>
> Best regards,
> Krzysztof
On 13/06/2022 02:25, Tomer Maimon wrote:
> Hi Krzysztof,
>
> Thanks for your clarification.
>
> We can remove the dt-binding file and use numbers in the DTS,
> appreciate if you can answer few additional questions:
> 1. Do you suggest adding all NPCM reset values to the NPCM reset
> document or the reset values should describe in the module
> documentation that uses it?
What is "NPCM reset document"? Are these reset values anyhow different
than interrupts or pins?
> 2. Some of the NPCM7XX document modules describe the reset value they
> use from the dt-binding for example:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
This is NPCM750
> If we remove the NPCM8XX dt-binding file should we describe the
> NPCM8XX values in the NPCM-ADC document file?
What is NPCM-ADC document file? What do you want to describe there?
Again - how is it different than interrupts?
Best regards,
Krzysztof
Hi Krzysztof,
On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 13/06/2022 02:25, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Thanks for your clarification.
> >
> > We can remove the dt-binding file and use numbers in the DTS,
> > appreciate if you can answer few additional questions:
> > 1. Do you suggest adding all NPCM reset values to the NPCM reset
> > document or the reset values should describe in the module
> > documentation that uses it?
>
> What is "NPCM reset document"? Are these reset values anyhow different
> than interrupts or pins?
No, they represent the same values.
>
> > 2. Some of the NPCM7XX document modules describe the reset value they
> > use from the dt-binding for example:
> > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
>
> This is NPCM750
>
> > If we remove the NPCM8XX dt-binding file should we describe the
> > NPCM8XX values in the NPCM-ADC document file?
>
> What is NPCM-ADC document file? What do you want to describe there?
> Again - how is it different than interrupts?
It is not different from the interrupts.
I will remove the dt-binding reset include file, the reset property
will use numbers and not macro's.
>
>
>
> Best regards,
> Krzysztof
Best regards,
Tomer