[PATCH 03/24] mtd: rawnand: sunxi_spl: cosmetic: harmonize register defines with non spl file
Andre Przywara
andre.przywara at arm.com
Fri Oct 17 09:57:44 CEST 2025
Hi,
please avoid the use of the word "cosmectic" in the subject if the
change is not really cosmetic, as in just fixing some formatting or a
typo. This is refactoring that should not affect functionality, but this
change is beyond what I would characterise as "cosmetic".
Otherwise those are good changes, many thanks for cleaning this up and
unifying the two drivers!
On 16/10/2025 15:27, Richard Genoud wrote:
> Harmonize registers definition in sunxi_nand{,_spl}.c files
>
> This is a first step to then include the same file from both
> sunxi_nand{,_spl}.c files
Please also mention here that you remove unused definitions.
> Signed-off-by: Richard Genoud <richard.genoud at bootlin.com>
> ---
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 148 ++++++++++++--------------
> 1 file changed, 66 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 19091ece632b..3ccf716b99ac 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -14,48 +14,32 @@
> #include <linux/mtd/rawnand.h>
>
> /* registers */
> -#define NFC_CTL 0x00000000
> -#define NFC_ST 0x00000004
> -#define NFC_INT 0x00000008
> -#define NFC_TIMING_CTL 0x0000000C
> -#define NFC_TIMING_CFG 0x00000010
> -#define NFC_ADDR_LOW 0x00000014
> -#define NFC_ADDR_HIGH 0x00000018
> -#define NFC_SECTOR_NUM 0x0000001C
> -#define NFC_CNT 0x00000020
> -#define NFC_CMD 0x00000024
> -#define NFC_RCMD_SET 0x00000028
> -#define NFC_WCMD_SET 0x0000002C
> -#define NFC_IO_DATA 0x00000030
> -#define NFC_ECC_CTL 0x00000034
> -#define NFC_ECC_ST 0x00000038
> -#define NFC_DEBUG 0x0000003C
> -#define NFC_ECC_CNT0 0x00000040
> -#define NFC_ECC_CNT1 0x00000044
> -#define NFC_ECC_CNT2 0x00000048
> -#define NFC_ECC_CNT3 0x0000004C
> -#define NFC_USER_DATA_BASE 0x00000050
> -#define NFC_EFNAND_STATUS 0x00000090
> -#define NFC_SPARE_AREA 0x000000A0
> -#define NFC_PATTERN_ID 0x000000A4
> +#define NFC_REG_CTL 0x00000000
> +#define NFC_REG_ST 0x00000004
> +#define NFC_REG_ADDR_LOW 0x00000014
> +#define NFC_REG_ADDR_HIGH 0x00000018
> +#define NFC_REG_CNT 0x00000020
> +#define NFC_REG_CMD 0x00000024
> +#define NFC_REG_RCMD_SET 0x00000028
> +#define NFC_REG_ECC_CTL 0x00000034
> +#define NFC_REG_ECC_ST 0x00000038
> +#define NFC_REG_SPARE_AREA 0x000000A0
> #define NFC_RAM0_BASE 0x00000400
> #define NFC_RAM1_BASE 0x00000800
>
> -#define NFC_CTL_EN (1 << 0)
> -#define NFC_CTL_RESET (1 << 1)
> -#define NFC_CTL_RAM_METHOD (1 << 14)
> -#define NFC_CTL_PAGE_SIZE_MASK (0xf << 8)
> -#define NFC_CTL_PAGE_SIZE(a) ((fls(a) - 11) << 8)
> +#define NFC_EN (1 << 0)
Is there a strong reason to lose the _CTL_ infix here? I think it's
better to keep the register name in the bit field definition, otherwise
it becomes too generic and is prone to conflicts.
> +#define NFC_RESET (1 << 1)
> +#define NFC_PAGE_SHIFT_MSK (0xf << 8)
> +#define NFC_PAGE_SIZE(a) ((fls(a) - 11) << 8)
>
> #define NFC_ECC_EN (1 << 0)
> -#define NFC_ECC_PIPELINE (1 << 3)
> #define NFC_ECC_EXCEPTION (1 << 4)
> -#define NFC_ECC_BLOCK_SIZE (1 << 5)
> -#define NFC_ECC_RANDOM_EN (1 << 9)
> -#define NFC_ECC_RANDOM_DIRECTION (1 << 10)
> +#define NFC_ECC_BLOCK_512 (1 << 5)
> +#define NFC_RANDOM_EN (1 << 9)
> +#define NFC_RANDOM_DIRECTION (1 << 10)
>
> -#define NFC_ADDR_NUM_OFFSET 16
> -#define NFC_SEND_ADDR (1 << 19)
> +#define NFC_ADR_NUM_OFFSET 16
> +#define NFC_SEND_ADR (1 << 19)
But ADDR with two D's looks more correct to me?
In general, can't we keep the better version of each name, and adjust
both the SPL and proper code?
Cheers,
Andre
> #define NFC_ACCESS_DIR (1 << 20)
> #define NFC_DATA_TRANS (1 << 21)
> #define NFC_SEND_CMD1 (1 << 22)
> @@ -66,17 +50,17 @@
> #define NFC_ROW_AUTO_INC (1 << 27)
> #define NFC_SEND_CMD3 (1 << 28)
> #define NFC_SEND_CMD4 (1 << 29)
> -#define NFC_RAW_CMD (0 << 30)
> -#define NFC_ECC_CMD (1 << 30)
> -#define NFC_PAGE_CMD (2 << 30)
> +#define NFC_NORMAL_OP (0 << 30)
> +#define NFC_ECC_OP (1 << 30)
> +#define NFC_PAGE_OP (2 << 30)
>
> -#define NFC_ST_CMD_INT_FLAG (1 << 1)
> -#define NFC_ST_DMA_INT_FLAG (1 << 2)
> -#define NFC_ST_CMD_FIFO_STAT (1 << 3)
> +#define NFC_CMD_INT_FLAG (1 << 1)
> +#define NFC_DMA_INT_FLAG (1 << 2)
> +#define NFC_CMD_FIFO_STATUS (1 << 3)
>
> #define NFC_READ_CMD_OFFSET 0
> -#define NFC_RANDOM_READ_CMD0_OFFSET 8
> -#define NFC_RANDOM_READ_CMD1_OFFSET 16
> +#define NFC_RND_READ_CMD0_OFFSET 8
> +#define NFC_RND_READ_CMD1_OFFSET 16
>
> #define NFC_CMD_RNDOUTSTART 0xE0
> #define NFC_CMD_RNDOUT 0x05
> @@ -143,8 +127,8 @@ static inline int check_value_negated(int offset, int unexpected_bits,
>
> static int nand_wait_cmd_fifo_empty(void)
> {
> - if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT,
> - DEFAULT_TIMEOUT_US)) {
> + if (!check_value_negated(SUNXI_NFC_BASE + NFC_REG_ST,
> + NFC_CMD_FIFO_STATUS, DEFAULT_TIMEOUT_US)) {
> printf("nand: timeout waiting for empty cmd FIFO\n");
> return -ETIMEDOUT;
> }
> @@ -154,7 +138,7 @@ static int nand_wait_cmd_fifo_empty(void)
>
> static int nand_wait_int(void)
> {
> - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG,
> + if (!check_value(SUNXI_NFC_BASE + NFC_REG_ST, NFC_CMD_INT_FLAG,
> DEFAULT_TIMEOUT_US)) {
> printf("nand: timeout waiting for interruption\n");
> return -ETIMEDOUT;
> @@ -171,8 +155,8 @@ static int nand_exec_cmd(u32 cmd)
> if (ret)
> return ret;
>
> - writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);
> - writel(cmd, SUNXI_NFC_BASE + NFC_CMD);
> + writel(NFC_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_REG_ST);
> + writel(cmd, SUNXI_NFC_BASE + NFC_REG_CMD);
>
> return nand_wait_int();
> }
> @@ -183,13 +167,13 @@ void nand_init(void)
>
> board_nand_init();
>
> - val = readl(SUNXI_NFC_BASE + NFC_CTL);
> + val = readl(SUNXI_NFC_BASE + NFC_REG_CTL);
> /* enable and reset CTL */
> - writel(val | NFC_CTL_EN | NFC_CTL_RESET,
> - SUNXI_NFC_BASE + NFC_CTL);
> + writel(val | NFC_EN | NFC_RESET,
> + SUNXI_NFC_BASE + NFC_REG_CTL);
>
> - if (!check_value_negated(SUNXI_NFC_BASE + NFC_CTL,
> - NFC_CTL_RESET, DEFAULT_TIMEOUT_US)) {
> + if (!check_value_negated(SUNXI_NFC_BASE + NFC_REG_CTL,
> + NFC_RESET, DEFAULT_TIMEOUT_US)) {
> printf("Couldn't initialize nand\n");
> }
>
> @@ -203,42 +187,42 @@ static void nand_apply_config(const struct nfc_config *conf)
>
> nand_wait_cmd_fifo_empty();
>
> - val = readl(SUNXI_NFC_BASE + NFC_CTL);
> - val &= ~NFC_CTL_PAGE_SIZE_MASK;
> - writel(val | NFC_CTL_PAGE_SIZE(conf->page_size),
> - SUNXI_NFC_BASE + NFC_CTL);
> - writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
> - writel(conf->page_size, SUNXI_NFC_BASE + NFC_SPARE_AREA);
> + val = readl(SUNXI_NFC_BASE + NFC_REG_CTL);
> + val &= ~NFC_PAGE_SHIFT_MSK;
> + writel(val | NFC_PAGE_SIZE(conf->page_size),
> + SUNXI_NFC_BASE + NFC_REG_CTL);
> + writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_REG_CNT);
> + writel(conf->page_size, SUNXI_NFC_BASE + NFC_REG_SPARE_AREA);
> }
>
> static int nand_load_page(const struct nfc_config *conf, u32 offs)
> {
> int page = offs / conf->page_size;
>
> - writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
> - (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
> + writel((NFC_CMD_RNDOUTSTART << NFC_RND_READ_CMD1_OFFSET) |
> + (NFC_CMD_RNDOUT << NFC_RND_READ_CMD0_OFFSET) |
> (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET),
> - SUNXI_NFC_BASE + NFC_RCMD_SET);
> - writel(((page & 0xFFFF) << 16), SUNXI_NFC_BASE + NFC_ADDR_LOW);
> - writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH);
> + SUNXI_NFC_BASE + NFC_REG_RCMD_SET);
> + writel(((page & 0xFFFF) << 16), SUNXI_NFC_BASE + NFC_REG_ADDR_LOW);
> + writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_REG_ADDR_HIGH);
>
> - return nand_exec_cmd(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
> - NFC_SEND_ADDR | NFC_WAIT_FLAG |
> - ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET));
> + return nand_exec_cmd(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_NORMAL_OP |
> + NFC_SEND_ADR | NFC_WAIT_FLAG |
> + ((conf->addr_cycles - 1) << NFC_ADR_NUM_OFFSET));
> }
>
> static int nand_change_column(u16 column)
> {
> int ret;
>
> - writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) |
> - (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) |
> + writel((NFC_CMD_RNDOUTSTART << NFC_RND_READ_CMD1_OFFSET) |
> + (NFC_CMD_RNDOUT << NFC_RND_READ_CMD0_OFFSET) |
> (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET),
> - SUNXI_NFC_BASE + NFC_RCMD_SET);
> - writel(column, SUNXI_NFC_BASE + NFC_ADDR_LOW);
> + SUNXI_NFC_BASE + NFC_REG_RCMD_SET);
> + writel(column, SUNXI_NFC_BASE + NFC_REG_ADDR_LOW);
>
> - ret = nand_exec_cmd(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD |
> - (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR |
> + ret = nand_exec_cmd(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_NORMAL_OP |
> + (1 << NFC_ADR_NUM_OFFSET) | NFC_SEND_ADR |
> NFC_CMD_RNDOUT);
> if (ret)
> return ret;
> @@ -276,16 +260,16 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> u8 *data = dest + data_off;
>
> /* Clear ECC status and restart ECC engine */
> - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);
> + writel(0, SUNXI_NFC_BASE + NFC_REG_ECC_ST);
> writel((rand_seed << 16) | (conf->ecc_strength << 12) |
> - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) |
> - (conf->ecc_size == 512 ? NFC_ECC_BLOCK_SIZE : 0) |
> + (conf->randomize ? NFC_RANDOM_EN : 0) |
> + (conf->ecc_size == 512 ? NFC_ECC_BLOCK_512 : 0) |
> NFC_ECC_EN | NFC_ECC_EXCEPTION,
> - SUNXI_NFC_BASE + NFC_ECC_CTL);
> + SUNXI_NFC_BASE + NFC_REG_ECC_CTL);
>
> /* Move the data in SRAM */
> nand_change_column(data_off);
> - writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
> + writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_REG_CNT);
> nand_exec_cmd(NFC_DATA_TRANS);
>
> /*
> @@ -293,10 +277,10 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> * the data.
> */
> nand_change_column(oob_off);
> - nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_CMD);
> + nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
>
> /* Get the ECC status */
> - ecc_st = readl(SUNXI_NFC_BASE + NFC_ECC_ST);
> + ecc_st = readl(SUNXI_NFC_BASE + NFC_REG_ECC_ST);
>
> /* ECC error detected. */
> if (ecc_st & 0xffff)
> @@ -315,8 +299,8 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> conf->ecc_size);
>
> /* Stop the ECC engine */
> - writel(readl(SUNXI_NFC_BASE + NFC_ECC_CTL) & ~NFC_ECC_EN,
> - SUNXI_NFC_BASE + NFC_ECC_CTL);
> + writel(readl(SUNXI_NFC_BASE + NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
> + SUNXI_NFC_BASE + NFC_REG_ECC_CTL);
>
> if (data_off + conf->ecc_size >= len)
> break;
More information about the U-Boot
mailing list