[PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[Christian Marangi]

With some research in some obscure old QSDK, it was possible to find the
MASK of the last register there were still set with raw shift and
convert them to FIELD_PREP API.

This is only a cleanup and modernize the code a bit and doesn't make
any behaviour change.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/mtd/nand/raw/qcom_nandc.c    | 36 ++++++++++++++--------------
 include/linux/mtd/nand-qpic-common.h |  6 ++++-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 6720b547892b..5eaa0be367cd 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -165,9 +165,9 @@ static void nandc_set_read_loc_first(struct nand_chip *chip,
 {
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
 	__le32 locreg_val;
-	u32 val = (((cw_offset) << READ_LOCATION_OFFSET) |
-		  ((read_size) << READ_LOCATION_SIZE) |
-		  ((is_last_read_loc) << READ_LOCATION_LAST));
+	u32 val = FIELD_PREP(READ_LOCATION_OFFSET_MASK, cw_offset) |
+		  FIELD_PREP(READ_LOCATION_SIZE_MASK, read_size) |
+		  FIELD_PREP(READ_LOCATION_LAST_MASK, is_last_read_loc);
 
 	locreg_val = cpu_to_le32(val);
 
@@ -197,9 +197,9 @@ static void nandc_set_read_loc_last(struct nand_chip *chip,
 {
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
 	__le32 locreg_val;
-	u32 val = (((cw_offset) << READ_LOCATION_OFFSET) |
-		  ((read_size) << READ_LOCATION_SIZE) |
-		  ((is_last_read_loc) << READ_LOCATION_LAST));
+	u32 val = FIELD_PREP(READ_LOCATION_OFFSET_MASK, cw_offset) |
+		  FIELD_PREP(READ_LOCATION_SIZE_MASK, read_size) |
+		  FIELD_PREP(READ_LOCATION_LAST_MASK, is_last_read_loc);
 
 	locreg_val = cpu_to_le32(val);
 
@@ -271,14 +271,14 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read, i
 	}
 
 	if (host->use_ecc) {
-		cfg0 = cpu_to_le32((host->cfg0 & ~(7U << CW_PER_PAGE)) |
-				(num_cw - 1) << CW_PER_PAGE);
+		cfg0 = cpu_to_le32((host->cfg0 & ~CW_PER_PAGE_MASK) |
+				   FIELD_PREP(CW_PER_PAGE_MASK, (num_cw - 1)));
 
 		cfg1 = cpu_to_le32(host->cfg1);
 		ecc_bch_cfg = cpu_to_le32(host->ecc_bch_cfg);
 	} else {
-		cfg0 = cpu_to_le32((host->cfg0_raw & ~(7U << CW_PER_PAGE)) |
-				(num_cw - 1) << CW_PER_PAGE);
+		cfg0 = cpu_to_le32((host->cfg0_raw & ~CW_PER_PAGE_MASK) |
+				   FIELD_PREP(CW_PER_PAGE_MASK, (num_cw - 1)));
 
 		cfg1 = cpu_to_le32(host->cfg1_raw);
 		ecc_bch_cfg = cpu_to_le32(ECC_CFG_ECC_DISABLE);
@@ -882,12 +882,12 @@ static void qcom_nandc_codeword_fixup(struct qcom_nand_host *host, int page)
 			    host->bbm_size - host->cw_data;
 
 	host->cfg0 &= ~(SPARE_SIZE_BYTES_MASK | UD_SIZE_BYTES_MASK);
-	host->cfg0 |= host->spare_bytes << SPARE_SIZE_BYTES |
-		      host->cw_data << UD_SIZE_BYTES;
+	host->cfg0 |= FIELD_PREP(SPARE_SIZE_BYTES_MASK, host->spare_bytes) |
+		      FIELD_PREP(UD_SIZE_BYTES_MASK, host->cw_data);
 
 	host->ecc_bch_cfg &= ~ECC_NUM_DATA_BYTES_MASK;
-	host->ecc_bch_cfg |= host->cw_data << ECC_NUM_DATA_BYTES;
-	host->ecc_buf_cfg = (host->cw_data - 1) << NUM_STEPS;
+	host->ecc_bch_cfg |= FIELD_PREP(ECC_NUM_DATA_BYTES_MASK, host->cw_data);
+	host->ecc_buf_cfg = FIELD_PREP(NUM_STEPS_MASK, host->cw_data - 1);
 }
 
 /* implements ecc->read_page() */
@@ -1531,7 +1531,7 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 			    FIELD_PREP(ECC_PARITY_SIZE_BYTES_BCH_MASK, host->ecc_bytes_hw);
 
 	if (!nandc->props->qpic_version2)
-		host->ecc_buf_cfg = 0x203 << NUM_STEPS;
+		host->ecc_buf_cfg = FIELD_PREP(NUM_STEPS_MASK, 0x203);
 
 	host->clrflashstatus = FS_READY_BSY_N;
 	host->clrreadstatus = 0xc0;
@@ -1817,7 +1817,7 @@ static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_sub
 		q_op.cmd_reg |= cpu_to_le32(PAGE_ACC | LAST_PAGE);
 		nandc->regs->addr0 = q_op.addr1_reg;
 		nandc->regs->addr1 = q_op.addr2_reg;
-		nandc->regs->cfg0 = cpu_to_le32(host->cfg0_raw & ~(7 << CW_PER_PAGE));
+		nandc->regs->cfg0 = cpu_to_le32(host->cfg0_raw & ~CW_PER_PAGE_MASK);
 		nandc->regs->cfg1 = cpu_to_le32(host->cfg1_raw);
 		instrs = 3;
 	} else if (q_op.cmd_reg != cpu_to_le32(OP_RESET_DEVICE)) {
@@ -1900,8 +1900,8 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	/* configure CMD1 and VLD for ONFI param probing in QPIC v1 */
 	if (!nandc->props->qpic_version2) {
 		nandc->regs->vld = cpu_to_le32((nandc->vld & ~READ_START_VLD));
-		nandc->regs->cmd1 = cpu_to_le32((nandc->cmd1 & ~(0xFF << READ_ADDR))
-				    | NAND_CMD_PARAM << READ_ADDR);
+		nandc->regs->cmd1 = cpu_to_le32((nandc->cmd1 & ~READ_ADDR_MASK) |
+						FIELD_PREP(READ_ADDR_MASK, NAND_CMD_PARAM));
 	}
 
 	nandc->regs->exec = cpu_to_le32(1);
diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
index 4d9b736ff8b7..35e7ee0f7809 100644
--- a/include/linux/mtd/nand-qpic-common.h
+++ b/include/linux/mtd/nand-qpic-common.h
@@ -108,7 +108,7 @@
 #define	ECC_FORCE_CLK_OPEN		BIT(30)
 
 /* NAND_DEV_CMD1 bits */
-#define	READ_ADDR			0
+#define	READ_ADDR_MASK			GENMASK(7, 0)
 
 /* NAND_DEV_CMD_VLD bits */
 #define	READ_START_VLD			BIT(0)
@@ -119,6 +119,7 @@
 
 /* NAND_EBI2_ECC_BUF_CFG bits */
 #define	NUM_STEPS			0
+#define	NUM_STEPS_MASK			GENMASK(9, 0)
 
 /* NAND_ERASED_CW_DETECT_CFG bits */
 #define	ERASED_CW_ECC_MASK		1
@@ -139,8 +140,11 @@
 
 /* NAND_READ_LOCATION_n bits */
 #define READ_LOCATION_OFFSET		0
+#define READ_LOCATION_OFFSET_MASK	GENMASK(9, 0)
 #define READ_LOCATION_SIZE		16
+#define READ_LOCATION_SIZE_MASK		GENMASK(25, 16)
 #define READ_LOCATION_LAST		31
+#define READ_LOCATION_LAST_MASK		BIT(31)
 
 /* Version Mask */
 #define	NAND_VERSION_MAJOR_MASK		0xf0000000
-- 
2.47.1

Re: [PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[Miquel Raynal]

Hello Christian,

On 09/02/2025 at 15:54:32 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:

> With some research in some obscure old QSDK, it was possible to find the
> MASK of the last register there were still set with raw shift and
> convert them to FIELD_PREP API.
>
> This is only a cleanup and modernize the code a bit and doesn't make
> any behaviour change.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c    | 36 ++++++++++++++--------------

I'm fine with your two patches. I was about to apply them, but the first
one needs to go through fixes, whereas the second through next, and they
are dependent on each other. I propose the following modification:
- create patch 1/2 with the content of the cleanup done just below, but
  only adapted to the very specific spot that is touched by the fix "fix
  broken config...". It would be a prerequisite for the fix.
- patch 2/2 would be the content of "fix broken config..."

And aside, a totally independent patch easy to apply on -rc1 with the
rest of this patch.

Would that work for you?

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[Christian Marangi]

On Mon, Feb 10, 2025 at 04:15:38PM +0100, Miquel Raynal wrote:
> Hello Christian,
> 
> On 09/02/2025 at 15:54:32 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:
> 
> > With some research in some obscure old QSDK, it was possible to find the
> > MASK of the last register there were still set with raw shift and
> > convert them to FIELD_PREP API.
> >
> > This is only a cleanup and modernize the code a bit and doesn't make
> > any behaviour change.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/qcom_nandc.c    | 36 ++++++++++++++--------------
> 
> I'm fine with your two patches. I was about to apply them, but the first
> one needs to go through fixes, whereas the second through next, and they
> are dependent on each other. I propose the following modification:
> - create patch 1/2 with the content of the cleanup done just below, but
>   only adapted to the very specific spot that is touched by the fix "fix
>   broken config...". It would be a prerequisite for the fix.
> - patch 2/2 would be the content of "fix broken config..."
> 
> And aside, a totally independent patch easy to apply on -rc1 with the
> rest of this patch.
> 
> Would that work for you?
>

Mhhh are they really dependent on each other?

I posted them in 2 separate patch as one should have priority and be
applied ASAP. The other is really a cleanup and from what I can see no
delta in the patch gets affected by the fix in the other patch.

In theory they should apply independently.

An alternative solution might be to just delay the cleanup patch and
post/merge it later in some week? Open to any suggetion to better handle
this but I feel they don't conflict on each other (please confirm if I'm
wrong about this)

-- 
	Ansuel

Re: [PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[Miquel Raynal]

Hi Christian,

>> I'm fine with your two patches. I was about to apply them, but the first
>> one needs to go through fixes, whereas the second through next, and they
>> are dependent on each other. I propose the following modification:
>> - create patch 1/2 with the content of the cleanup done just below, but
>>   only adapted to the very specific spot that is touched by the fix "fix
>>   broken config...". It would be a prerequisite for the fix.
>> - patch 2/2 would be the content of "fix broken config..."
>> 
>> And aside, a totally independent patch easy to apply on -rc1 with the
>> rest of this patch.
>> 
>> Would that work for you?
>>
>
> Mhhh are they really dependent on each other?
>
> I posted them in 2 separate patch as one should have priority and be
> applied ASAP. The other is really a cleanup and from what I can see no
> delta in the patch gets affected by the fix in the other patch.
>
> In theory they should apply independently.

Ah ok, I must have misread them, fine then.

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: qcom: finish converting register to FIELD_PREP

[Miquel Raynal]

On Sun, 09 Feb 2025 15:54:32 +0100, Christian Marangi wrote:
> With some research in some obscure old QSDK, it was possible to find the
> MASK of the last register there were still set with raw shift and
> convert them to FIELD_PREP API.
> 
> This is only a cleanup and modernize the code a bit and doesn't make
> any behaviour change.
> 
> [...]

Applied to nand/next, thanks!

[1/1] mtd: rawnand: qcom: finish converting register to FIELD_PREP
      commit: 1db50b96b059ca8e5548cb3e0e38a888b325f96b

Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl