[RFC PATCH 2/2] lib: reset: thead: Correct the naming convention of dts

[Guo Ren]

On Mon, Jun 12, 2023 at 9:05 AM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>
> On 12 Jun 2023, at 01:57, Guo Ren <guoren at kernel.org> wrote:
> >
> > Hi Conor,
> >
> > Jisheng Zhang would update the Linux yaml patch, here is the final dts
> > format of the reset controller:
> >
> >               reset-controller at ffff019050 {
> >                       compatible = "thead,cpu-reset";
> >                       reg = <0xff 0xff019050 0x0 0x4>, <0xff
> > 0xff015004 0x0 0x0>;
> >                       reset-ctrl = <0x1c>;
> >                       clone-csrs = <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3
>
> This is pretty horrible, there’s no indirect CSR read/write
> instruction, so what are you expecting drivers to do with this? Have a
We've done it in the opensbi:
https://github.com/riscv-software-src/opensbi/blob/master/lib/utils/reset/fdt_reset_thead.c

A lot of SoC customers would intergrate their own CSRs though our rtl
code interface, these CSRs are initilized in their primary hart in
early boot stage / jtag gdbinit script, the secondary harts just copy
the CSRs' values from boot hart. That means we could provide a unified
way to all customer.

> big switch statement for every possible CSR?
We needn't big switch statement, boot hart store the csrs setting
codes in the memory, and secondary harts excute them to set the csrs.
Here is the code snippet:

struct custom_csr custom_csr[MAX_CUSTOM_CSR];

#define CSR_OPCODE 0x39073
static void clone_csrs(int cnt)
{
unsigned long i;

for (i = 0; i < cnt; i++) {
/* Write csr BIT[31 - 20] to stub */
__reset_thead_csr_stub[3*i + 1] = CSR_OPCODE | (custom_csr[i].index << 20);

/* Mask csr BIT[31 - 20] */
*(u32 *)&__fdt_reset_thead_csrr &= BIT(20) - 1;
smp_mb();

/* Write csr BIT[31 - 20] to __fdt_reset_thead_csrr */
*(u32 *)&__fdt_reset_thead_csrr |= custom_csr[i].index << 20;
smp_mb();

RISCV_FENCE_I;

custom_csr[i].value = __fdt_reset_thead_csrr();
}

}
==========

/* Prepare clone csrs */
val_w = fdt_getprop(fdt, nodeoff, "csr-copy", &len);
if (len > 0 && val_w) {
int cnt;

cnt = len / sizeof(fdt32_t);
if (cnt > MAX_CUSTOM_CSR)
sbi_hart_hang();

for (i = 0; i < cnt; i++) {
custom_csr[i].index = fdt32_to_cpu(val_w[i]);
}

if (cnt)
clone_csrs(cnt); // prepare the secondary harts' excution code
}






>
> Jess
>
> > 0x7c5 0x7cc 0x7ce>;
> >                };
> >
> > The reset-ctrl is used to control different parts of soc, generally, a
> > bit indicates a reset signal (a core/a interconnect/a subsystem).
> >
> > On Thu, May 25, 2023 at 2:06 PM Guo Ren <guoren at kernel.org> wrote:
> >>
> >> On Thu, May 25, 2023 at 1:33 PM Conor Dooley <conor at kernel.org> wrote:
> >>>
> >>>
> >>>
> >>> On 25 May 2023 04:15:36 IST, Guo Ren <guoren at kernel.org> wrote:
> >>>
> >>>> So it could be "thead,cpu-reset", okay?
> >>>
> >>> As a generic fallback compatible.
> >>>
> >>>> Actually, our core could let SoC vendors define their own custom
> >>>> CSRs/custom reset values of CSRs, so we don't know what would be added
> >>>> in the future. Put a array in dts instead of hard-code table is much
> >>>> more flexiblity.
> >>>
> >>> If there's going to be 700 different variations depending on what people do with openc910, then allowing it to be passed sounds like a good idea.
> >> Yes, that is what we want. th1520 is a example.
> >>
> >> Thanks for the review and correction! Your help is greatly appreciated
> >> in improving th1520 upstream.
> >>
> >>>
> >>> Thanks,
> >>> Conor.
> >>
> >>
> >>
> >> --
> >> Best Regards
> >> Guo Ren
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
>


-- 
Best Regards
 Guo Ren