[v3,1/5] mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2 controller
Commit Message
v7.2 controller has different ECC level field size and shift in the acc
control register than its predecessor and successor controller. It needs
to be set specifically.
Fixes: decba6d47869 ("mtd: brcmnand: Add v7.2 controller support")
Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes in v3: None
Changes in v2:
- Use driver static data for ECC level shift
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 74 +++++++++++++-----------
1 file changed, 41 insertions(+), 33 deletions(-)
Comments
Hi William,
william.zhang@broadcom.com wrote on Tue, 27 Jun 2023 12:37:34 -0700:
> v7.2 controller has different ECC level field size and shift in the acc
> control register than its predecessor and successor controller. It needs
> to be set specifically.
>
> Fixes: decba6d47869 ("mtd: brcmnand: Add v7.2 controller support")
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Use driver static data for ECC level shift
>
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 74 +++++++++++++-----------
> 1 file changed, 41 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 2e9c2e2d9c9f..69709419516a 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -272,6 +272,7 @@ struct brcmnand_controller {
> const unsigned int *page_sizes;
> unsigned int page_size_shift;
> unsigned int max_oob;
> + u32 ecc_level_shift;
> u32 features;
>
> /* for low-power standby/resume only */
> @@ -596,6 +597,34 @@ enum {
> INTFC_CTLR_READY = BIT(31),
> };
>
> +/***********************************************************************
> + * NAND ACC CONTROL bitfield
> + *
> + * Some bits have remained constant throughout hardware revision, while
> + * others have shifted around.
> + ***********************************************************************/
> +
> +/* Constant for all versions (where supported) */
> +enum {
> + /* See BRCMNAND_HAS_CACHE_MODE */
> + ACC_CONTROL_CACHE_MODE = BIT(22),
> +
> + /* See BRCMNAND_HAS_PREFETCH */
> + ACC_CONTROL_PREFETCH = BIT(23),
> +
> + ACC_CONTROL_PAGE_HIT = BIT(24),
> + ACC_CONTROL_WR_PREEMPT = BIT(25),
> + ACC_CONTROL_PARTIAL_PAGE = BIT(26),
> + ACC_CONTROL_RD_ERASED = BIT(27),
> + ACC_CONTROL_FAST_PGM_RDIN = BIT(28),
> + ACC_CONTROL_WR_ECC = BIT(30),
> + ACC_CONTROL_RD_ECC = BIT(31),
> +
> + ACC_CONTROL_ECC_SHIFT = 16,
> + /* Only for v7.2 */
> + ACC_CONTROL_ECC_EXT_SHIFT = 13,
> +};
These do not look like they fit the purpose of enums. At least keep
these two last definitions outside like before (you can rename them, I
don't mind).
LGTM otherwise.
> +
> static inline bool brcmnand_non_mmio_ops(struct brcmnand_controller *ctrl)
> {
> #if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA)
> @@ -737,6 +766,12 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
> else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
> ctrl->features |= BRCMNAND_HAS_WP;
>
> + /* v7.2 has different ecc level shift in the acc register */
> + if (ctrl->nand_version == 0x0702)
> + ctrl->ecc_level_shift = ACC_CONTROL_ECC_EXT_SHIFT;
> + else
> + ctrl->ecc_level_shift = ACC_CONTROL_ECC_SHIFT;
> +
> return 0;
> }
>
> @@ -931,30 +966,6 @@ static inline int brcmnand_cmd_shift(struct brcmnand_controller *ctrl)
> return 0;
> }
>
> -/***********************************************************************
> - * NAND ACC CONTROL bitfield
> - *
> - * Some bits have remained constant throughout hardware revision, while
> - * others have shifted around.
> - ***********************************************************************/
> -
> -/* Constant for all versions (where supported) */
> -enum {
> - /* See BRCMNAND_HAS_CACHE_MODE */
> - ACC_CONTROL_CACHE_MODE = BIT(22),
> -
> - /* See BRCMNAND_HAS_PREFETCH */
> - ACC_CONTROL_PREFETCH = BIT(23),
> -
> - ACC_CONTROL_PAGE_HIT = BIT(24),
> - ACC_CONTROL_WR_PREEMPT = BIT(25),
> - ACC_CONTROL_PARTIAL_PAGE = BIT(26),
> - ACC_CONTROL_RD_ERASED = BIT(27),
> - ACC_CONTROL_FAST_PGM_RDIN = BIT(28),
> - ACC_CONTROL_WR_ECC = BIT(30),
> - ACC_CONTROL_RD_ECC = BIT(31),
> -};
> -
> static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
> {
> if (ctrl->nand_version == 0x0702)
> @@ -967,18 +978,15 @@ static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
> return GENMASK(4, 0);
> }
>
> -#define NAND_ACC_CONTROL_ECC_SHIFT 16
> -#define NAND_ACC_CONTROL_ECC_EXT_SHIFT 13
> -
> static inline u32 brcmnand_ecc_level_mask(struct brcmnand_controller *ctrl)
> {
> u32 mask = (ctrl->nand_version >= 0x0600) ? 0x1f : 0x0f;
>
> - mask <<= NAND_ACC_CONTROL_ECC_SHIFT;
> + mask <<= ACC_CONTROL_ECC_SHIFT;
>
> /* v7.2 includes additional ECC levels */
> - if (ctrl->nand_version >= 0x0702)
> - mask |= 0x7 << NAND_ACC_CONTROL_ECC_EXT_SHIFT;
> + if (ctrl->nand_version == 0x0702)
> + mask |= 0x7 << ACC_CONTROL_ECC_EXT_SHIFT;
>
> return mask;
> }
> @@ -992,8 +1000,8 @@ static void brcmnand_set_ecc_enabled(struct brcmnand_host *host, int en)
>
> if (en) {
> acc_control |= ecc_flags; /* enable RD/WR ECC */
> - acc_control |= host->hwcfg.ecc_level
> - << NAND_ACC_CONTROL_ECC_SHIFT;
> + acc_control &= ~brcmnand_ecc_level_mask(ctrl);
> + acc_control |= host->hwcfg.ecc_level << ctrl->ecc_level_shift;
> } else {
> acc_control &= ~ecc_flags; /* disable RD/WR ECC */
> acc_control &= ~brcmnand_ecc_level_mask(ctrl);
> @@ -2561,7 +2569,7 @@ static int brcmnand_set_cfg(struct brcmnand_host *host,
> tmp &= ~brcmnand_ecc_level_mask(ctrl);
> tmp &= ~brcmnand_spare_area_mask(ctrl);
> if (ctrl->nand_version >= 0x0302) {
> - tmp |= cfg->ecc_level << NAND_ACC_CONTROL_ECC_SHIFT;
> + tmp |= cfg->ecc_level << ctrl->ecc_level_shift;
> tmp |= cfg->spare_area_size;
> }
> nand_writereg(ctrl, acc_control_offs, tmp);
Thanks,
Miquèl
@@ -272,6 +272,7 @@ struct brcmnand_controller {
const unsigned int *page_sizes;
unsigned int page_size_shift;
unsigned int max_oob;
+ u32 ecc_level_shift;
u32 features;
/* for low-power standby/resume only */
@@ -596,6 +597,34 @@ enum {
INTFC_CTLR_READY = BIT(31),
};
+/***********************************************************************
+ * NAND ACC CONTROL bitfield
+ *
+ * Some bits have remained constant throughout hardware revision, while
+ * others have shifted around.
+ ***********************************************************************/
+
+/* Constant for all versions (where supported) */
+enum {
+ /* See BRCMNAND_HAS_CACHE_MODE */
+ ACC_CONTROL_CACHE_MODE = BIT(22),
+
+ /* See BRCMNAND_HAS_PREFETCH */
+ ACC_CONTROL_PREFETCH = BIT(23),
+
+ ACC_CONTROL_PAGE_HIT = BIT(24),
+ ACC_CONTROL_WR_PREEMPT = BIT(25),
+ ACC_CONTROL_PARTIAL_PAGE = BIT(26),
+ ACC_CONTROL_RD_ERASED = BIT(27),
+ ACC_CONTROL_FAST_PGM_RDIN = BIT(28),
+ ACC_CONTROL_WR_ECC = BIT(30),
+ ACC_CONTROL_RD_ECC = BIT(31),
+
+ ACC_CONTROL_ECC_SHIFT = 16,
+ /* Only for v7.2 */
+ ACC_CONTROL_ECC_EXT_SHIFT = 13,
+};
+
static inline bool brcmnand_non_mmio_ops(struct brcmnand_controller *ctrl)
{
#if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA)
@@ -737,6 +766,12 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
ctrl->features |= BRCMNAND_HAS_WP;
+ /* v7.2 has different ecc level shift in the acc register */
+ if (ctrl->nand_version == 0x0702)
+ ctrl->ecc_level_shift = ACC_CONTROL_ECC_EXT_SHIFT;
+ else
+ ctrl->ecc_level_shift = ACC_CONTROL_ECC_SHIFT;
+
return 0;
}
@@ -931,30 +966,6 @@ static inline int brcmnand_cmd_shift(struct brcmnand_controller *ctrl)
return 0;
}
-/***********************************************************************
- * NAND ACC CONTROL bitfield
- *
- * Some bits have remained constant throughout hardware revision, while
- * others have shifted around.
- ***********************************************************************/
-
-/* Constant for all versions (where supported) */
-enum {
- /* See BRCMNAND_HAS_CACHE_MODE */
- ACC_CONTROL_CACHE_MODE = BIT(22),
-
- /* See BRCMNAND_HAS_PREFETCH */
- ACC_CONTROL_PREFETCH = BIT(23),
-
- ACC_CONTROL_PAGE_HIT = BIT(24),
- ACC_CONTROL_WR_PREEMPT = BIT(25),
- ACC_CONTROL_PARTIAL_PAGE = BIT(26),
- ACC_CONTROL_RD_ERASED = BIT(27),
- ACC_CONTROL_FAST_PGM_RDIN = BIT(28),
- ACC_CONTROL_WR_ECC = BIT(30),
- ACC_CONTROL_RD_ECC = BIT(31),
-};
-
static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
{
if (ctrl->nand_version == 0x0702)
@@ -967,18 +978,15 @@ static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
return GENMASK(4, 0);
}
-#define NAND_ACC_CONTROL_ECC_SHIFT 16
-#define NAND_ACC_CONTROL_ECC_EXT_SHIFT 13
-
static inline u32 brcmnand_ecc_level_mask(struct brcmnand_controller *ctrl)
{
u32 mask = (ctrl->nand_version >= 0x0600) ? 0x1f : 0x0f;
- mask <<= NAND_ACC_CONTROL_ECC_SHIFT;
+ mask <<= ACC_CONTROL_ECC_SHIFT;
/* v7.2 includes additional ECC levels */
- if (ctrl->nand_version >= 0x0702)
- mask |= 0x7 << NAND_ACC_CONTROL_ECC_EXT_SHIFT;
+ if (ctrl->nand_version == 0x0702)
+ mask |= 0x7 << ACC_CONTROL_ECC_EXT_SHIFT;
return mask;
}
@@ -992,8 +1000,8 @@ static void brcmnand_set_ecc_enabled(struct brcmnand_host *host, int en)
if (en) {
acc_control |= ecc_flags; /* enable RD/WR ECC */
- acc_control |= host->hwcfg.ecc_level
- << NAND_ACC_CONTROL_ECC_SHIFT;
+ acc_control &= ~brcmnand_ecc_level_mask(ctrl);
+ acc_control |= host->hwcfg.ecc_level << ctrl->ecc_level_shift;
} else {
acc_control &= ~ecc_flags; /* disable RD/WR ECC */
acc_control &= ~brcmnand_ecc_level_mask(ctrl);
@@ -2561,7 +2569,7 @@ static int brcmnand_set_cfg(struct brcmnand_host *host,
tmp &= ~brcmnand_ecc_level_mask(ctrl);
tmp &= ~brcmnand_spare_area_mask(ctrl);
if (ctrl->nand_version >= 0x0302) {
- tmp |= cfg->ecc_level << NAND_ACC_CONTROL_ECC_SHIFT;
+ tmp |= cfg->ecc_level << ctrl->ecc_level_shift;
tmp |= cfg->spare_area_size;
}
nand_writereg(ctrl, acc_control_offs, tmp);