Hi Miquel, > -----Original Message----- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Friday, June 8, 2018 6:06 PM > To: Naga Sureshkumar Relli <nagas...@xilinx.com> > Cc: boris.brezil...@bootlin.com; rich...@nod.at; w...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; f.faine...@gmail.com; > mma...@broadcom.com; rog...@ti.com; la...@linux-mips.org; a...@thorsis.com; > honghui.zh...@mediatek.com; linux-...@lists.infradead.org; > linux-kernel@vger.kernel.org; > nagasureshkumarre...@gmail.com > Subject: Re: [LINUX PATCH v9 4/4] mtd: rawnand: pl353: Add basic driver for > arm pl353 > smc nand interface > > Hi Naga, > > > > > + ecc->read_page = pl353_nand_read_page_hwecc; > > > > + ecc->size = PL353_NAND_ECC_SIZE; > > > > + ecc->write_page = pl353_nand_write_page_hwecc; > > > > + pl353_smc_set_ecc_pg_size(mtd->writesize); > > > > + switch (mtd->writesize) { > > > > + case SZ_512: > > > > + case SZ_1K: > > > > + case SZ_2K: > > > > + pl353_smc_set_ecc_mode(PL353_SMC_ECCMODE_APB); > > > > + break; > > > > + default: > > > > + /* > > > > + * The software ECC routines won't work with the > > > > + * SMC controller > > > > + */ > > > > + ecc->calculate = nand_calculate_ecc; > > > > + ecc->correct = nand_correct_data; > > > > + ecc->size = 256; > > > > + break; > > > > + } > > > > + if (mtd->writesize <= SZ_512) > > > > + xnand->addr_cycles = 1; > > > > + else > > > > + xnand->addr_cycles = 2; > > > > + > > > > + if (chip->options & NAND_ROW_ADDR_3) > > > > + xnand->addr_cycles += 3; > > > > + else > > > > + xnand->addr_cycles += 2; > > > > + > > > > + if (mtd->oobsize == 16) > > > > + mtd_set_ooblayout(mtd, > > > > &pl353_ecc_ooblayout16_ops); > > > > + else if (mtd->oobsize == 64) > > > > + mtd_set_ooblayout(mtd, > > > > &pl353_ecc_ooblayout64_ops); > > > > > > else? > > You mean to say, add an error condition? > > I do. Ok, I will add it in next version. > > > > > > > > + } > > > > +} > > > > + > > > > +/** > > > > + * pl353_nand_probe - Probe method for the NAND driver > > > > + * @pdev: Pointer to the platform_device structure > > > > + * > > > > + * This function initializes the driver data structures and the > > > > hardware. > > > > + * > > > > + * Return: 0 on success or error value on failure > > > > + */ > > > > +static int pl353_nand_probe(struct platform_device *pdev) { > > > > + struct pl353_nand_info *xnand; > > > > > > xnand is a strange name, more and more because its a bout NAND > > > controller data, not NAND chip. > > We added this name to represent Xilinx Nand(xnand), > > > > > I see where the x comes from. > > Maybe just nfc (for NAND flash controller) or xnfc if you prefer. What > I want is to clearly make the distinction between what is a NAND chip, > what is a NAND controller. Ok, got it. I will update like xnfc. > > > > > + struct mtd_info *mtd; > > > > + struct nand_chip *nand_chip; > > > > > > This one you can call it just "nand" or "chip". > > Ok, I will update. > > > > > > > > > + struct resource *res; > > > > + > > > > + xnand = devm_kzalloc(&pdev->dev, sizeof(*xnand), GFP_KERNEL); > > > > + if (!xnand) > > > > + return -ENOMEM; > > > > + > > > > + /* Map physical address of NAND flash */ > > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > + xnand->nand_base = devm_ioremap_resource(&pdev->dev, res); > > > > + if (IS_ERR(xnand->nand_base)) > > > > + return PTR_ERR(xnand->nand_base); > > > > + > > > > + nand_chip = &xnand->chip; > > > > + mtd = nand_to_mtd(nand_chip); > > > > + nand_chip->exec_op = pl353_nfc_exec_op; > > > > + nand_set_controller_data(nand_chip, xnand); > > > > + mtd->priv = nand_chip; > > > > + mtd->owner = THIS_MODULE; > > > > + mtd->name = PL353_NAND_DRIVER_NAME; > > > > > > A label property in the DT might overwrite this value. > > Could you please explain a bit more ? > > > > I meant something like this: > > https://elixir.bootlin.com/linux/latest/source/drivers/mtd/nand/raw/marvell_nand.c#L251 > 5 Understood. Thanks. I will update in next version.
Thanks, Naga Sureshkumar Relli. > > Thanks, > Miquèl
