[PATCH V5 1/2] Nand driver for Nomadik 8815 SoC (on NHK8815 board)

vimal singh vimal.newwork at gmail.com
Fri Jul 24 08:10:11 EDT 2009


Hi,

Please run ''checkpatch.pl' for this patch and try to fix the errors.

Few more comments below:

On Fri, Jul 24, 2009 at 5:02 PM, Alessandro Rubini<rubini-list at gnudd.com> wrote:
> From: Alessandro Rubini <rubini at unipv.it>
>
>
> Signed-off-by: Alessandro Rubini <rubini at unipv.it>
> Acked-by: Andrea Gallo <andrea.gallo at stericsson.com>
> ---
---sinp---

> +static inline int parity(int b) /* uses low 8 bits: returns 0 or all-1 */
> +{
> +       b = b ^ (b >> 4);
> +       b = b ^ (b >> 2);
> +       return (b ^ (b >> 1)) & 1
> +               ? ~0 : 0;
> +}

This function is not required anymore, can be removed.

> +
> +static struct nand_ecclayout nomadik_ecc_layout = {
> +       .eccbytes = 3 * 4,
> +       .eccpos = { /* each subpage has 16 bytes: pos 2,3,4 hosts ECC */
> +               0x02, 0x03, 0x04,
> +               0x12, 0x13, 0x14,
> +               0x22, 0x23, 0x24,
> +               0x32, 0x33, 0x34},
> +       /* let's keep bytes 5,6,7 for us, just in case we change ECC algo */
> +       .oobfree = { {0x08, 0x08}, {0x18, 0x08}, {0x28, 0x08}, {0x38, 0x08} },
> +};
> +
> +static void nomadik_ecc_control(struct mtd_info *mtd, int mode)
> +{
> +       /* No need to enable hw ecc, it's on by default */
> +}
> +
> +static void nomadik_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
> +{
> +       struct nand_chip *nand = mtd->priv;
> +       struct nomadik_nand_host *host = nand->priv;
> +
> +       if (cmd == NAND_CMD_NONE)
> +               return;
> +
> +       if (ctrl & NAND_CLE)
> +               writeb(cmd, host->cmd_va);
> +       else
> +               writeb(cmd, host->addr_va);
> +}
> +
> +static int nomadik_nand_probe(struct platform_device *pdev)
> +{
> +       struct nomadik_nand_platform_data *pdata = pdev->dev.platform_data;
> +       struct nomadik_nand_host *host;
> +       struct mtd_info *mtd;
> +       struct nand_chip *nand;
> +       struct resource *res;
> +       int ret = 0;
> +
> +       /* Allocate memory for the device structure (and zero it) */
> +       host = kzalloc(sizeof(struct nomadik_nand_host), GFP_KERNEL);
> +       if (!host) {
> +               dev_err(&pdev->dev, "Failed to allocate device structure.\n");
> +               return -ENOMEM;
> +       }
> +
> +       /* Call the client's init function, if any */
> +       if (pdata->init && (ret = pdata->init()) < 0) {

'checkpatch.pl' here... (do not use assignment in if condition)

> +               dev_err(&pdev->dev, "Init function failed\n");
> +               goto err;
> +       }
> +
> +       /* ioremap three regions */
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_addr");
> +       if (!res) {ret = -EIO; goto err_unmap; }
> +       host->addr_va = ioremap(res->start, res->end - res->start + 1);
> +
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_data");
> +       if (!res) {ret = -EIO; goto err_unmap; }
> +       host->data_va = ioremap(res->start, res->end - res->start + 1);
> +
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_cmd");
> +       if (!res) {ret = -EIO; goto err_unmap; }
> +       host->cmd_va = ioremap(res->start, res->end - res->start + 1);

'checkpatch.pl' here... (trailing statements should be on next line)

> +
> +       if (!host->addr_va || !host->data_va || !host->cmd_va) {
> +               ret = -ENOMEM;
> +               goto err_unmap;
> +       }
> +
> +       /* Link all private pointers */
> +       mtd = &host->mtd;
> +       nand = &host->nand;
> +       mtd->priv = nand;
> +       nand->priv = host;
> +
> +       host->mtd.owner = THIS_MODULE;
> +       nand->IO_ADDR_R = host->data_va;
> +       nand->IO_ADDR_W = host->data_va;
> +       nand->cmd_ctrl = nomadik_cmd_ctrl;
> +
> +       /*
> +        * This stanza declares ECC_HW but uses soft routines. It's because
> +        * HW claims to make the calculation but not the correction. However,
> +        * I haven't managed to get the desired data out of it until now.
> +        */
> +       nand->ecc.mode = NAND_ECC_HW;

this can be 'NAND_ECC_SOFT' here... then

> +       nand->ecc.calculate = nand_calculate_ecc;
> +       nand->ecc.correct = nand_correct_data;
you need not to do these here.

> +       nand->ecc.hwctl = nomadik_ecc_control;
> +       nand->ecc.size = 512;
> +       nand->ecc.bytes = 3;
> +
> +       nand->options = pdata->options;
> +
> +       /*
> +        * Scan to find existance of the device
> +        */
> +       if (nand_scan(&host->mtd, 1)) {
> +               ret = -ENXIO;
> +               goto err_unmap;
> +       }
> +
> +#ifdef CONFIG_MTD_PARTITIONS
> +       add_mtd_partitions(&host->mtd, pdata->parts, pdata->nparts);
> +#else
> +       pr_info("Registering %s as whole device\n", mtd->name);
> +       add_mtd_device(mtd);
> +#endif
> +
> +       platform_set_drvdata(pdev, host);
> +       return 0;
> +
> + err_unmap:
> +       if (host->cmd_va) iounmap(host->cmd_va);
> +       if (host->data_va) iounmap(host->data_va);
> +       if (host->addr_va) iounmap(host->addr_va);

'checkpatch.pl' here... (trailing statements should be on next line)

-vimal



More information about the linux-mtd mailing list