[RFC PATCH 2/2] lib: reset: thead: Correct the naming convention of dts
Conor Dooley
conor at kernel.org
Tue May 23 08:51:00 PDT 2023
Hey Guo Ren,
On Tue, May 23, 2023 at 05:46:49AM -0400, guoren at kernel.org wrote:
> From: Guo Ren <guoren at linux.alibaba.com>
>
> Correct the naming convention to fit Linux kernel upstream rule.
Understatement of the year contender? ;)
This is a backwards incompatible change, based on suggestions that I
made on LKML, with a view to creating a yaml binding for this thing.
Everyone else might disagree with me, but I think "proper" bindings
should be written for this stuff so that use would be common across SBI
providers etc *but* in doing so do you not want to keep OpenSBI
backwards compatible with the markdown "binding" in the process?
Skimming this patch, the old way of doing things would no longer work?
> Link: https://lore.kernel.org/linux-riscv/CAJF2gTRG5edPnVmhMtv67OANpOynL0br0wcrQCave88_KkyZcg@mail.gmail.com/
> Signed-off-by: Guo Ren <guoren at linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren at kernel.org>
> Cc: Conor Dooley <conor.dooley at microchip.com>
> Cc: Jisheng Zhang <jszhang at kernel.org>
> Cc: Wei Fu <wefu at redhat.com>
> ---
> docs/platform/thead-c9xx.md | 21 +++++-------
> lib/utils/reset/fdt_reset_thead.c | 57 +++++++++++++++----------------
> 2 files changed, 35 insertions(+), 43 deletions(-)
>
> diff --git a/docs/platform/thead-c9xx.md b/docs/platform/thead-c9xx.md
> index 8bb9e91f1a9b..35cca94b5bb9 100644
> --- a/docs/platform/thead-c9xx.md
> +++ b/docs/platform/thead-c9xx.md
> @@ -19,9 +19,6 @@ because it uses generic platform.
> CROSS_COMPILE=riscv64-linux-gnu- PLATFORM=generic /usr/bin/make
> ```
>
> -The *T-HEAD C9xx* DTB provided to OpenSBI generic firmwares will usually have
> -"riscv,clint0", "riscv,plic0", "thead,reset-sample" compatible strings.
> -
> DTS Example1: (Single core, eg: Allwinner D1 - c906)
> ----------------------------------------------------
>
> @@ -75,8 +72,8 @@ DTS Example1: (Single core, eg: Allwinner D1 - c906)
> }
> ```
>
> -DTS Example2: (Multi cores with soc reset-regs)
> ------------------------------------------------
> +DTS Example2: (Multi cores, eg: T-HEAD th1520 - c910)
> +-----------------------------------------------------
>
> ```
> cpus {
> @@ -143,13 +140,11 @@ DTS Example2: (Multi cores with soc reset-regs)
> compatible = "simple-bus";
> ranges;
>
> - reset: reset-sample {
> - compatible = "thead,reset-sample";
> - entry-reg = <0xff 0xff019050>;
> - entry-cnt = <4>;
> - control-reg = <0xff 0xff015004>;
> - control-val = <0x1c>;
> - csr-copy = <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc>;
> + reset-controller at ffff019050 {
> + compatible = "thead,th1520-cpu-reset";
> + reg = <0xff 0xff019050 0x0 0x4>, <0xff 0xff015004 0x0 0x0>;
> + reset-ctrl-val = <0x1c>;
> + csr-copy = <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc 0x7ce>;
I also did not suggest either rest-ctrl-val or csr-copy, as both are
surely able to be determined from match data based on the compatible
string.
Thanks,
Conor.
> };
>
> clint0: clint at ffdc000000 {
> @@ -186,7 +181,7 @@ DTS Example2: (Multi cores with old reset csrs)
> -----------------------------------------------
> ```
> reset: reset-sample {
> - compatible = "thead,reset-sample";
> + compatible = "thead,common-cpu-reset";
> using-csr-reset;
> csr-copy = <0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc
> 0x3b0 0x3b1 0x3b2 0x3b3
> diff --git a/lib/utils/reset/fdt_reset_thead.c b/lib/utils/reset/fdt_reset_thead.c
> index ff7d2981b42f..760c117f6178 100644
> --- a/lib/utils/reset/fdt_reset_thead.c
> +++ b/lib/utils/reset/fdt_reset_thead.c
> @@ -5,6 +5,8 @@
> #include <libfdt.h>
> #include <sbi/riscv_io.h>
> #include <sbi/sbi_bitops.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi/sbi_error.h>
> #include <sbi/sbi_hart.h>
> #include <sbi/sbi_scratch.h>
> #include <sbi/sbi_system.h>
> @@ -58,11 +60,10 @@ extern void __thead_pre_start_warm(void);
> static int thead_reset_init(void *fdt, int nodeoff,
> const struct fdt_match *match)
> {
> - char *p;
> - const fdt64_t *val;
> + int rc, len, i;
> + u32 tmp;
> const fdt32_t *val_w;
> - int len, i;
> - u32 t, tmp = 0;
> + uint64_t reg_addr, reg_size;
>
> /* Prepare clone csrs */
> val_w = fdt_getprop(fdt, nodeoff, "csr-copy", &len);
> @@ -85,45 +86,41 @@ static int thead_reset_init(void *fdt, int nodeoff,
> if (fdt_getprop(fdt, nodeoff, "using-csr-reset", &len)) {
> csr_write(0x7c7, (ulong)&__thead_pre_start_warm);
> csr_write(0x7c6, -1);
> + goto out;
> }
>
> /* Custom reset method for secondary harts */
> - val = fdt_getprop(fdt, nodeoff, "entry-reg", &len);
> - if (len > 0 && val) {
> - p = (char *)(ulong)fdt64_to_cpu(*val);
> -
> - val_w = fdt_getprop(fdt, nodeoff, "entry-cnt", &len);
> - if (len > 0 && val_w) {
> - tmp = fdt32_to_cpu(*val_w);
> -
> - for (i = 0; i < tmp; i++) {
> - t = (ulong)&__thead_pre_start_warm;
> - writel(t, p + (8 * i));
> - t = (u64)(ulong)&__thead_pre_start_warm >> 32;
> - writel(t, p + (8 * i) + 4);
> - }
> - }
> + rc = fdt_get_node_addr_size(fdt, nodeoff, 0, ®_addr, ®_size);
> + if (rc < 0)
> + return SBI_ENODEV;
> +
> + for (i = 0; i < reg_size; i++) {
> + tmp = (ulong)&__thead_pre_start_warm;
> + writel(tmp, (char *)(ulong)reg_addr + (i * 8));
> + tmp = (u64)(ulong)&__thead_pre_start_warm >> 32;
> + writel(tmp, (char *)(ulong)reg_addr + (i * 8) + 4);
> + }
>
> - val = fdt_getprop(fdt, nodeoff, "control-reg", &len);
> - if (len > 0 && val) {
> - p = (void *)(ulong)fdt64_to_cpu(*val);
> + rc = fdt_get_node_addr_size(fdt, nodeoff, 1, ®_addr, ®_size);
> + if (rc < 0)
> + return SBI_ENODEV;
>
> - val_w = fdt_getprop(fdt, nodeoff, "control-val", &len);
> - if (len > 0 && val_w) {
> - tmp = fdt32_to_cpu(*val_w);
> - tmp |= readl(p);
> - writel(tmp, p);
> - }
> - }
> + val_w = fdt_getprop(fdt, nodeoff, "reset-ctrl-val", &len);
> + if (len > 0 && val_w) {
> + tmp = fdt32_to_cpu(*val_w);
> + tmp |= readl((char *)(ulong)reg_addr);
> + writel(tmp, (char *)(ulong)reg_addr);
> }
>
> +out:
> sbi_system_reset_add_device(&thead_reset);
>
> return 0;
> }
>
> static const struct fdt_match thead_reset_match[] = {
> - { .compatible = "thead,reset-sample" },
> + { .compatible = "thead,common-cpu-reset" },
> + { .compatible = "thead,th1520-cpu-reset" },
> { },
> };
>
> --
> 2.36.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20230523/10e8f96e/attachment-0001.sig>
More information about the opensbi
mailing list