On Tue, 28 Oct 2025 10:12:26 +0100
Richard Genoud <[email protected]> wrote:

Hi Richard,

many thanks for doing this change.

> The sunxi_ccm_reg is legacy, drop its usage from nand related code
> 
> For that, CCU_NAND0_CLK_CFG and CCU_AHB_GATE1 are added to the clock
> files when missing.
> And clock code in sunxi_nand{,_spl}.c and board.c are changed to use the
> new scheme.
> 
> Moreover, drop AHB_DIV_1 in favor of the more readable CCM_NAND_CTRL_M/N
> 
> Suggested-by: Andre Przywara <[email protected]>
> Signed-off-by: Richard Genoud <[email protected]>
> ---
>  arch/arm/include/asm/arch-sunxi/clock_sun4i.h      |  1 +
>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h      |  1 +
>  arch/arm/include/asm/arch-sunxi/clock_sun8i_a83t.h |  1 +
>  arch/arm/include/asm/arch-sunxi/clock_sun9i.h      |  2 ++
>  board/sunxi/board.c                                | 11 ++++++-----
>  drivers/mtd/nand/raw/sunxi_nand.c                  | 11 +++++------
>  drivers/mtd/nand/raw/sunxi_nand_spl.c              | 13 +++++++------
>  7 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> index 00bdd5f938df..caa4b62b3e2d 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> @@ -11,6 +11,7 @@
>  #define _SUNXI_CLOCK_SUN4I_H
>  
>  #define CCU_AHB_GATE0                0x60
> +#define CCU_NAND0_CLK_CFG    0x80
>  #define CCU_MMC0_CLK_CFG     0x88
>  #define CCU_MMC1_CLK_CFG     0x8c
>  #define CCU_MMC2_CLK_CFG     0x90
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> index 28c3faccbbcf..bdb08a32d01b 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> @@ -11,6 +11,7 @@
>  #define _SUNXI_CLOCK_SUN6I_H
>  
>  #define CCU_AHB_GATE0                0x060
> +#define CCU_NAND0_CLK_CFG    0x80

Just a nit, but can you please change this to 0x080, to align with the
other numbers?

>  #define CCU_MMC0_CLK_CFG     0x088
>  #define CCU_MMC1_CLK_CFG     0x08c
>  #define CCU_MMC2_CLK_CFG     0x090
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun8i_a83t.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun8i_a83t.h
> index 5ad2163926ad..70cd0ca00d21 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun8i_a83t.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun8i_a83t.h
> @@ -14,6 +14,7 @@
>  #define _SUNXI_CLOCK_SUN8I_A83T_H
>  
>  #define CCU_AHB_GATE0                0x060
> +#define CCU_NAND0_CLK_CFG    0x80

Same here.

>  #define CCU_MMC0_CLK_CFG     0x088
>  #define CCU_MMC1_CLK_CFG     0x08c
>  #define CCU_MMC2_CLK_CFG     0x090
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun9i.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
> index 8d696e533f8f..3448f3fb3225 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun9i.h
> @@ -12,11 +12,13 @@
>  #include <linux/bitops.h>
>  #endif
>  
> +#define CCU_NAND0_CLK_CFG    0x400
>  #define CCU_MMC0_CLK_CFG     0x410
>  #define CCU_MMC1_CLK_CFG     0x414
>  #define CCU_MMC2_CLK_CFG     0x418
>  #define CCU_MMC3_CLK_CFG     0x41c
>  #define CCU_AHB_GATE0                0x580
> +#define CCU_AHB_GATE1                0x584
>  #define CCU_AHB_RESET0_CFG   0x5a0

For the records, I checked the value of the offsets, they look correct.

>  struct sunxi_ccm_reg {
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 2929bc17f084..09b531b7f52a 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -307,15 +307,16 @@ static void nand_pinmux_setup(void)
>  
>  static void nand_clock_setup(void)
>  {
> -     struct sunxi_ccm_reg *const ccm =
> -             (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +     void * const ccm = (void *)SUNXI_CCM_BASE;
>  
> -     setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
> +     setbits_le32(ccm + CCU_AHB_GATE0,
> +                  (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
>  #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I || \
>      defined CONFIG_MACH_SUN9I || defined CONFIG_MACH_SUN50I

So the usage of #define's for the offsets instead of struct members would
now allow for those preprocessor guards to be replaced by C statements:
        if (IS_ENABLED(CONFIG_MACH_SUN6I) || IS_ENABLED(CONFIG_MACH_SUN8I)
        ...

> -     setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0));
> +     setbits_le32(ccm + CCU_AHB_RESET0_CFG, (1 << AHB_GATE_OFFSET_NAND0));
>  #endif
> -     setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
> +     setbits_le32(ccm + CCU_NAND0_CLK_CFG, CCM_NAND_CTRL_ENABLE |
> +                  CCM_NAND_CTRL_N(0) | CCM_NAND_CTRL_M(1));
>  }
>  
>  void board_nand_init(void)
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c 
> b/drivers/mtd/nand/raw/sunxi_nand.c
> index c6b9b2a4ebac..ba2740ed1876 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -289,8 +289,7 @@ static inline struct sunxi_nfc *to_sunxi_nfc(struct 
> nand_hw_control *ctrl)
>  
>  static void sunxi_nfc_set_clk_rate(unsigned long hz)
>  {
> -     struct sunxi_ccm_reg *const ccm =
> -     (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +     void * const ccm = (void *)SUNXI_CCM_BASE;
>       int div_m, div_n;
>  
>       div_m = (clock_get_pll6() + hz - 1) / hz;
> @@ -305,14 +304,14 @@ static void sunxi_nfc_set_clk_rate(unsigned long hz)
>       /* config mod clock */
>       writel(CCM_NAND_CTRL_ENABLE | CCM_NAND_CTRL_PLL6 |
>              CCM_NAND_CTRL_N(div_n) | CCM_NAND_CTRL_M(div_m),
> -            &ccm->nand0_clk_cfg);
> +            ccm + CCU_NAND0_CLK_CFG);
>  
>       /* gate on nand clock */
> -     setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0));
> +     setbits_le32(ccm + CCU_AHB_GATE0, (1 << AHB_GATE_OFFSET_NAND0));
>  #ifdef CONFIG_MACH_SUN9I

same here

> -     setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
> +     setbits_le32(ccm + CCU_AHB_GATE1, (1 << AHB_GATE_OFFSET_DMA));
>  #else
> -     setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
> +     setbits_le32(ccm + CCU_AHB_GATE0, (1 << AHB_GATE_OFFSET_DMA));
>  #endif
>  }
>  
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c 
> b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index e90450439e41..2fcf52658008 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -533,14 +533,15 @@ unsigned int nand_page_size(void)
>  
>  void nand_deselect(void)
>  {
> -     struct sunxi_ccm_reg *const ccm =
> -             (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +     void * const ccm = (void *)SUNXI_CCM_BASE;
>  
> -     clrbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
> +     clrbits_le32(ccm + CCU_AHB_GATE0,
> +                  (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0));
>  #ifdef CONFIG_MACH_SUN9I

and here.

But the actual changes look alright, including the usage of
CCM_NAND_CTRL_N() and CCM_NAND_CTRL_M().

Cheers,
Andre

> -     clrbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA));
> +     clrbits_le32(ccm + CCU_AHB_GATE1, (1 << AHB_GATE_OFFSET_DMA));
>  #else
> -     clrbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA));
> +     clrbits_le32(ccm + CCU_AHB_GATE0, (1 << AHB_GATE_OFFSET_DMA));
>  #endif
> -     clrbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1);
> +     clrbits_le32(ccm + CCU_NAND0_CLK_CFG, CCM_NAND_CTRL_ENABLE |
> +                  CCM_NAND_CTRL_N(0) | CCM_NAND_CTRL_M(1));
>  }

Reply via email to