| Message ID | 20220701192300.2293643-6-conor@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Canaan devicetree fixes | expand |
On Fri, Jul 01, 2022 at 08:22:51PM +0100, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > The k210 U-Boot port has been using the clocks defined in the > devicetree to bring up the board's SRAM, but this violates the > dt-schema. As such, move the clocks to a dedicated node with > the same compatible string & document it. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > .../memory-controllers/canaan,k210-sram.yaml | 52 +++++++++++++++++++ > 1 file changed, 52 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml > > diff --git a/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml > new file mode 100644 > index 000000000000..82be32757713 > --- /dev/null > +++ b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml > @@ -0,0 +1,52 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/memory-controllers/canaan,k210-sram.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Canaan K210 SRAM memory controller > + > +description: | Don't need '|'. > + The Canaan K210 SRAM memory controller is initialised and programmed by > + firmware, but an OS might want to read its registers for error reporting > + purposes and to learn about the DRAM topology. How the OS going to do that? You don't have any way defined to access the registers. Also, where is the SRAM address itself defined? > + > +maintainers: > + - Conor Dooley <conor@kernel.org> > + > +properties: > + compatible: > + enum: > + - canaan,k210-sram > + > + clocks: > + minItems: 1 > + items: > + - description: sram0 clock > + - description: sram1 clock > + - description: aisram clock > + > + clock-names: > + minItems: 1 > + items: > + - const: sram0 > + - const: sram1 > + - const: aisram > + > +required: > + - compatible > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/k210-clk.h> > + memory-controller { > + compatible = "canaan,k210-sram"; > + clocks = <&sysclk K210_CLK_SRAM0>, > + <&sysclk K210_CLK_SRAM1>, > + <&sysclk K210_CLK_AI>; > + clock-names = "sram0", "sram1", "aisram"; > + }; > -- > 2.37.0 > >
On 05/07/2022 20:23, Rob Herring wrote: > On Fri, Jul 01, 2022 at 08:22:51PM +0100, Conor Dooley wrote: >> From: Conor Dooley <conor.dooley@microchip.com> >> >> The k210 U-Boot port has been using the clocks defined in the >> devicetree to bring up the board's SRAM, but this violates the >> dt-schema. As such, move the clocks to a dedicated node with >> the same compatible string & document it. >> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> .../memory-controllers/canaan,k210-sram.yaml | 52 +++++++++++++++++++ >> 1 file changed, 52 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >> new file mode 100644 >> index 000000000000..82be32757713 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >> @@ -0,0 +1,52 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/memory-controllers/canaan,k210-sram.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Canaan K210 SRAM memory controller >> + >> +description: | > > Don't need '|'. > >> + The Canaan K210 SRAM memory controller is initialised and programmed by >> + firmware, but an OS might want to read its registers for error reporting >> + purposes and to learn about the DRAM topology. > > How the OS going to do that? You don't have any way defined to access > the registers. Eugh, copy paste. I'll rephrase in the respin. It should be "initialised by firmware." There are no registers, only clocks. > > Also, where is the SRAM address itself defined? The actual sram is in the memory node. > >> + >> +maintainers: >> + - Conor Dooley <conor@kernel.org> >> + >> +properties: >> + compatible: >> + enum: >> + - canaan,k210-sram >> + >> + clocks: >> + minItems: 1 >> + items: >> + - description: sram0 clock >> + - description: sram1 clock >> + - description: aisram clock >> + >> + clock-names: >> + minItems: 1 >> + items: >> + - const: sram0 >> + - const: sram1 >> + - const: aisram >> + >> +required: >> + - compatible >> + - clocks >> + - clock-names >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/k210-clk.h> >> + memory-controller { >> + compatible = "canaan,k210-sram"; >> + clocks = <&sysclk K210_CLK_SRAM0>, >> + <&sysclk K210_CLK_SRAM1>, >> + <&sysclk K210_CLK_AI>; >> + clock-names = "sram0", "sram1", "aisram"; >> + }; >> -- >> 2.37.0 >> >>
On 05/07/2022 20:28, Conor.Dooley@microchip.com wrote: > > > On 05/07/2022 20:23, Rob Herring wrote: >> On Fri, Jul 01, 2022 at 08:22:51PM +0100, Conor Dooley wrote: >>> From: Conor Dooley <conor.dooley@microchip.com> >>> >>> The k210 U-Boot port has been using the clocks defined in the >>> devicetree to bring up the board's SRAM, but this violates the >>> dt-schema. As such, move the clocks to a dedicated node with >>> the same compatible string & document it. >>> >>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>> --- >>> .../memory-controllers/canaan,k210-sram.yaml | 52 +++++++++++++++++++ >>> 1 file changed, 52 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >>> new file mode 100644 >>> index 000000000000..82be32757713 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml >>> @@ -0,0 +1,52 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/memory-controllers/canaan,k210-sram.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Canaan K210 SRAM memory controller >>> + >>> +description: | >> >> Don't need '|'. >> >>> + The Canaan K210 SRAM memory controller is initialised and programmed by >>> + firmware, but an OS might want to read its registers for error reporting >>> + purposes and to learn about the DRAM topology. >> >> How the OS going to do that? You don't have any way defined to access >> the registers. > > Eugh, copy paste. I'll rephrase in the respin. It should be "initialised by > firmware." There are no registers, only clocks. s/firmware/bootloader > >> >> Also, where is the SRAM address itself defined? > > The actual sram is in the memory node. > >> >>> + >>> +maintainers: >>> + - Conor Dooley <conor@kernel.org> >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - canaan,k210-sram >>> + >>> + clocks: >>> + minItems: 1 >>> + items: >>> + - description: sram0 clock >>> + - description: sram1 clock >>> + - description: aisram clock >>> + >>> + clock-names: >>> + minItems: 1 >>> + items: >>> + - const: sram0 >>> + - const: sram1 >>> + - const: aisram >>> + >>> +required: >>> + - compatible >>> + - clocks >>> + - clock-names >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/clock/k210-clk.h> >>> + memory-controller { >>> + compatible = "canaan,k210-sram"; >>> + clocks = <&sysclk K210_CLK_SRAM0>, >>> + <&sysclk K210_CLK_SRAM1>, >>> + <&sysclk K210_CLK_AI>; >>> + clock-names = "sram0", "sram1", "aisram"; >>> + }; >>> -- >>> 2.37.0 >>> >>> > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
diff --git a/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml new file mode 100644 index 000000000000..82be32757713 --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/memory-controllers/canaan,k210-sram.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Canaan K210 SRAM memory controller + +description: | + The Canaan K210 SRAM memory controller is initialised and programmed by + firmware, but an OS might want to read its registers for error reporting + purposes and to learn about the DRAM topology. + +maintainers: + - Conor Dooley <conor@kernel.org> + +properties: + compatible: + enum: + - canaan,k210-sram + + clocks: + minItems: 1 + items: + - description: sram0 clock + - description: sram1 clock + - description: aisram clock + + clock-names: + minItems: 1 + items: + - const: sram0 + - const: sram1 + - const: aisram + +required: + - compatible + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/k210-clk.h> + memory-controller { + compatible = "canaan,k210-sram"; + clocks = <&sysclk K210_CLK_SRAM0>, + <&sysclk K210_CLK_SRAM1>, + <&sysclk K210_CLK_AI>; + clock-names = "sram0", "sram1", "aisram"; + };