drivers/mtd/nand/raw/qcom_nandc.c | 36 ++++++++++++++-------------- include/linux/mtd/nand-qpic-common.h | 6 ++++- 2 files changed, 23 insertions(+), 19 deletions(-)
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.1On 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
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
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
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
© 2016 - 2025 Red Hat, Inc.