| Message ID | 20220608095623.22327-13-tmaimon77@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand |
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 <tmaimon77@gmail.com> > --- > .../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 <tmaimon77@gmail.com> > > +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 <tmaimon77@gmail.com> > + */ > + > +#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/4a69902f-a545-23a1-1430-e5ece16997e9@linaro.org/ 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 <krzysztof.kozlowski@linaro.org> 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 <tmaimon77@gmail.com> > > --- > > .../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 <tmaimon77@gmail.com> > > > > +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 <tmaimon77@gmail.com> > > + */ > > + > > +#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/4a69902f-a545-23a1-1430-e5ece16997e9@linaro.org/ > > 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 <krzysztof.kozlowski@linaro.org> 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 <krzysztof.kozlowski@linaro.org> 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
On 16/06/2022 06:24, Tomer Maimon wrote: > Hi Krzysztof, > > On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> 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. We do not document in the bindings actual pin or interrupt numbers... >> >>> 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. I have no clue what are you referring now... This is NPCM8xx and it has no binding header with reset values. What to remove then? Best regards, Krzysztof
Hi Krzysztof, On Thu, 16 Jun 2022 at 16:39, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 16/06/2022 06:24, Tomer Maimon wrote: > > Hi Krzysztof, > > > > On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> 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. > > > We do not document in the bindings actual pin or interrupt numbers... > > >> > >>> 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. > > I have no clue what are you referring now... This is NPCM8xx and it has > no binding header with reset values. What to remove then? I refer nuvoton,npcm8xx-reset.h file, we don't need it. > > > Best regards, > Krzysztof
On 16/06/2022 06:41, Tomer Maimon wrote: >>>> 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. >> >> I have no clue what are you referring now... This is NPCM8xx and it has >> no binding header with reset values. What to remove then? > I refer nuvoton,npcm8xx-reset.h file, we don't need it. There is no such file in kernel, I believe. If you refer to the patchset here, then of course it should not be sent. Best regards, Krzysztof
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 <tmaimon77@gmail.com> +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 */ +/* + * Copyright (c) 2022 Nuvoton Technology corporation. + * Author: Tomer Maimon <tmaimon77@gmail.com> + */ + +#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) */ +#define NPCM8XX_RESET_GDMA0 3 +#define NPCM8XX_RESET_UDC1 5 +#define NPCM8XX_RESET_GMAC3 6 +#define NPCM8XX_RESET_UART_2_3 7 +#define NPCM8XX_RESET_UDC2 8 +#define NPCM8XX_RESET_PECI 9 +#define NPCM8XX_RESET_AES 10 +#define NPCM8XX_RESET_UART_0_1 11 +#define NPCM8XX_RESET_MC 12 +#define NPCM8XX_RESET_SMB2 13 +#define NPCM8XX_RESET_SMB3 14 +#define NPCM8XX_RESET_SMB4 15 +#define NPCM8XX_RESET_SMB5 16 +#define NPCM8XX_RESET_PWM_M0 18 +#define NPCM8XX_RESET_TIMER_0_4 19 +#define NPCM8XX_RESET_TIMER_5_9 20 +#define NPCM8XX_RESET_GMAC4 21 +#define NPCM8XX_RESET_UDC4 22 +#define NPCM8XX_RESET_UDC5 23 +#define NPCM8XX_RESET_UDC6 24 +#define NPCM8XX_RESET_UDC3 25 +#define NPCM8XX_RESET_ADC 27 +#define NPCM8XX_RESET_SMB6 28 +#define NPCM8XX_RESET_SMB7 29 +#define NPCM8XX_RESET_SMB0 30 +#define NPCM8XX_RESET_SMB1 31 + +/* Reset lines on IP2 reset module (NPCM8XX_RESET_IPSRST2) */ +#define NPCM8XX_RESET_MFT0 0 +#define NPCM8XX_RESET_MFT1 1 +#define NPCM8XX_RESET_MFT2 2 +#define NPCM8XX_RESET_MFT3 3 +#define NPCM8XX_RESET_MFT4 4 +#define NPCM8XX_RESET_MFT5 5 +#define NPCM8XX_RESET_MFT6 6 +#define NPCM8XX_RESET_MFT7 7 +#define NPCM8XX_RESET_MMC 8 +#define NPCM8XX_RESET_GFX_SYS 10 +#define NPCM8XX_RESET_AHB_PCIBRG 11 +#define NPCM8XX_RESET_VDMA 12 +#define NPCM8XX_RESET_ECE 13 +#define NPCM8XX_RESET_VCD 14 +#define NPCM8XX_RESET_VIRUART1 16 +#define NPCM8XX_RESET_VIRUART2 17 +#define NPCM8XX_RESET_SIOX1 18 +#define NPCM8XX_RESET_SIOX2 19 +#define NPCM8XX_RESET_BT 20 +#define NPCM8XX_RESET_3DES 21 +#define NPCM8XX_RESET_PSPI2 23 +#define NPCM8XX_RESET_GMAC2 25 +#define NPCM8XX_RESET_USBH1 26 +#define NPCM8XX_RESET_GMAC1 28 +#define NPCM8XX_RESET_CP1 31 + +/* Reset lines on IP3 reset module (NPCM8XX_RESET_IPSRST3) */ +#define NPCM8XX_RESET_PWM_M1 0 +#define NPCM8XX_RESET_SMB12 1 +#define NPCM8XX_RESET_SPIX 2 +#define NPCM8XX_RESET_SMB13 3 +#define NPCM8XX_RESET_UDC0 4 +#define NPCM8XX_RESET_UDC7 5 +#define NPCM8XX_RESET_UDC8 6 +#define NPCM8XX_RESET_UDC9 7 +#define NPCM8XX_RESET_USBHUB 8 +#define NPCM8XX_RESET_PCI_MAILBOX 9 +#define NPCM8XX_RESET_GDMA1 10 +#define NPCM8XX_RESET_GDMA2 11 +#define NPCM8XX_RESET_SMB14 12 +#define NPCM8XX_RESET_SHA 13 +#define NPCM8XX_RESET_SEC_ECC 14 +#define NPCM8XX_RESET_PCIE_RC 15 +#define NPCM8XX_RESET_TIMER_10_14 16 +#define NPCM8XX_RESET_RNG 17 +#define NPCM8XX_RESET_SMB15 18 +#define NPCM8XX_RESET_SMB8 19 +#define NPCM8XX_RESET_SMB9 20 +#define NPCM8XX_RESET_SMB10 21 +#define NPCM8XX_RESET_SMB11 22 +#define NPCM8XX_RESET_ESPI 23 +#define NPCM8XX_RESET_USB_PHY_1 24 +#define NPCM8XX_RESET_USB_PHY_2 25 + +/* Reset lines on IP4 reset module (NPCM8XX_RESET_IPSRST4) */ +#define NPCM8XX_RESET_SMB16 0 +#define NPCM8XX_RESET_SMB17 1 +#define NPCM8XX_RESET_SMB18 2 +#define NPCM8XX_RESET_SMB19 3 +#define NPCM8XX_RESET_SMB20 4 +#define NPCM8XX_RESET_SMB21 5 +#define NPCM8XX_RESET_SMB22 6 +#define NPCM8XX_RESET_SMB23 7 +#define NPCM8XX_RESET_I3C0 8 +#define NPCM8XX_RESET_I3C1 9 +#define NPCM8XX_RESET_I3C2 10 +#define NPCM8XX_RESET_I3C3 11 +#define NPCM8XX_RESET_I3C4 12 +#define NPCM8XX_RESET_I3C5 13 +#define NPCM8XX_RESET_UART4 16 +#define NPCM8XX_RESET_UART5 17 +#define NPCM8XX_RESET_UART6 18 +#define NPCM8XX_RESET_PCIMBX2 19 +#define NPCM8XX_RESET_SMB24 22 +#define NPCM8XX_RESET_SMB25 23 +#define NPCM8XX_RESET_SMB26 24 +#define NPCM8XX_RESET_USBPHY3 25 +#define NPCM8XX_RESET_PCIRCPHY 27 +#define NPCM8XX_RESET_PWM_M2 28 +#define NPCM8XX_RESET_JTM1 29 +#define NPCM8XX_RESET_JTM2 30 +#define NPCM8XX_RESET_USBH2 31 + +#endif
Add binding document and device tree binding constants for Nuvoton BMC NPCM8XX reset controller. Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> --- .../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