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

[Guo Ren]

On Tue, Jun 13, 2023 at 10:43 AM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>
> On 13 Jun 2023, at 03:21, Guo Ren <guoren at kernel.org> wrote:
> >
> > 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();
> > }
>
> So you want your highly-privileged firmware to be dynamically
> generating code, with the ability to have a WX mapping in the
> first place?
We don't expect WX mapping, but the current fdt_reset_init is after
the pmp config. I need to move it into early_init. Thx for pointing it
out.



>
> Jess
>
> > }
> > ==========
> >
> > /* 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
>
>


--
Best Regards
 Guo Ren