Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938873AbcJXNCH (ORCPT ); Mon, 24 Oct 2016 09:02:07 -0400 Received: from up.free-electrons.com ([163.172.77.33]:44046 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934336AbcJXNCE (ORCPT ); Mon, 24 Oct 2016 09:02:04 -0400 Date: Mon, 24 Oct 2016 15:02:01 +0200 From: Boris Brezillon To: Sergio Prado Cc: kgene@kernel.org, krzk@kernel.org, javier@osg.samsung.com, linux@armlinux.org.uk, linux@simtec.co.uk, richard@nod.at, dwmw2@infradead.org, computersforpeace@gmail.com, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org Subject: Re: [PATCH v3 3/3] mtd: s3c2410: parse the device configuration from OF node Message-ID: <20161024150201.01952f67@bbrezillon> In-Reply-To: <1476999766-32526-4-git-send-email-sergio.prado@e-labworks.com> References: <1476999766-32526-1-git-send-email-sergio.prado@e-labworks.com> <1476999766-32526-4-git-send-email-sergio.prado@e-labworks.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9173 Lines: 297 On Thu, 20 Oct 2016 19:42:46 -0200 Sergio Prado wrote: > Allows configuring Samsung's s3c2410 memory controller using a > devicetree. > > Signed-off-by: Sergio Prado > --- > 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 > #include > #include > +#include > +#include > > #include > #include > @@ -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 {