Re: [PATCH v3 3/3] mtd: s3c2410: parse the device configuration from OF node

From: Boris Brezillon
Date: Mon Oct 24 2016 - 09:02:09 EST


On Thu, 20 Oct 2016 19:42:46 -0200
Sergio Prado <sergio.prado@xxxxxxxxxxxxxx> wrote:

> Allows configuring Samsung's s3c2410 memory controller using a
> devicetree.
>
> Signed-off-by: Sergio Prado <sergio.prado@xxxxxxxxxxxxxx>
> ---
> drivers/mtd/nand/s3c2410.c | 158 ++++++++++++++++++++++---
> include/linux/platform_data/mtd-nand-s3c2410.h | 1 +
> 2 files changed, 143 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index 371db0d48135..ec170be881bc 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -39,6 +39,8 @@
> #include <linux/slab.h>
> #include <linux/clk.h>
> #include <linux/cpufreq.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/nand.h>
> @@ -185,6 +187,22 @@ struct s3c2410_nand_info {
> #endif
> };
>
> +struct s3c24XX_nand_devtype_data {
> + enum s3c_cpu_type type;
> +};
> +
> +static const struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = {
> + .type = TYPE_S3C2410,
> +};
> +
> +static const struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = {
> + .type = TYPE_S3C2412,
> +};
> +
> +static const struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = {
> + .type = TYPE_S3C2440,
> +};
> +
> /* conversion functions */
>
> static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd)
> @@ -794,6 +812,30 @@ static int s3c2410_nand_add_partition(struct s3c2410_nand_info *info,
> return -ENODEV;
> }
>
> +static int s3c2410_nand_setup_data_interface(struct mtd_info *mtd,
> + const struct nand_data_interface *conf,
> + bool check_only)
> +{
> + struct s3c2410_nand_info *info = s3c2410_nand_mtd_toinfo(mtd);
> + struct s3c2410_platform_nand *pdata = info->platform;
> + const struct nand_sdr_timings *timings;
> + int tacls;
> +
> + timings = nand_get_sdr_timings(conf);
> + if (IS_ERR(timings))
> + return -ENOTSUPP;
> +
> + tacls = timings->tCLS_min - timings->tWP_min;
> + if (tacls < 0)
> + tacls = 0;
> +
> + pdata->tacls = DIV_ROUND_UP(tacls, 1000);
> + pdata->twrph0 = DIV_ROUND_UP(timings->tWP_min, 1000);
> + pdata->twrph1 = DIV_ROUND_UP(timings->tCLH_min, 1000);

You seem to only apply the timings in s3c2410_nand_setrate(), which is
only called at probe time or on a cpufreq even, but the core can change
timings at runtime (this is what happens each time you reset the chip).

To support that you have 2 options:
- apply the timings in ->select_chip()
- apply the timings here

> +
> + return 0;
> +}
> +
> /**
> * s3c2410_nand_init_chip - initialise a single instance of an chip
> * @info: The base NAND controller the chip is on.
> @@ -808,9 +850,12 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
> struct s3c2410_nand_mtd *nmtd,
> struct s3c2410_nand_set *set)
> {
> + struct device_node *np = info->device->of_node;
> struct nand_chip *chip = &nmtd->chip;
> void __iomem *regs = info->regs;
>
> + nand_set_flash_node(chip, set->of_node);
> +
> chip->write_buf = s3c2410_nand_write_buf;
> chip->read_buf = s3c2410_nand_read_buf;
> chip->select_chip = s3c2410_nand_select_chip;
> @@ -819,6 +864,13 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
> chip->options = set->options;
> chip->controller = &info->controller;
>
> + /*
> + * let's keep behavior unchanged for legacy boards booting via pdata and
> + * auto-detect timings only when booting with a device tree.
> + */
> + if (np)
> + chip->setup_data_interface = s3c2410_nand_setup_data_interface;
> +
> switch (info->cpu_type) {
> case TYPE_S3C2410:
> chip->IO_ADDR_W = regs + S3C2410_NFDATA;
> @@ -859,12 +911,9 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
> chip->ecc.mode = info->platform->ecc_mode;
>
> /* If you use u-boot BBT creation code, specifying this flag will
> - * let the kernel fish out the BBT from the NAND, and also skip the
> - * full NAND scan that can take 1/2s or so. Little things... */
> - if (set->flash_bbt) {
> + * let the kernel fish out the BBT from the NAND */
> + if (set->flash_bbt)
> chip->bbt_options |= NAND_BBT_USE_FLASH;
> - chip->options |= NAND_SKIP_BBTSCAN;
> - }
> }
>
> /**
> @@ -943,6 +992,77 @@ static int s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
> return -EINVAL;
> }
>
> + if (chip->bbt_options & NAND_BBT_USE_FLASH)
> + chip->options |= NAND_SKIP_BBTSCAN;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id s3c24xx_nand_dt_ids[] = {
> + {
> + .compatible = "samsung,s3c2410-nand",
> + .data = &s3c2410_nand_devtype_data,
> + }, {
> + .compatible = "samsung,s3c2412-nand", /* also compatible with s3c6400 */
> + .data = &s3c2412_nand_devtype_data,
> + }, {
> + .compatible = "samsung,s3c2440-nand",
> + .data = &s3c2440_nand_devtype_data,
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, s3c24xx_nand_dt_ids);
> +
> +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> +{
> + const struct s3c24XX_nand_devtype_data *devtype_data;
> + struct s3c2410_platform_nand *pdata;
> + struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> + struct device_node *np = pdev->dev.of_node, *child;
> + struct s3c2410_nand_set *sets;
> +
> + devtype_data = of_device_get_match_data(&pdev->dev);
> + if (!devtype_data)
> + return -ENODEV;
> +
> + info->cpu_type = devtype_data->type;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + pdev->dev.platform_data = pdata;
> +
> + pdata->nr_sets = of_get_child_count(np);
> + if (!pdata->nr_sets)
> + return 0;
> +
> + sets = devm_kzalloc(&pdev->dev, sizeof(*sets) * pdata->nr_sets, GFP_KERNEL);
> + if (!sets)
> + return -ENOMEM;
> +
> + pdata->sets = sets;
> +
> + for_each_available_child_of_node(np, child) {
> +
> + sets->name = (char *)child->name;
> + sets->of_node = child;
> + sets->nr_chips = 1;
> +
> + of_node_get(child);
> +
> + sets++;
> + }
> +
> + return 0;
> +}
> +
> +static int s3c24xx_nand_probe_pdata(struct platform_device *pdev)
> +{
> + struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> +
> + info->cpu_type = platform_get_device_id(pdev)->driver_data;
> +
> return 0;
> }
>
> @@ -955,8 +1075,7 @@ static int s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
> */
> static int s3c24xx_nand_probe(struct platform_device *pdev)
> {
> - struct s3c2410_platform_nand *plat = to_nand_plat(pdev);
> - enum s3c_cpu_type cpu_type;
> + struct s3c2410_platform_nand *plat;
> struct s3c2410_nand_info *info;
> struct s3c2410_nand_mtd *nmtd;
> struct s3c2410_nand_set *sets;
> @@ -966,8 +1085,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> int nr_sets;
> int setno;
>
> - cpu_type = platform_get_device_id(pdev)->driver_data;
> -
> info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> if (info == NULL) {
> err = -ENOMEM;
> @@ -989,6 +1106,16 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>
> s3c2410_nand_clk_set_state(info, CLOCK_ENABLE);
>
> + if (pdev->dev.of_node)
> + err = s3c24xx_nand_probe_dt(pdev);
> + else
> + err = s3c24xx_nand_probe_pdata(pdev);
> +
> + if (err)
> + goto exit_error;
> +
> + plat = to_nand_plat(pdev);
> +
> /* allocate and map the resource */
>
> /* currently we assume we have the one resource */
> @@ -997,7 +1124,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>
> info->device = &pdev->dev;
> info->platform = plat;
> - info->cpu_type = cpu_type;
>
> info->regs = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(info->regs)) {
> @@ -1007,12 +1133,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>
> dev_dbg(&pdev->dev, "mapped registers at %p\n", info->regs);
>
> - /* initialise the hardware */
> -
> - err = s3c2410_nand_inithw(info);
> - if (err != 0)
> - goto exit_error;
> -
> sets = (plat != NULL) ? plat->sets : NULL;
> nr_sets = (plat != NULL) ? plat->nr_sets : 1;
>
> @@ -1056,6 +1176,11 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> sets++;
> }
>
> + /* initialise the hardware */
> + err = s3c2410_nand_inithw(info);
> + if (err != 0)
> + goto exit_error;
> +
> err = s3c2410_nand_cpufreq_register(info);
> if (err < 0) {
> dev_err(&pdev->dev, "failed to init cpufreq support\n");
> @@ -1156,6 +1281,7 @@ static int s3c24xx_nand_resume(struct platform_device *dev)
> .id_table = s3c24xx_driver_ids,
> .driver = {
> .name = "s3c24xx-nand",
> + .of_match_table = s3c24xx_nand_dt_ids,
> },
> };
>
> diff --git a/include/linux/platform_data/mtd-nand-s3c2410.h b/include/linux/platform_data/mtd-nand-s3c2410.h
> index 729af13d1773..f01659026b26 100644
> --- a/include/linux/platform_data/mtd-nand-s3c2410.h
> +++ b/include/linux/platform_data/mtd-nand-s3c2410.h
> @@ -40,6 +40,7 @@ struct s3c2410_nand_set {
> char *name;
> int *nr_map;
> struct mtd_partition *partitions;
> + struct device_node *of_node;
> };
>
> struct s3c2410_platform_nand {