| Message ID | 20220705215213.1802496-5-mail@conchuod.ie |
|---|---|
| State | New |
| Headers | show |
| Series | Canaan devicetree fixes | expand |
Damien, Krzysztof,
I know this particular version has not been posted for all that
long, but this binding is (functionally) unchanged for a few
versions now. Are you happy with this approach Damien?
U-Boot only cares about the compatible & the clocks property,
not the regs etc.
I (lazily) tested it in U-Boot with the following diff:
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 3cc8379133..314db88340 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -82,11 +82,14 @@
sram: memory@80000000 {
device_type = "memory";
+ reg = <0x80000000 0x400000>, /* sram0 4 MiB */
+ <0x80400000 0x200000>, /* sram1 2 MiB */
+ <0x80600000 0x200000>; /* aisram 2 MiB */
+ u-boot,dm-pre-reloc;
+ };
+
+ sram_controller: memory-controller {
compatible = "canaan,k210-sram";
- reg = <0x80000000 0x400000>,
- <0x80400000 0x200000>,
- <0x80600000 0x200000>;
- reg-names = "sram0", "sram1", "aisram";
clocks = <&sysclk K210_CLK_SRAM0>,
<&sysclk K210_CLK_SRAM1>,
<&sysclk K210_CLK_AI>;
If so, could you queue this for 5.20 please Krzysztof, unless
you've got concerns about it?
Thanks,
Conor.
On 05/07/2022 22:52, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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..f81fb866e319
> --- /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 responsible for the system's 8 MiB
> + of SRAM. The controller is initialised by the bootloader, which configures
> + its clocks, before OS bringup.
> +
> +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 7/11/22 04:39, Conor.Dooley@microchip.com wrote: > Damien, Krzysztof, > > I know this particular version has not been posted for all that > long, but this binding is (functionally) unchanged for a few > versions now. Are you happy with this approach Damien? > U-Boot only cares about the compatible & the clocks property, > not the regs etc. > > I (lazily) tested it in U-Boot with the following diff: If both the kernel and u-boot still work as expected with this change, I am OK with it. > > diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi > index 3cc8379133..314db88340 100644 > --- a/arch/riscv/dts/k210.dtsi > +++ b/arch/riscv/dts/k210.dtsi > @@ -82,11 +82,14 @@ > > sram: memory@80000000 { > device_type = "memory"; > + reg = <0x80000000 0x400000>, /* sram0 4 MiB */ > + <0x80400000 0x200000>, /* sram1 2 MiB */ > + <0x80600000 0x200000>; /* aisram 2 MiB */ > + u-boot,dm-pre-reloc; > + }; > + > + sram_controller: memory-controller { > compatible = "canaan,k210-sram"; > - reg = <0x80000000 0x400000>, > - <0x80400000 0x200000>, > - <0x80600000 0x200000>; > - reg-names = "sram0", "sram1", "aisram"; > clocks = <&sysclk K210_CLK_SRAM0>, > <&sysclk K210_CLK_SRAM1>, > <&sysclk K210_CLK_AI>; > > If so, could you queue this for 5.20 please Krzysztof, unless > you've got concerns about it? > > Thanks, > Conor. > > On 05/07/2022 22:52, Conor Dooley wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> 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..f81fb866e319 >> --- /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 responsible for the system's 8 MiB >> + of SRAM. The controller is initialised by the bootloader, which configures >> + its clocks, before OS bringup. >> + >> +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 06/07/2022 00:52, 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> > --- Does not apply to my tree. Please rebase and resend. Best regards, Krzysztof
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..f81fb866e319 --- /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 responsible for the system's 8 MiB + of SRAM. The controller is initialised by the bootloader, which configures + its clocks, before OS bringup. + +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"; + };