| Message ID | 20230816173420.83232-6-CFSworks@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | Allwinner R528/T113s PSCI | expand |
On Wed, 16 Aug 2023 10:34:20 -0700 Sam Edwards <cfsworks@gmail.com> wrote: Hi Sam, > This is a hack for reviewer QoL. It is not being submitted for mainline > inclusion. > --- > arch/arm/cpu/armv7/sunxi/psci.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c > index b4ce4f6def..27bac291d5 100644 > --- a/arch/arm/cpu/armv7/sunxi/psci.c > +++ b/arch/arm/cpu/armv7/sunxi/psci.c > @@ -60,6 +60,18 @@ > > #define SUN8I_R528_C0_STATUS_STANDBYWFI (16) > > +/* 3 hacks for compatibility across v1/v2 of Andre's R528 support series */ > +#ifndef SUNXI_R_CPUCFG_BASE > +#define SUNXI_R_CPUCFG_BASE 0 > +#endif Mmh, I didn't find a better solution than keeping this in. > +#ifndef SUNXI_PRCM_BASE > +#define SUNXI_PRCM_BASE 0 So this is now handled. As Samuel pointed out, the R329 (another member of the "NCAT2" family), actually documents a PRCM block, and arguably the T113s/D1 have that as well, it's just not very useful (at least for U-Boot), so we didn't need it so far. I just put the address documented in the R329 manual into cpu_sunxi_ncat2.h, so the symbol expands properly. > +#endif > +#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE) > +#undef SUNXI_CPUCFG_BASE > +#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE So what's the story with this? Do we name this differently (SUNXI_CPUX_BASE) because the IP block is different from the other SoCs? Or is there another SUNXI_CPUCFG IP block on the R528/T113s SoCs? If not, I think we should use the SUNXI_CPUCFG_BASE name directly in cpu_sunxi_ncat2.h, as we never claimed that same names for some MMIO address blocks means they are compatible. Please let me know if I miss something. Cheers, Andre > +#endif > + > static void __secure cp15_write_cntp_tval(u32 tval) > { > asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
On 9/27/23 10:32, Andre Przywara wrote: > On Wed, 16 Aug 2023 10:34:20 -0700 > Sam Edwards <cfsworks@gmail.com> wrote: > > Hi Sam, Hi Andre, > Mmh, I didn't find a better solution than keeping this in. I'll keep it if your R528 v2 doesn't find some other way to address it. >> +#endif >> +#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE) >> +#undef SUNXI_CPUCFG_BASE >> +#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE > > So what's the story with this? Do we name this differently > (SUNXI_CPUX_BASE) because the IP block is different from the other SoCs? > Or is there another SUNXI_CPUCFG IP block on the R528/T113s SoCs? > > If not, I think we should use the SUNXI_CPUCFG_BASE name directly in > cpu_sunxi_ncat2.h, as we never claimed that same names for some MMIO > address blocks means they are compatible. > > Please let me know if I miss something. That's just for compatibility with R528 series v1. It's expected that you'll rename it to SUNXI_CPUCFG_BASE for v2. The preprocessor trickery looks for *both* being defined and applies the update. The rest of the code proceeds using SUNXI_CPUCFG_BASE. (Keep in mind this is particular patch is a hack patch, it's not considered for inclusion.) Warm regards, Sam
On Wed, 27 Sep 2023 17:28:51 -0600 Sam Edwards <cfsworks@gmail.com> wrote: > On 9/27/23 10:32, Andre Przywara wrote: > > On Wed, 16 Aug 2023 10:34:20 -0700 > > Sam Edwards <cfsworks@gmail.com> wrote: > > > > Hi Sam, > > Hi Andre, > > > Mmh, I didn't find a better solution than keeping this in. > > I'll keep it if your R528 v2 doesn't find some other way to address it. > > >> +#endif > >> +#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE) > >> +#undef SUNXI_CPUCFG_BASE > >> +#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE > > > > So what's the story with this? Do we name this differently > > (SUNXI_CPUX_BASE) because the IP block is different from the other SoCs? > > Or is there another SUNXI_CPUCFG IP block on the R528/T113s SoCs? > > > > If not, I think we should use the SUNXI_CPUCFG_BASE name directly in > > cpu_sunxi_ncat2.h, as we never claimed that same names for some MMIO > > address blocks means they are compatible. > > > > Please let me know if I miss something. > > That's just for compatibility with R528 series v1. It's expected that > you'll rename it to SUNXI_CPUCFG_BASE for v2. The preprocessor trickery > looks for *both* being defined and applies the update. The rest of the > code proceeds using SUNXI_CPUCFG_BASE. (Keep in mind this is particular > patch is a hack patch, it's not considered for inclusion.) Yes, I got this, but surely the expectation is that those fixes should not be needed anymore after a v2 of the R528 support series, right? Which I am preparing as we speak, so I am supposed to fix them there, and just wanted to double check whether my solution is in line with what you had in mind. After all you seem to be deeper into this CPUCFG stuff than I am. Cheers, Andre
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c index b4ce4f6def..27bac291d5 100644 --- a/arch/arm/cpu/armv7/sunxi/psci.c +++ b/arch/arm/cpu/armv7/sunxi/psci.c @@ -60,6 +60,18 @@ #define SUN8I_R528_C0_STATUS_STANDBYWFI (16) +/* 3 hacks for compatibility across v1/v2 of Andre's R528 support series */ +#ifndef SUNXI_R_CPUCFG_BASE +#define SUNXI_R_CPUCFG_BASE 0 +#endif +#ifndef SUNXI_PRCM_BASE +#define SUNXI_PRCM_BASE 0 +#endif +#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE) +#undef SUNXI_CPUCFG_BASE +#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE +#endif + static void __secure cp15_write_cntp_tval(u32 tval) { asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));