Messages in this thread | | | Date | Mon, 2 Jan 2023 10:33:28 -0600 | Subject | Re: [PATCH 7/7] mtd: rawnand: sunxi: Precompute the ECC_CTL register value | From | Samuel Holland <> |
| |
Hi Miquèl,
On 1/2/23 03:30, Miquel Raynal wrote: > Hi Samuel, > > samuel@sholland.org wrote on Thu, 29 Dec 2022 12:15:26 -0600: > >> This removes an unnecessary memory allocation, and avoids recomputing >> the same register value every time ECC is enabled. > > I am fine with the "let's not recompute the register value each time" > idea, but I like having a dedicated object reserved for the ECC > engine, that is separated from the main controller structure (both > are two different things, even though they are usually well > integrated). > > If it's actually useless you can still get rid of the allocation and in > the structure you can save the ecc_ctrl reg value instead of mode.
OK, I will do this, and split it into two patches: one for replacing mode with ecc_ctrl, and the second to keep the structure but get rid of the allocation.
Regards, Samuel
> The other patches in the series look good to me. > > Thanks, > Miquèl > >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> drivers/mtd/nand/raw/sunxi_nand.c | 75 ++++++------------------------- >> 1 file changed, 13 insertions(+), 62 deletions(-) >> >> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c >> index a3bc9f7f9e5a..5c5a567d8870 100644 >> --- a/drivers/mtd/nand/raw/sunxi_nand.c >> +++ b/drivers/mtd/nand/raw/sunxi_nand.c >> @@ -169,22 +169,13 @@ struct sunxi_nand_chip_sel { >> s8 rb; >> }; >> >> -/** >> - * struct sunxi_nand_hw_ecc - stores information related to HW ECC support >> - * >> - * @mode: the sunxi ECC mode field deduced from ECC requirements >> - */ >> -struct sunxi_nand_hw_ecc { >> - int mode; >> -}; >> - >> /** >> * struct sunxi_nand_chip - stores NAND chip device related information >> * >> * @node: used to store NAND chips into a list >> * @nand: base NAND chip structure >> - * @ecc: ECC controller structure >> * @clk_rate: clk_rate required for this NAND chip >> + * @ecc_ctl: ECC_CTL register value for this NAND chip >> * @timing_cfg: TIMING_CFG register value for this NAND chip >> * @timing_ctl: TIMING_CTL register value for this NAND chip >> * @nsels: number of CS lines required by the NAND chip >> @@ -193,8 +184,8 @@ struct sunxi_nand_hw_ecc { >> struct sunxi_nand_chip { >> struct list_head node; >> struct nand_chip nand; >> - struct sunxi_nand_hw_ecc *ecc; >> unsigned long clk_rate; >> + u32 ecc_ctl; >> u32 timing_cfg; >> u32 timing_ctl; >> int nsels; >> @@ -689,26 +680,15 @@ static void sunxi_nfc_hw_ecc_enable(struct nand_chip *nand) >> { >> struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand); >> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller); >> - u32 ecc_ctl; >> - >> - ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL); >> - ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE | >> - NFC_ECC_BLOCK_SIZE_MSK); >> - ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(sunxi_nand->ecc->mode) | >> - NFC_ECC_EXCEPTION | NFC_ECC_PIPELINE; >> - >> - if (nand->ecc.size == 512) >> - ecc_ctl |= NFC_ECC_BLOCK_512; >> >> - writel(ecc_ctl, nfc->regs + NFC_REG_ECC_CTL); >> + writel(sunxi_nand->ecc_ctl, nfc->regs + NFC_REG_ECC_CTL); >> } >> >> static void sunxi_nfc_hw_ecc_disable(struct nand_chip *nand) >> { >> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller); >> >> - writel(readl(nfc->regs + NFC_REG_ECC_CTL) & ~NFC_ECC_EN, >> - nfc->regs + NFC_REG_ECC_CTL); >> + writel(0, nfc->regs + NFC_REG_ECC_CTL); >> } >> >> static inline void sunxi_nfc_user_data_to_buf(u32 user_data, u8 *buf) >> @@ -1626,11 +1606,6 @@ static const struct mtd_ooblayout_ops sunxi_nand_ooblayout_ops = { >> .free = sunxi_nand_ooblayout_free, >> }; >> >> -static void sunxi_nand_hw_ecc_ctrl_cleanup(struct sunxi_nand_chip *sunxi_nand) >> -{ >> - kfree(sunxi_nand->ecc); >> -} >> - >> static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> struct nand_ecc_ctrl *ecc, >> struct device_node *np) >> @@ -1641,7 +1616,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> struct mtd_info *mtd = nand_to_mtd(nand); >> struct nand_device *nanddev = mtd_to_nanddev(mtd); >> int nsectors; >> - int ret; >> int i; >> >> if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) { >> @@ -1675,10 +1649,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> if (ecc->size != 512 && ecc->size != 1024) >> return -EINVAL; >> >> - sunxi_nand->ecc = kzalloc(sizeof(*sunxi_nand->ecc), GFP_KERNEL); >> - if (!sunxi_nand->ecc) >> - return -ENOMEM; >> - >> /* Prefer 1k ECC chunk over 512 ones */ >> if (ecc->size == 512 && mtd->writesize > 512) { >> ecc->size = 1024; >> @@ -1699,12 +1669,9 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> >> if (i >= ARRAY_SIZE(strengths)) { >> dev_err(nfc->dev, "unsupported strength\n"); >> - ret = -ENOTSUPP; >> - goto err; >> + return -ENOTSUPP; >> } >> >> - sunxi_nand->ecc->mode = i; >> - >> /* HW ECC always request ECC bytes for 1024 bytes blocks */ >> ecc->bytes = DIV_ROUND_UP(ecc->strength * fls(8 * 1024), 8); >> >> @@ -1713,10 +1680,14 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> >> nsectors = mtd->writesize / ecc->size; >> >> - if (mtd->oobsize < ((ecc->bytes + 4) * nsectors)) { >> - ret = -EINVAL; >> - goto err; >> - } >> + if (mtd->oobsize < ((ecc->bytes + 4) * nsectors)) >> + return -EINVAL; >> + >> + sunxi_nand->ecc_ctl = NFC_ECC_MODE(i) | NFC_ECC_EXCEPTION | >> + NFC_ECC_PIPELINE | NFC_ECC_EN; >> + >> + if (ecc->size == 512) >> + sunxi_nand->ecc_ctl |= NFC_ECC_BLOCK_512; >> >> ecc->read_oob = sunxi_nfc_hw_ecc_read_oob; >> ecc->write_oob = sunxi_nfc_hw_ecc_write_oob; >> @@ -1739,25 +1710,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand, >> ecc->write_oob_raw = nand_write_oob_std; >> >> return 0; >> - >> -err: >> - kfree(sunxi_nand->ecc); >> - >> - return ret; >> -} >> - >> -static void sunxi_nand_ecc_cleanup(struct sunxi_nand_chip *sunxi_nand) >> -{ >> - struct nand_ecc_ctrl *ecc = &sunxi_nand->nand.ecc; >> - >> - switch (ecc->engine_type) { >> - case NAND_ECC_ENGINE_TYPE_ON_HOST: >> - sunxi_nand_hw_ecc_ctrl_cleanup(sunxi_nand); >> - break; >> - case NAND_ECC_ENGINE_TYPE_NONE: >> - default: >> - break; >> - } >> } >> >> static int sunxi_nand_attach_chip(struct nand_chip *nand) >> @@ -1970,7 +1922,6 @@ static void sunxi_nand_chips_cleanup(struct sunxi_nfc *nfc) >> ret = mtd_device_unregister(nand_to_mtd(chip)); >> WARN_ON(ret); >> nand_cleanup(chip); >> - sunxi_nand_ecc_cleanup(sunxi_nand); >> list_del(&sunxi_nand->node); >> } >> }
| |