Re: [PATCH 2/6] mips: bmips: rework and cache CBR addr handling

Florian Fainelli
2024-05-03T19:00:17Z

[-- Attachment #1: Type: text/plain, Size: 6062 bytes --]

On 5/3/24 06:54, Christian Marangi wrote:
> Rework the handling of the CBR address and cache it. This address
> doesn't chance and can be cache instead of calling the register every
> time.

s/change/change/
s/be cache/be cached/
s/calling the register/reading from the register/

>
> This is in preparation of permitting to tweak the CBR address in DT with
> broken SoC or bootloader.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> arch/mips/bmips/dma.c | 7 +++----
> arch/mips/bmips/setup.c | 6 +++++-
> arch/mips/include/asm/bmips.h | 1 +
> arch/mips/kernel/smp-bmips.c | 31 ++++++++++++++-----------------
> 4 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/arch/mips/bmips/dma.c b/arch/mips/bmips/dma.c
> index 3779e7855bd7..799cc3e12fc3 100644
> --- a/arch/mips/bmips/dma.c
> +++ b/arch/mips/bmips/dma.c
> @@ -9,7 +9,6 @@ bool bmips_rac_flush_disable;
>
> void arch_sync_dma_for_cpu_all(void)
> {
> - void __iomem *cbr = BMIPS_GET_CBR();
> u32 cfg;
>
> if (boot_cpu_type() != CPU_BMIPS3300 &&
> @@ -21,7 +20,7 @@ void arch_sync_dma_for_cpu_all(void)
> return;
>
> /* Flush stale data out of the readahead cache */
> - cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
> - __raw_writel(cfg | 0x100, cbr + BMIPS_RAC_CONFIG);
> - __raw_readl(cbr + BMIPS_RAC_CONFIG);
> + cfg = __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_writel(cfg | 0x100, bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
> }
> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
> index 66a8ba19c287..18561d426f89 100644
> --- a/arch/mips/bmips/setup.c
> +++ b/arch/mips/bmips/setup.c
> @@ -34,6 +34,8 @@
> #define REG_BCM6328_OTP ((void __iomem *)CKSEG1ADDR(0x1000062c))
> #define BCM6328_TP1_DISABLED BIT(9)
>
> +/* CBR addr doesn't change and we can cache it */
> +void __iomem *bmips_cbr_addr;

Maybe __ro_after_init and __read_mostly, too?

> extern bool bmips_rac_flush_disable;
>
> static const unsigned long kbase = VMLINUX_LOAD_ADDRESS & 0xfff00000;
> @@ -111,7 +113,7 @@ static void bcm6358_quirks(void)
> * because the bootloader is not initializing it properly.
> */
> bmips_rac_flush_disable = !!(read_c0_brcm_cmt_local() & (1 << 31)) ||
> - !!BMIPS_GET_CBR();
> + !!bmips_cbr_addr;
> }
>
> static void bcm6368_quirks(void)
> @@ -144,6 +146,8 @@ static void __init bmips_init_cfe(void)
>
> void __init prom_init(void)
> {
> + /* Cache CBR addr before CPU/DMA setup */
> + bmips_cbr_addr = BMIPS_GET_CBR();
> bmips_init_cfe();
> bmips_cpu_setup();
> register_bmips_smp_ops();
> diff --git a/arch/mips/include/asm/bmips.h b/arch/mips/include/asm/bmips.h
> index 581a6a3c66e4..3a1cdfddb987 100644
> --- a/arch/mips/include/asm/bmips.h
> +++ b/arch/mips/include/asm/bmips.h
> @@ -81,6 +81,7 @@ extern char bmips_smp_movevec[];
> extern char bmips_smp_int_vec[];
> extern char bmips_smp_int_vec_end[];
>
> +extern void __iomem *bmips_cbr_addr;
> extern int bmips_smp_enabled;
> extern int bmips_cpu_offset;
> extern cpumask_t bmips_booted_mask;
> diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
> index b3dbf9ecb0d6..6048c471b5ee 100644
> --- a/arch/mips/kernel/smp-bmips.c
> +++ b/arch/mips/kernel/smp-bmips.c
> @@ -518,14 +518,12 @@ static void bmips_set_reset_vec(int cpu, u32 val)
> info.val = val;
> bmips_set_reset_vec_remote(&info);
> } else {
> - void __iomem *cbr = BMIPS_GET_CBR();
> -
> if (cpu == 0)
> - __raw_writel(val, cbr + BMIPS_RELO_VECTOR_CONTROL_0);
> + __raw_writel(val, bmips_cbr_addr + BMIPS_RELO_VECTOR_CONTROL_0);
> else {
> if (current_cpu_type() != CPU_BMIPS4380)
> return;
> - __raw_writel(val, cbr + BMIPS_RELO_VECTOR_CONTROL_1);
> + __raw_writel(val, bmips_cbr_addr + BMIPS_RELO_VECTOR_CONTROL_1);
> }
> }
> __sync();
> @@ -591,7 +589,6 @@ asmlinkage void __weak plat_wired_tlb_setup(void)
>
> void bmips_cpu_setup(void)
> {
> - void __iomem __maybe_unused *cbr = BMIPS_GET_CBR();

Could keep the local variable here to minimize the amount of changes,
should not matter how the resulting code is generated, as it should
cache it in a register.

> u32 __maybe_unused cfg;
>
> switch (current_cpu_type()) {
> @@ -607,17 +604,17 @@ void bmips_cpu_setup(void)
> clear_c0_brcm_reset(BIT(16));
>
> /* Flush and enable RAC */
> - cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
> - __raw_writel(cfg | 0x100, cbr + BMIPS_RAC_CONFIG);
> - __raw_readl(cbr + BMIPS_RAC_CONFIG);
> + cfg = __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_writel(cfg | 0x100, bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
>
> - cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
> - __raw_writel(cfg | 0xf, cbr + BMIPS_RAC_CONFIG);
> - __raw_readl(cbr + BMIPS_RAC_CONFIG);
> + cfg = __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_writel(cfg | 0xf, bmips_cbr_addr + BMIPS_RAC_CONFIG);
> + __raw_readl(bmips_cbr_addr + BMIPS_RAC_CONFIG);
>
> - cfg = __raw_readl(cbr + BMIPS_RAC_ADDRESS_RANGE);
> - __raw_writel(cfg | 0x0fff0000, cbr + BMIPS_RAC_ADDRESS_RANGE);
> - __raw_readl(cbr + BMIPS_RAC_ADDRESS_RANGE);
> + cfg = __raw_readl(bmips_cbr_addr + BMIPS_RAC_ADDRESS_RANGE);
> + __raw_writel(cfg | 0x0fff0000, bmips_cbr_addr + BMIPS_RAC_ADDRESS_RANGE);
> + __raw_readl(bmips_cbr_addr + BMIPS_RAC_ADDRESS_RANGE);
> break;
>
> case CPU_BMIPS4380:
> @@ -627,9 +624,9 @@ void bmips_cpu_setup(void)
> case 0x2a042:
> case 0x2a044:
> case 0x2a060:
> - cfg = __raw_readl(cbr + BMIPS_L2_CONFIG);
> - __raw_writel(cfg & ~0x07000000, cbr + BMIPS_L2_CONFIG);
> - __raw_readl(cbr + BMIPS_L2_CONFIG);
> + cfg = __raw_readl(bmips_cbr_addr + BMIPS_L2_CONFIG);
> + __raw_writel(cfg & ~0x07000000, bmips_cbr_addr + BMIPS_L2_CONFIG);
> + __raw_readl(bmips_cbr_addr + BMIPS_L2_CONFIG);
> }
>
> /* clear BHTD to enable branch history table */

--
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]