diff mbox series

[v2,12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

Message ID 20220608095623.22327-13-tmaimon77@gmail.com
State New
Headers show
Series [v2,01/20] clocksource: timer-npcm7xx: Add NPCM845 timer | expand

Commit Message

Tomer Maimon June 8, 2022, 9:56 a.m. UTC
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

Comments

Tomer Maimon June 9, 2022, 10:05 p.m. UTC | #1
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
Krzysztof Kozlowski June 10, 2022, 9:55 a.m. UTC | #2
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
Krzysztof Kozlowski June 16, 2022, 1:38 p.m. UTC | #3
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
Krzysztof Kozlowski June 16, 2022, 1:42 p.m. UTC | #4
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 mbox series

Patch

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