| Message ID | 1559251257-12383-1-git-send-email-kdasu.kdev@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | [1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions | expand |
On Thu, 30 May 2019 17:20:35 -0400 Kamal Dasu <kdasu.kdev@gmail.com> wrote: > Refactored NAND ECC and CMD address configuration code to use inline > functions. I'd expect the compiler to be smart enough to decide when inlining is appropriate. Did you check that adding the inline specifier actually makes a difference? > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------ > 1 file changed, 62 insertions(+), 38 deletions(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index ce0b8ff..77b7850 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl, > __raw_writel(val, ctrl->nand_fc + word * 4); > } > > +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl) > +{ > + > + /* Clear error addresses */ > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); > +} > + > +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl) > +{ > + u64 err_addr; > + > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > + BRCMNAND_UNCORR_EXT_ADDR) > + & 0xffff) << 32); > + > + return err_addr; > +} > + > +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl) > +{ > + u64 err_addr; > + > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > + BRCMNAND_CORR_EXT_ADDR) > + & 0xffff) << 32); > + > + return err_addr; > +} > + > +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct brcmnand_host *host = nand_get_controller_data(chip); > + struct brcmnand_controller *ctrl = host->ctrl; > + > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > + (host->cs << 16) | ((addr >> 32) & 0xffff)); > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > + lower_32_bits(addr)); > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > +} > + > static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs, > enum brcmnand_cs_reg reg) > { > @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd) > { > struct brcmnand_controller *ctrl = host->ctrl; > int ret; > + u64 cmd_addr; > + > + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > + > + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); > > - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, > - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); > BUG_ON(ctrl->cmd_pending != 0); > ctrl->cmd_pending = cmd; > > @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command, > if (!native_cmd) > return; > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > - > + brcmnand_set_cmd_addr(mtd, addr); > brcmnand_send_cmd(host, native_cmd); > brcmnand_waitfunc(chip); > > @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, > struct brcmnand_controller *ctrl = host->ctrl; > int i, j, ret = 0; > > - /* Clear error addresses */ > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); > - brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); > - brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); > - > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > + brcmnand_clear_ecc_addr(ctrl); > > for (i = 0; i < trans; i++, addr += FC_BYTES) { > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > - lower_32_bits(addr)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > + brcmnand_set_cmd_addr(mtd, addr); > /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */ > brcmnand_send_cmd(host, CMD_PAGE_READ); > brcmnand_waitfunc(chip); > @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, > host->hwcfg.sector_size_1k); > > if (!ret) { > - *err_addr = brcmnand_read_reg(ctrl, > - BRCMNAND_UNCORR_ADDR) | > - ((u64)(brcmnand_read_reg(ctrl, > - BRCMNAND_UNCORR_EXT_ADDR) > - & 0xffff) << 32); > + *err_addr = brcmnand_get_uncorrecc_addr(ctrl); > + > if (*err_addr) > ret = -EBADMSG; > } > > if (!ret) { > - *err_addr = brcmnand_read_reg(ctrl, > - BRCMNAND_CORR_ADDR) | > - ((u64)(brcmnand_read_reg(ctrl, > - BRCMNAND_CORR_EXT_ADDR) > - & 0xffff) << 32); > + *err_addr = brcmnand_get_correcc_addr(ctrl); > + > if (*err_addr) > ret = -EUCLEAN; > } > @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, > dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf); > > try_dmaread: > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0); > + brcmnand_clear_ecc_addr(ctrl); > > if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) { > err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES, > @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip, > goto out; > } > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > - > for (i = 0; i < trans; i++, addr += FC_BYTES) { > /* full address MUST be set before populating FC */ > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > - lower_32_bits(addr)); > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > + brcmnand_set_cmd_addr(mtd, addr); > > if (buf) { > brcmnand_soc_data_bus_prepare(ctrl->soc, false);
Boris, On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon <boris.brezillon@collabora.com> wrote: > > On Thu, 30 May 2019 17:20:35 -0400 > Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > Refactored NAND ECC and CMD address configuration code to use inline > > functions. > > I'd expect the compiler to be smart enough to decide when inlining is > appropriate. Did you check that adding the inline specifier actually > makes a difference? This was done to make the code more readable. It does not make any difference to performance. > > > > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------ > > 1 file changed, 62 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index ce0b8ff..77b7850 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl, > > __raw_writel(val, ctrl->nand_fc + word * 4); > > } > > > > +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl) > > +{ > > + > > + /* Clear error addresses */ > > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); > > +} > > + > > +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl) > > +{ > > + u64 err_addr; > > + > > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); > > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > > + BRCMNAND_UNCORR_EXT_ADDR) > > + & 0xffff) << 32); > > + > > + return err_addr; > > +} > > + > > +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl) > > +{ > > + u64 err_addr; > > + > > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); > > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > > + BRCMNAND_CORR_EXT_ADDR) > > + & 0xffff) << 32); > > + > > + return err_addr; > > +} > > + > > +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) > > +{ > > + struct nand_chip *chip = mtd_to_nand(mtd); > > + struct brcmnand_host *host = nand_get_controller_data(chip); > > + struct brcmnand_controller *ctrl = host->ctrl; > > + > > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > + (host->cs << 16) | ((addr >> 32) & 0xffff)); > > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > > + lower_32_bits(addr)); > > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > +} > > + > > static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs, > > enum brcmnand_cs_reg reg) > > { > > @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd) > > { > > struct brcmnand_controller *ctrl = host->ctrl; > > int ret; > > + u64 cmd_addr; > > + > > + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > + > > + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); > > > > - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, > > - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); > > BUG_ON(ctrl->cmd_pending != 0); > > ctrl->cmd_pending = cmd; > > > > @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command, > > if (!native_cmd) > > return; > > > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > - > > + brcmnand_set_cmd_addr(mtd, addr); > > brcmnand_send_cmd(host, native_cmd); > > brcmnand_waitfunc(chip); > > > > @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, > > struct brcmnand_controller *ctrl = host->ctrl; > > int i, j, ret = 0; > > > > - /* Clear error addresses */ > > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); > > - brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); > > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); > > - brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); > > - > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > + brcmnand_clear_ecc_addr(ctrl); > > > > for (i = 0; i < trans; i++, addr += FC_BYTES) { > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > > - lower_32_bits(addr)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > + brcmnand_set_cmd_addr(mtd, addr); > > /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */ > > brcmnand_send_cmd(host, CMD_PAGE_READ); > > brcmnand_waitfunc(chip); > > @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, > > host->hwcfg.sector_size_1k); > > > > if (!ret) { > > - *err_addr = brcmnand_read_reg(ctrl, > > - BRCMNAND_UNCORR_ADDR) | > > - ((u64)(brcmnand_read_reg(ctrl, > > - BRCMNAND_UNCORR_EXT_ADDR) > > - & 0xffff) << 32); > > + *err_addr = brcmnand_get_uncorrecc_addr(ctrl); > > + > > if (*err_addr) > > ret = -EBADMSG; > > } > > > > if (!ret) { > > - *err_addr = brcmnand_read_reg(ctrl, > > - BRCMNAND_CORR_ADDR) | > > - ((u64)(brcmnand_read_reg(ctrl, > > - BRCMNAND_CORR_EXT_ADDR) > > - & 0xffff) << 32); > > + *err_addr = brcmnand_get_correcc_addr(ctrl); > > + > > if (*err_addr) > > ret = -EUCLEAN; > > } > > @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, > > dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf); > > > > try_dmaread: > > - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0); > > + brcmnand_clear_ecc_addr(ctrl); > > > > if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) { > > err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES, > > @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip, > > goto out; > > } > > > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > - (host->cs << 16) | ((addr >> 32) & 0xffff)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > - > > for (i = 0; i < trans; i++, addr += FC_BYTES) { > > /* full address MUST be set before populating FC */ > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > > - lower_32_bits(addr)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > + brcmnand_set_cmd_addr(mtd, addr); > > > > if (buf) { > > brcmnand_soc_data_bus_prepare(ctrl->soc, false); > Thanks Kamal
On Mon, 3 Jun 2019 10:11:20 -0400 Kamal Dasu <kdasu.kdev@gmail.com> wrote: > Boris, > > On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon > <boris.brezillon@collabora.com> wrote: > > > > On Thu, 30 May 2019 17:20:35 -0400 > > Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > > > Refactored NAND ECC and CMD address configuration code to use inline > > > functions. > > > > I'd expect the compiler to be smart enough to decide when inlining is > > appropriate. Did you check that adding the inline specifier actually > > makes a difference? > > This was done to make the code more readable. It does not make any > difference to performance. I meant dropping the inline specifier, not going back to manual inlining. As a general rule, you don't need to add the 'inline' specifier unless your function is defined in a header. In all other cases the compiler is able to inline things on its own when it sees the number of instructions is small enough or when the function is only called once.
On Mon, Jun 3, 2019 at 10:18 AM Boris Brezillon <boris.brezillon@collabora.com> wrote: > > On Mon, 3 Jun 2019 10:11:20 -0400 > Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > Boris, > > > > On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon > > <boris.brezillon@collabora.com> wrote: > > > > > > On Thu, 30 May 2019 17:20:35 -0400 > > > Kamal Dasu <kdasu.kdev@gmail.com> wrote: > > > > > > > Refactored NAND ECC and CMD address configuration code to use inline > > > > functions. > > > > > > I'd expect the compiler to be smart enough to decide when inlining is > > > appropriate. Did you check that adding the inline specifier actually > > > makes a difference? > > > > This was done to make the code more readable. It does not make any > > difference to performance. > > I meant dropping the inline specifier, not going back to manual > inlining. As a general rule, you don't need to add the 'inline' > specifier unless your function is defined in a header. In all other > cases the compiler is able to inline things on its own when it sees the > number of instructions is small enough or when the function is only > called once. Oh ok got it, will get rid if the inline specifier and send a v2 version of the change. Thanks Kamal
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..77b7850 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl, __raw_writel(val, ctrl->nand_fc + word * 4); } +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl) +{ + + /* Clear error addresses */ + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); +} + +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, + BRCMNAND_UNCORR_EXT_ADDR) + & 0xffff) << 32); + + return err_addr; +} + +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, + BRCMNAND_CORR_EXT_ADDR) + & 0xffff) << 32); + + return err_addr; +} + +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, + (host->cs << 16) | ((addr >> 32) & 0xffff)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, + lower_32_bits(addr)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); +} + static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs, enum brcmnand_cs_reg reg) { @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd) { struct brcmnand_controller *ctrl = host->ctrl; int ret; + u64 cmd_addr; + + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); BUG_ON(ctrl->cmd_pending != 0); ctrl->cmd_pending = cmd; @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command, if (!native_cmd) return; - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0xffff)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); - + brcmnand_set_cmd_addr(mtd, addr); brcmnand_send_cmd(host, native_cmd); brcmnand_waitfunc(chip); @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, struct brcmnand_controller *ctrl = host->ctrl; int i, j, ret = 0; - /* Clear error addresses */ - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); - - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0xffff)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_clear_ecc_addr(ctrl); for (i = 0; i < trans; i++, addr += FC_BYTES) { - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, - lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + brcmnand_set_cmd_addr(mtd, addr); /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */ brcmnand_send_cmd(host, CMD_PAGE_READ); brcmnand_waitfunc(chip); @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, host->hwcfg.sector_size_1k); if (!ret) { - *err_addr = brcmnand_read_reg(ctrl, - BRCMNAND_UNCORR_ADDR) | - ((u64)(brcmnand_read_reg(ctrl, - BRCMNAND_UNCORR_EXT_ADDR) - & 0xffff) << 32); + *err_addr = brcmnand_get_uncorrecc_addr(ctrl); + if (*err_addr) ret = -EBADMSG; } if (!ret) { - *err_addr = brcmnand_read_reg(ctrl, - BRCMNAND_CORR_ADDR) | - ((u64)(brcmnand_read_reg(ctrl, - BRCMNAND_CORR_EXT_ADDR) - & 0xffff) << 32); + *err_addr = brcmnand_get_correcc_addr(ctrl); + if (*err_addr) ret = -EUCLEAN; } @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf); try_dmaread: - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0); + brcmnand_clear_ecc_addr(ctrl); if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) { err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES, @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip, goto out; } - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0xffff)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); - for (i = 0; i < trans; i++, addr += FC_BYTES) { /* full address MUST be set before populating FC */ - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, - lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + brcmnand_set_cmd_addr(mtd, addr); if (buf) { brcmnand_soc_data_bus_prepare(ctrl->soc, false);
Refactored NAND ECC and CMD address configuration code to use inline functions. Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------ 1 file changed, 62 insertions(+), 38 deletions(-)