Hi Roger,

On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
> Move NAND specific device tree parsing to NAND driver.
> 
> The NAND controller node must have a compatible id, register space
> resource and interrupt resource.
> 
> Signed-off-by: Roger Quadros <rog...@ti.com>

This one's going to need rebased, as there are some conflicting of_node
changes in l2-mtd.git.

> ---
> v4: Warn if using older incompatible DT i.e. compatible property not present
> in nand node.
> 
>  arch/arm/mach-omap2/gpmc-nand.c              |   5 +-
>  drivers/memory/omap-gpmc.c                   | 143 
> +++++++--------------------
>  drivers/mtd/nand/omap2.c                     | 136 +++++++++++++++++++++----
>  include/linux/platform_data/mtd-nand-omap2.h |   3 +-
>  4 files changed, 155 insertions(+), 132 deletions(-)

Also, this is going to be hard to manage across trees, as you touch
three drivers all at once. Is it not possible to split any of this apart
better?

> 
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index ffe646a..e07ca27 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -95,10 +95,7 @@ int gpmc_nand_init(struct omap_nand_platform_data 
> *gpmc_nand_data,
>       gpmc_nand_res[1].start = gpmc_get_irq();
>  
>       memset(&s, 0, sizeof(struct gpmc_settings));
> -     if (gpmc_nand_data->of_node)
> -             gpmc_read_settings_dt(gpmc_nand_data->of_node, &s);
> -     else
> -             gpmc_set_legacy(gpmc_nand_data, &s);
> +     gpmc_set_legacy(gpmc_nand_data, &s);
>  
>       s.device_nand = true;
>  
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index e75226d..318c187 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -29,7 +29,6 @@
>  #include <linux/of_device.h>
>  #include <linux/of_platform.h>
>  #include <linux/omap-gpmc.h>
> -#include <linux/mtd/nand.h>
>  #include <linux/pm_runtime.h>
>  
>  #include <linux/platform_data/mtd-nand-omap2.h>
> @@ -1716,105 +1715,6 @@ static void __maybe_unused 
> gpmc_read_timings_dt(struct device_node *np,
>               of_property_read_bool(np, "gpmc,time-para-granularity");
>  }
>  
> -#if IS_ENABLED(CONFIG_MTD_NAND)
> -
> -static const char * const nand_xfer_types[] = {
> -     [NAND_OMAP_PREFETCH_POLLED]             = "prefetch-polled",
> -     [NAND_OMAP_POLLED]                      = "polled",
> -     [NAND_OMAP_PREFETCH_DMA]                = "prefetch-dma",
> -     [NAND_OMAP_PREFETCH_IRQ]                = "prefetch-irq",
> -};
> -
> -static int gpmc_probe_nand_child(struct platform_device *pdev,
> -                              struct device_node *child)
> -{
> -     u32 val;
> -     const char *s;
> -     struct gpmc_timings gpmc_t;
> -     struct omap_nand_platform_data *gpmc_nand_data;
> -
> -     if (of_property_read_u32(child, "reg", &val) < 0) {
> -             dev_err(&pdev->dev, "%s has no 'reg' property\n",
> -                     child->full_name);
> -             return -ENODEV;
> -     }
> -
> -     gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
> -                                   GFP_KERNEL);
> -     if (!gpmc_nand_data)
> -             return -ENOMEM;
> -
> -     gpmc_nand_data->cs = val;
> -     gpmc_nand_data->of_node = child;
> -
> -     /* Detect availability of ELM module */
> -     gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
> -     if (gpmc_nand_data->elm_of_node == NULL)
> -             gpmc_nand_data->elm_of_node =
> -                                     of_parse_phandle(child, "elm_id", 0);
> -
> -     /* select ecc-scheme for NAND */
> -     if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
> -             pr_err("%s: ti,nand-ecc-opt not found\n", __func__);
> -             return -ENODEV;
> -     }
> -
> -     if (!strcmp(s, "sw"))
> -             gpmc_nand_data->ecc_opt = OMAP_ECC_HAM1_CODE_SW;
> -     else if (!strcmp(s, "ham1") ||
> -              !strcmp(s, "hw") || !strcmp(s, "hw-romcode"))
> -             gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_HAM1_CODE_HW;
> -     else if (!strcmp(s, "bch4"))
> -             if (gpmc_nand_data->elm_of_node)
> -                     gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_BCH4_CODE_HW;
> -             else
> -                     gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
> -     else if (!strcmp(s, "bch8"))
> -             if (gpmc_nand_data->elm_of_node)
> -                     gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_BCH8_CODE_HW;
> -             else
> -                     gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
> -     else if (!strcmp(s, "bch16"))
> -             if (gpmc_nand_data->elm_of_node)
> -                     gpmc_nand_data->ecc_opt =
> -                             OMAP_ECC_BCH16_CODE_HW;
> -             else
> -                     pr_err("%s: BCH16 requires ELM support\n", __func__);
> -     else
> -             pr_err("%s: ti,nand-ecc-opt invalid value\n", __func__);
> -
> -     /* select data transfer mode for NAND controller */
> -     if (!of_property_read_string(child, "ti,nand-xfer-type", &s))
> -             for (val = 0; val < ARRAY_SIZE(nand_xfer_types); val++)
> -                     if (!strcasecmp(s, nand_xfer_types[val])) {
> -                             gpmc_nand_data->xfer_type = val;
> -                             break;
> -                     }
> -
> -     gpmc_nand_data->flash_bbt = of_get_nand_on_flash_bbt(child);
> -
> -     val = of_get_nand_bus_width(child);
> -     if (val == 16)
> -             gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
> -
> -     gpmc_read_timings_dt(child, &gpmc_t);
> -     gpmc_nand_init(gpmc_nand_data, &gpmc_t);
> -
> -     return 0;
> -}
> -#else
> -static int gpmc_probe_nand_child(struct platform_device *pdev,
> -                              struct device_node *child)
> -{
> -     return 0;
> -}
> -#endif
> -
>  #if IS_ENABLED(CONFIG_MTD_ONENAND)
>  static int gpmc_probe_onenand_child(struct platform_device *pdev,
>                                struct device_node *child)
> @@ -1933,9 +1833,42 @@ static int gpmc_probe_generic_child(struct 
> platform_device *pdev,
>               goto err;
>       }
>  
> -     ret = of_property_read_u32(child, "bank-width", &gpmc_s.device_width);
> -     if (ret < 0)
> -             goto err;
> +     if (of_node_cmp(child->name, "nand") == 0) {
> +             /* NAND specific setup */
> +             u32 val;
> +
> +             /* Warn about older DT blobs with no compatible property */
> +             if (!of_property_read_bool(child, "compatible")) {
> +                     dev_warn(&pdev->dev,
> +                              "Incompatible NAND node: missing compatible");
> +                     ret = -EINVAL;
> +                     goto err;
> +             }
> +
> +             val = of_get_nand_bus_width(child);
> +             switch (val) {
> +             case 8:
> +                     gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
> +                     break;
> +             case 16:
> +                     gpmc_s.device_width = GPMC_DEVWIDTH_16BIT;
> +                     break;
> +             default:
> +                     dev_err(&pdev->dev, "%s: invalid 'nand-bus-width'\n",
> +                             child->name);
> +                     ret = -EINVAL;
> +                     goto err;
> +             }
> +
> +             /* disable write protect */
> +             gpmc_configure(GPMC_CONFIG_WP, 0);
> +             gpmc_s.device_nand = true;
> +     } else {
> +             ret = of_property_read_u32(child, "bank-width",
> +                                        &gpmc_s.device_width);
> +             if (ret < 0)
> +                     goto err;
> +     }
>  
>       ret = gpmc_cs_program_settings(cs, &gpmc_s);
>       if (ret < 0)
> @@ -2018,9 +1951,7 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>               if (!child->name)
>                       continue;
>  
> -             if (of_node_cmp(child->name, "nand") == 0)
> -                     ret = gpmc_probe_nand_child(pdev, child);
> -             else if (of_node_cmp(child->name, "onenand") == 0)
> +             if (of_node_cmp(child->name, "onenand") == 0)
>                       ret = gpmc_probe_onenand_child(pdev, child);
>               else
>                       ret = gpmc_probe_generic_child(pdev, child);
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index c35405c..228f498 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_mtd.h>
>  
>  #include <linux/mtd/nand_bch.h>
>  #include <linux/platform_data/elm.h>
> @@ -177,6 +178,8 @@ struct omap_nand_info {
>       struct gpmc_nand_regs           reg;
>       struct gpmc_nand_ops            *ops;
>       /* generated at runtime depending on ECC algorithm and layout selected 
> */
> +     bool                            flash_bbt;
> +     /* generated at runtime depending on ECC algorithm and layout */
>       struct nand_ecclayout           oobinfo;
>       /* fields specific for BCHx_HW ECC scheme */
>       struct device                   *elm_dev;
> @@ -1668,10 +1671,84 @@ static bool omap2_nand_ecc_check(struct 
> omap_nand_info *info,
>       return true;
>  }
>  
> +static const char * const nand_xfer_types[] = {
> +     [NAND_OMAP_PREFETCH_POLLED] = "prefetch-polled",
> +     [NAND_OMAP_POLLED] = "polled",
> +     [NAND_OMAP_PREFETCH_DMA] = "prefetch-dma",
> +     [NAND_OMAP_PREFETCH_IRQ] = "prefetch-irq",
> +};
> +
> +static int omap_get_dt_info(struct device *dev, struct omap_nand_info *info)
> +{
> +     struct device_node *child = dev->of_node;
> +     int i;
> +     const char *s;
> +
> +     /* In old bindings, CS num is embedded in reg property */
> +     if (of_property_read_u32(child, "reg", &info->gpmc_cs) < 0) {
> +             dev_err(dev, "reg not found in DT\n");
> +             return -EINVAL;
> +     }
> +
> +     /* detect availability of ELM module. Won't be present pre-OMAP4 */
> +     info->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
> +     if (!info->elm_of_node)
> +             dev_dbg(dev, "ti,elm-id not in DT\n");
> +
> +     /* select ecc-scheme for NAND */
> +     if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
> +             dev_err(dev, "ti,nand-ecc-opt not found\n");
> +             return -EINVAL;
> +     }
> +
> +     if (!strcmp(s, "sw")) {
> +             info->ecc_opt = OMAP_ECC_HAM1_CODE_SW;
> +     } else if (!strcmp(s, "ham1") ||
> +                !strcmp(s, "hw") || !strcmp(s, "hw-romcode")) {
> +             info->ecc_opt = OMAP_ECC_HAM1_CODE_HW;
> +     } else if (!strcmp(s, "bch4")) {
> +             if (info->elm_of_node)
> +                     info->ecc_opt = OMAP_ECC_BCH4_CODE_HW;
> +             else
> +                     info->ecc_opt = OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
> +     } else if (!strcmp(s, "bch8")) {
> +             if (info->elm_of_node)
> +                     info->ecc_opt = OMAP_ECC_BCH8_CODE_HW;
> +             else
> +                     info->ecc_opt = OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
> +     } else if (!strcmp(s, "bch16")) {
> +             info->ecc_opt = OMAP_ECC_BCH16_CODE_HW;
> +     } else {
> +             dev_err(dev, "unrecognized value for ti,nand-ecc-opt\n");
> +             return -EINVAL;
> +     }
> +
> +     /* select data transfer mode */
> +     if (!of_property_read_string(child, "ti,nand-xfer-type", &s)) {
> +             for (i = 0; i < ARRAY_SIZE(nand_xfer_types); i++) {
> +                     if (!strcasecmp(s, nand_xfer_types[i])) {
> +                             info->xfer_type = i;
> +                             goto next;
> +                     }
> +             }
> +
> +             dev_err(dev, "unrecognized value for ti,nand-xfer-type\n");
> +             return -EINVAL;
> +     }
> +
> +next:
> +     of_get_nand_on_flash_bbt(child);
> +
> +     if (of_get_nand_bus_width(child) == 16)
> +             info->devsize = NAND_BUSWIDTH_16;
> +
> +     return 0;
> +}
> +
>  static int omap_nand_probe(struct platform_device *pdev)
>  {
>       struct omap_nand_info           *info;
> -     struct omap_nand_platform_data  *pdata;
> +     struct omap_nand_platform_data  *pdata = NULL;
>       struct mtd_info                 *mtd;
>       struct nand_chip                *nand_chip;
>       struct nand_ecclayout           *ecclayout;
> @@ -1682,33 +1759,42 @@ static int omap_nand_probe(struct platform_device 
> *pdev)
>       unsigned                        oob_index;
>       struct resource                 *res;
>       struct mtd_part_parser_data     ppdata = {};
> -
> -     pdata = dev_get_platdata(&pdev->dev);
> -     if (pdata == NULL) {
> -             dev_err(&pdev->dev, "platform data missing\n");
> -             return -ENODEV;
> -     }
> +     struct device                   *dev = &pdev->dev;
>  
>       info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
>                               GFP_KERNEL);
>       if (!info)
>               return -ENOMEM;
>  
> -     platform_set_drvdata(pdev, info);
> +     info->pdev = pdev;
>  
> +     if (dev->of_node) {
> +             if (omap_get_dt_info(dev, info))
> +                     return -EINVAL;
> +     } else {
> +             pdata = dev_get_platdata(&pdev->dev);
> +             if (!pdata) {
> +                     dev_err(&pdev->dev, "platform data missing\n");
> +                     return -EINVAL;
> +             }
> +
> +             info->gpmc_cs = pdata->cs;
> +             info->reg = pdata->reg;
> +             info->of_node = pdata->of_node;
> +             info->ecc_opt = pdata->ecc_opt;
> +             info->dev_ready = pdata->dev_ready;
> +             info->xfer_type = pdata->xfer_type;
> +             info->devsize = pdata->devsize;
> +             info->elm_of_node = pdata->elm_of_node;
> +             info->flash_bbt = pdata->flash_bbt;
> +     }
> +
> +     platform_set_drvdata(pdev, info);
>       info->ops = gpmc_omap_get_nand_ops(&info->reg, info->gpmc_cs);
>       if (!info->ops) {
>               dev_err(&pdev->dev, "Failed to get GPMC->NAND interface\n");
>               return -ENODEV;
>       }
> -     info->pdev              = pdev;
> -     info->gpmc_cs           = pdata->cs;
> -     info->of_node           = pdata->of_node;
> -     info->ecc_opt           = pdata->ecc_opt;
> -     info->dev_ready = pdata->dev_ready;
> -     info->xfer_type = pdata->xfer_type;
> -     info->devsize = pdata->devsize;
> -     info->elm_of_node = pdata->elm_of_node;
>  
>       mtd                     = &info->mtd;
>       mtd->priv               = &info->nand;
> @@ -1744,7 +1830,7 @@ static int omap_nand_probe(struct platform_device *pdev)
>               nand_chip->chip_delay = 50;
>       }
>  
> -     if (pdata->flash_bbt)
> +     if (info->flash_bbt)
>               nand_chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>       else
>               nand_chip->options |= NAND_SKIP_BBTSCAN;
> @@ -2049,9 +2135,13 @@ scan_tail:
>               goto return_error;
>       }
>  
> -     ppdata.of_node = pdata->of_node;
> -     mtd_device_parse_register(mtd, NULL, &ppdata, pdata->parts,
> -                               pdata->nr_parts);
> +     if (dev->of_node) {
> +             ppdata.of_node = dev->of_node;

The latest l2-mtd.git changed how the partitions' of_node is passed. Now
this is handled by nand_set_flash_node().

> +             mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> +
> +     } else {
> +             mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
> +     }
>  
>       platform_set_drvdata(pdev, mtd);
>  
> @@ -2083,11 +2173,17 @@ static int omap_nand_remove(struct platform_device 
> *pdev)
>       return 0;
>  }
>  
> +static const struct of_device_id omap_nand_ids[] = {
> +     { .compatible = "ti,omap2-nand", },
> +     {},
> +};
> +
>  static struct platform_driver omap_nand_driver = {
>       .probe          = omap_nand_probe,
>       .remove         = omap_nand_remove,
>       .driver         = {
>               .name   = DRIVER_NAME,
> +             .of_match_table = of_match_ptr(omap_nand_ids),
>       },
>  };
>  
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h 
> b/include/linux/platform_data/mtd-nand-omap2.h
> index a067f58..ff27e5a 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -76,11 +76,10 @@ struct omap_nand_platform_data {
>       int                     devsize;
>       enum omap_ecc           ecc_opt;
>  
> -     /* for passing the partitions */
> -     struct device_node      *of_node;
>       struct device_node      *elm_of_node;
>  
>       /* deprecated */
>       struct gpmc_nand_regs   reg;
> +     struct device_node      *of_node;

I'm a little confused here. Do you have a mixed platform data / device
tree setup here? That's odd. (It also seems if that was really
necessary, you could have the board file set pdev->dev.of_node before
registering it, then you don't need this field.) But really, if you're
partly using device tree, can't you just convert completely? Or is this
a two-phase process, and you're planning to convert omap2 to full device
tree?

>  };
>  #endif

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to