| Message ID | 20220608095623.22327-1-tmaimon77@gmail.com |
|---|---|
| Headers | show |
| Series | Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand |
On 08/06/2022 11:56, Tomer Maimon wrote: > Add nuvoton,sysgcr syscon property to the reset > node to handle the general control registers. Wrong wrapping. Best regards, Krzysztof
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, Thanks for your comments On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/06/2022 11:56, Tomer Maimon wrote: > > Add nuvoton,sysgcr syscon property to the reset > > node to handle the general control registers. > > Wrong wrapping. it will be very helpful if you could point me what wrong wrapped in the commit message, is it the explanation or the header? or something else? > > Best regards, > Krzysztof Best regards, Tomer
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
Hi Tomer, On Thu, 9 Jun 2022 at 14:30, Tomer Maimon <tmaimon77@gmail.com> wrote: > > Hi Krzysztof, > > Thanks for your comments > > On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 08/06/2022 11:56, Tomer Maimon wrote: > > > Add nuvoton,sysgcr syscon property to the reset > > > node to handle the general control registers. > > > > Wrong wrapping. > it will be very helpful if you could point me what wrong wrapped in > the commit message, is it the explanation or the header? or something > else? The commit message body should be wrapped at 72 chars. You can fit more on the first line if you reflow: Add nuvoton,sysgcr syscon property to the reset node to handle the general control registers. > > > > Best regards, > > Krzysztof > > Best regards, > > Tomer
Quoting Tomer Maimon (2022-06-08 02:56:10) > diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c > new file mode 100644 > index 000000000000..40340c3611b5 > --- /dev/null > +++ b/drivers/clk/clk-npcm8xx.c > @@ -0,0 +1,756 @@ [...] > + > +#define PLLCON_LOKI BIT(31) > +#define PLLCON_LOKS BIT(30) > +#define PLLCON_FBDV GENMASK(27, 16) > +#define PLLCON_OTDV2 GENMASK(15, 13) > +#define PLLCON_PWDEN BIT(12) > +#define PLLCON_OTDV1 GENMASK(10, 8) > +#define PLLCON_INDV GENMASK(5, 0) > + > +static unsigned long npcm8xx_clk_pll_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct npcm8xx_clk_pll *pll = to_npcm8xx_clk_pll(hw); > + unsigned long fbdv, indv, otdv1, otdv2; > + unsigned int val; > + u64 ret; > + > + if (parent_rate == 0) { > + pr_debug("%s: parent rate is zero", __func__); Missing newline. > + return 0; > + } > + > + val = readl_relaxed(pll->pllcon); > + > + indv = FIELD_GET(PLLCON_INDV, val);
Hi Krzysztof, Sorry, probably I missed your comments (too many patches to handle at one time :-))... On Wed, 8 Jun 2022 at 13:21, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/06/2022 11:56, Tomer Maimon wrote: > > This adds initial device tree support for the > > Nuvoton NPCM845 Board Management controller (BMC) SoC family. > > > > The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and > > have various peripheral IPs. > > > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> > > --- > > arch/arm64/boot/dts/Makefile | 1 + > > .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 197 ++++++++++++++++++ > > .../boot/dts/nuvoton/nuvoton-npcm845.dtsi | 76 +++++++ > > 3 files changed, 274 insertions(+) > > create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi > > create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi > > > > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile > > index 1ba04e31a438..7b107fa7414b 100644 > > --- a/arch/arm64/boot/dts/Makefile > > +++ b/arch/arm64/boot/dts/Makefile > > @@ -19,6 +19,7 @@ subdir-y += lg > > subdir-y += marvell > > subdir-y += mediatek > > subdir-y += microchip > > +subdir-y += nuvoton > > subdir-y += nvidia > > subdir-y += qcom > > subdir-y += realtek > > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi > > new file mode 100644 > > index 000000000000..97e108c50760 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi > > @@ -0,0 +1,197 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com > > + > > +#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h> > > +#include <dt-bindings/interrupt-controller/arm-gic.h> > > +#include <dt-bindings/interrupt-controller/irq.h> > > + > > +/ { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + interrupt-parent = <&gic>; > > + > > + /* external reference clock */ > > + clk_refclk: clk-refclk { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <25000000>; > > Ignored comment. Could we use it as a default clock-frequency? > > > + clock-output-names = "refclk"; > > + }; > > + > > + /* external reference clock for cpu. float in normal operation */ > > + clk_sysbypck: clk-sysbypck { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <1000000000>; > > Ignored comment. same as above > > > + clock-output-names = "sysbypck"; > > + }; > > + > > + /* external reference clock for MC. float in normal operation */ > > + clk_mcbypck: clk-mcbypck { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <1050000000>; same as above > > + clock-output-names = "mcbypck"; > > + }; > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + compatible = "simple-bus"; > > + interrupt-parent = <&gic>; > > + ranges; > > + > > + gcr: gcr@f0800000 { I understand it sounds generic but I try to be as much compatible with NPCM7XX https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L91 > > Ignored comment. > > > + compatible = "nuvoton,npcm845-gcr", "syscon", > > + "simple-mfd"; > > This is not a simple-mfd... I see original bindings defined it that way, > but why? I think they should be corrected - remove simple-mfd from the > bindings and DTS. will remove in both places in V3 > > > > + reg = <0x0 0xf0800000 0x0 0x1000>; > > + }; > > + > > + gic: interrupt-controller@dfff9000 { > > + compatible = "arm,gic-400"; > > + reg = <0x0 0xdfff9000 0x0 0x1000>, > > + <0x0 0xdfffa000 0x0 0x2000>, > > + <0x0 0xdfffc000 0x0 0x2000>, > > + <0x0 0xdfffe000 0x0 0x2000>; > > + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > > + #interrupt-cells = <3>; > > + interrupt-controller; > > + #address-cells = <0>; > > + ppi-partitions { > > + ppi_cluster0: interrupt-partition-0 { > > + affinity = <&cpu0 &cpu1 &cpu2 &cpu3>; > > + }; > > + }; > > + }; > > + }; > > + > > + ahb { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + compatible = "simple-bus"; > > + interrupt-parent = <&gic>; > > + ranges; > > + > > + rstc: rstc@f0801000 { > > Ignored comment. > I understand it sounds generic but I try to be as much compatible with NPCM7XX https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L109 > Four comments from v1 ignored in this patch alone. > one more comment in V1 "+ cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a35"; + clocks = <&clk NPCM8XX_CLK_CPU>; + reg = <0x0 0x0>; Why do you have two address cells? A bit more complicated and not necessary, I think." the arm,cortex-a35 is 64 Bit this is why we use #address-cells = <2>; and therefore reg = <0x0 0x0>; > I'll stop reviewing, it is a waste of my time. > > NAK for this change. > > Best regards, > Krzysztof Again sorry to miss these comments in V1. Appreciate your time. Best regards, Tomer
Hi Stephen, Thanks for your comment, it will be addressed next patch set On Fri, 10 Jun 2022 at 01:14, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Tomer Maimon (2022-06-08 02:56:10) > > diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c > > new file mode 100644 > > index 000000000000..40340c3611b5 > > --- /dev/null > > +++ b/drivers/clk/clk-npcm8xx.c > > @@ -0,0 +1,756 @@ > [...] > > + > > +#define PLLCON_LOKI BIT(31) > > +#define PLLCON_LOKS BIT(30) > > +#define PLLCON_FBDV GENMASK(27, 16) > > +#define PLLCON_OTDV2 GENMASK(15, 13) > > +#define PLLCON_PWDEN BIT(12) > > +#define PLLCON_OTDV1 GENMASK(10, 8) > > +#define PLLCON_INDV GENMASK(5, 0) > > + > > +static unsigned long npcm8xx_clk_pll_recalc_rate(struct clk_hw *hw, > > + unsigned long parent_rate) > > +{ > > + struct npcm8xx_clk_pll *pll = to_npcm8xx_clk_pll(hw); > > + unsigned long fbdv, indv, otdv1, otdv2; > > + unsigned int val; > > + u64 ret; > > + > > + if (parent_rate == 0) { > > + pr_debug("%s: parent rate is zero", __func__); > > Missing newline. > > > + return 0; > > + } > > + > > + val = readl_relaxed(pll->pllcon); > > + > > + indv = FIELD_GET(PLLCON_INDV, val); Best regards, Tomer
Hi Benjamin, Thanks a lot for your explanation. will be applied in next patch set Best regards, Tomer On Fri, 10 Jun 2022 at 01:11, Benjamin Fair <benjaminfair@google.com> wrote: > > Hi Tomer, > > On Thu, 9 Jun 2022 at 14:30, Tomer Maimon <tmaimon77@gmail.com> wrote: > > > > Hi Krzysztof, > > > > Thanks for your comments > > > > On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > > > > > > On 08/06/2022 11:56, Tomer Maimon wrote: > > > > Add nuvoton,sysgcr syscon property to the reset > > > > node to handle the general control registers. > > > > > > Wrong wrapping. > > it will be very helpful if you could point me what wrong wrapped in > > the commit message, is it the explanation or the header? or something > > else? > > The commit message body should be wrapped at 72 chars. You can fit > more on the first line if you reflow: > > Add nuvoton,sysgcr syscon property to the reset node to handle the > general control registers. > > > > > > > Best regards, > > > Krzysztof > > > > Best regards, > > > > Tomer
On 10/06/2022 00:29, Tomer Maimon wrote: >>> + clk_refclk: clk-refclk { >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <25000000>; >> >> Ignored comment. > Could we use it as a default clock-frequency? In DTS? If my assumption, that this clock is not on SoC itself, is correct, then the answer is no, you cannot. The clock physically sits on the board, so it is defined by board DTS. Feel free to embed in SoC DTSI most of the clock properties, but the core property - frequency - must be outside. >> >>> + clock-output-names = "refclk"; >>> + }; >>> + >>> + /* external reference clock for cpu. float in normal operation */ >>> + clk_sysbypck: clk-sysbypck { >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <1000000000>; >> >> Ignored comment. > same as above >> >>> + clock-output-names = "sysbypck"; >>> + }; >>> + >>> + /* external reference clock for MC. float in normal operation */ >>> + clk_mcbypck: clk-mcbypck { >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <1050000000>; > same as above >>> + clock-output-names = "mcbypck"; >>> + }; >>> + >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + compatible = "simple-bus"; >>> + interrupt-parent = <&gic>; >>> + ranges; >>> + >>> + gcr: gcr@f0800000 { > I understand it sounds generic but I try to be as much compatible with NPCM7XX > https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L91 Then fix NPCM7XX. Please do not multiple bad choices because it looks similar. Fix the other wrong one. >> >> Ignored comment. >> >>> + compatible = "nuvoton,npcm845-gcr", "syscon", >>> + "simple-mfd"; >> >> This is not a simple-mfd... I see original bindings defined it that way, >> but why? I think they should be corrected - remove simple-mfd from the >> bindings and DTS. > will remove in both places in V3 >> >> >>> + reg = <0x0 0xf0800000 0x0 0x1000>; >>> + }; >>> + >>> + gic: interrupt-controller@dfff9000 { >>> + compatible = "arm,gic-400"; >>> + reg = <0x0 0xdfff9000 0x0 0x1000>, >>> + <0x0 0xdfffa000 0x0 0x2000>, >>> + <0x0 0xdfffc000 0x0 0x2000>, >>> + <0x0 0xdfffe000 0x0 0x2000>; >>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; >>> + #interrupt-cells = <3>; >>> + interrupt-controller; >>> + #address-cells = <0>; >>> + ppi-partitions { >>> + ppi_cluster0: interrupt-partition-0 { >>> + affinity = <&cpu0 &cpu1 &cpu2 &cpu3>; >>> + }; >>> + }; >>> + }; >>> + }; >>> + >>> + ahb { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + compatible = "simple-bus"; >>> + interrupt-parent = <&gic>; > >>> + ranges; >>> + >>> + rstc: rstc@f0801000 { >> >> Ignored comment. >> > I understand it sounds generic but I try to be as much compatible with NPCM7XX > https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L109 Fix 7xx. 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
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
This patchset adds initial support for the Nuvoton Arbel NPCM8XX Board Management controller (BMC) SoC family. The Nuvoton Arbel NPCM8XX SoC is a fourth-generation BMC. The NPCM8XX computing subsystem comprises a quadcore ARM Cortex A35 ARM-V8 architecture. This patchset adds minimal architecture and drivers such as: Clocksource, Clock, Reset, and WD. Some of the Arbel NPCM8XX peripherals are based on Poleg NPCM7XX. This patchset was tested on the Arbel NPCM8XX evaluation board. Addressed comments from: - Arnd Bergmann : https://www.spinics.net/lists/arm-kernel/msg982728.html https://www.spinics.net/lists/linux-serial/msg48179.html - Stephen Boyd: https://www.spinics.net/lists/arm-kernel/msg983594.html - Krzysztof Kozlowski: https://www.spinics.net/lists/kernel/msg4368955.html https://www.spinics.net/lists/arm-kernel/msg982427.html https://lore.kernel.org/all/973d75b8-0eb6-ff5b-6cd2-9b7d7c5cbcaa@linaro.org/ https://www.spinics.net/lists/arm-kernel/msg982778.html https://www.spinics.net/lists/arm-kernel/msg982588.html - Ilpo Järvinen: https://www.spinics.net/lists/kernel/msg4368936.html - Stephen Boyd: https://www.spinics.net/lists/arm-kernel/msg983596.html - Geert Uytterhoeven : https://www.spinics.net/lists/kernel/msg4369514.html Changes since version 1: - NPCM8XX clock driver - Modify dt-binding. - Remove unsed definition and include. - Include alphabetically. - Use clock devm. - NPCM reset driver - Modify dt-binding. - Modify syscon name. - Add syscon support to NPCM7XX dts reset node. - use data structure. - NPCM8XX device tree: - Modify evb compatible name. - Add NPCM7xx compatible. - Remove disable nodes from the EVB DTS. Tomer Maimon (20): clocksource: timer-npcm7xx: Add NPCM845 timer dt-bindings: serial: 8250: Add npcm845 compatible string tty: serial: 8250: Add NPCM845 UART support dt-bindings: watchdog: npcm: Add npcm845 compatible string watchdog: npcm_wdt: Add NPCM845 watchdog support dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock clk: npcm8xx: add clock controller dt-bindings: reset: modify to general NPCM name dt-bindings: reset: npcm: add GCR syscon property ARM: dts: nuvoton: add reset syscon property reset: npcm: using syscon instead of device data dt-bindings: reset: npcm: Add support for NPCM8XX reset: npcm: Add NPCM8XX support dt-bindings: arm: npcm: Add maintainer dt-bindings: arm: npcm: Add nuvoton,npcm845 compatible string dt-bindings: arm: npcm: Add nuvoton,npcm845 GCR compatible string arm64: npcm: Add support for Nuvoton NPCM8XX BMC SoC arm64: dts: nuvoton: Add initial NPCM8XX device tree arm64: dts: nuvoton: Add initial NPCM845 EVB device tree arm64: defconfig: Add Nuvoton NPCM family support .../devicetree/bindings/arm/npcm/npcm.yaml | 7 + .../bindings/arm/npcm/nuvoton,gcr.yaml | 2 + .../bindings/clock/nuvoton,npcm845-clk.yaml | 63 ++ ...750-reset.yaml => nuvoton,npcm-reset.yaml} | 21 +- .../devicetree/bindings/serial/8250.yaml | 1 + .../bindings/watchdog/nuvoton,npcm-wdt.txt | 3 +- MAINTAINERS | 3 + arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 1 + arch/arm64/Kconfig.platforms | 11 + arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/nuvoton/Makefile | 2 + .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 197 +++++ .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts | 30 + .../boot/dts/nuvoton/nuvoton-npcm845.dtsi | 76 ++ arch/arm64/configs/defconfig | 3 + drivers/clk/Kconfig | 6 + drivers/clk/Makefile | 1 + drivers/clk/clk-npcm8xx.c | 756 ++++++++++++++++++ drivers/clocksource/timer-npcm7xx.c | 1 + drivers/reset/reset-npcm.c | 202 ++++- drivers/tty/serial/8250/8250_of.c | 1 + drivers/watchdog/npcm_wdt.c | 1 + .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 ++ .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 +++ 24 files changed, 1530 insertions(+), 37 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml rename Documentation/devicetree/bindings/reset/{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml} (58%) create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi create mode 100644 drivers/clk/clk-npcm8xx.c create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h