| Message ID | 1474125760-28168-1-git-send-email-sergio.prado@e-labworks.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Sergio, On Sat, 17 Sep 2016 12:22:40 -0300 Sergio Prado <sergio.prado@e-labworks.com> wrote: > Tested on FriendlyARM Mini2440 > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > --- > .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ DT maintainers usually ask people to keep the DT bindings doc in a separate patch. > drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- > include/linux/platform_data/mtd-nand-s3c2410.h | 1 + > 3 files changed, 195 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > new file mode 100644 > index 000000000000..1c39f6cf483b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > @@ -0,0 +1,70 @@ > +* Samsung S3C2410 and compatible NAND flash controller > + > +Required properties: > +- compatible : The possible values are: > + "samsung,s3c2410-nand" > + "samsung,s3c2412-nand" > + "samsung,s3c2440-nand" > + "samsung,s3c6400-nand" > +- reg : register's location and length. > +- #address-cells, #size-cells : see nand.txt > +- clocks : phandle to the nand controller clock > +- clock-names : must contain "nand" > + > +Optional properties: > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > +- samsung,twrph0 : active time for nWE/nOE > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive Can you try to extract these information from the nand_sdr_timings struct instead of passing it in your DT? > +- samsung,ignore_unset_ecc : boolean to ignore error when we have > + 0xff,0xff,0xff read ECC, on the > + assumption that it is an un-eccd page > + > +Optional children nodes: > +Children nodes representing the available nand chips. > + > +Optional children properties: > +- nand-ecc-mode : see nand.txt > +- nand-on-flash-bbt : see nand.txt > + > +Each children device node may optionally contain a 'partitions' sub-node, > +which further contains sub-nodes describing the flash partition mapping. > +See partition.txt for more detail. > + > +Example: > + > +nand@4e000000 { s/nand/nand-controller/ > + compatible = "samsung,s3c2440-nand"; > + reg = <0x4e000000 0x40>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + clocks = <&clocks HCLK_NAND>; > + clock-names = "nand"; > + > + samsung,tacls = <0>; > + samsung,twrph0 = <25>; > + samsung,twrph1 = <15>; As said above, I think these timings can be extracted from the nand_sdr_timings struct, which is know automatically attached to nand_chip at detection time. > + samsung,ignore_unset_ecc; Just discovered this ->ignore_unset_ecc property, and I don't understand why it's here for... Either you don't want to have ECC, and in this case you should set NAND_ECC_NONE, or you want to have ECC calculated, and in this case, the only valid situation where ECC bytes are 0xff is when the page is erased. If I'm right, please fix the driver instead of adding this DT property. If I'm wrong, could you explain in more detail when you have ECC bytes set to 0xff? > + > + nand@0 { @0 implies having a reg property. I don't see any in your example, and it's not document in the required property list. Is your controller able to connect to different CS? > + nand-ecc-mode = "soft"; > + nand-on-flash-bbt; > + > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + partition@0 { > + label = "u-boot"; > + reg = <0 0x040000>; > + }; > + > + partition@40000 { > + label = "kernel"; > + reg = <0x040000 0x500000>; > + }; > + }; > + }; > +}; > diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c > index d9309cf0ce2e..ecbb9c9c1e9a 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,26 @@ struct s3c2410_nand_info { > #endif > }; > > +struct s3c24XX_nand_devtype_data { > + enum s3c_cpu_type type; > +}; > + > +struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = { > + .type = TYPE_S3C2410, > +}; > + > +struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = { > + .type = TYPE_S3C2412, > +}; > + > +struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = { > + .type = TYPE_S3C2440, > +}; > + > +struct s3c24XX_nand_devtype_data s3c6400_nand_devtype_data = { > + .type = TYPE_S3C2412, > +}; > + > /* conversion functions */ > > static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd) > @@ -813,6 +835,8 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info, > 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; > @@ -947,6 +971,96 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info, > } > } > > +#ifdef CONFIG_OF_MTD Hm, I thought this symbol had been dropped, but apparently I forgot to remove it. You should make it dependent on CONFIG_OF, not CONFIG_OF_MTD. Anyway, I'm not even sure this ifdef is needed. Just test if pdev->dev.of_node is NULL in s3c24xx_nand_probe_dt() and return -1 in this case. > +static const struct of_device_id s3c24xx_nand_dt_ids[] = { > + { > + .compatible = "samsung,s3c2410-nand", > + .data = &s3c2410_nand_devtype_data, > + }, { > + .compatible = "samsung,s3c2412-nand", > + .data = &s3c2412_nand_devtype_data, > + }, { > + .compatible = "samsung,s3c2440-nand", > + .data = &s3c2440_nand_devtype_data, > + }, { > + .compatible = "samsung,s3c6400-nand", > + .data = &s3c6400_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; > + const struct of_device_id *of_id; > + struct s3c2410_nand_set *sets; > + > + of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev); > + if (!of_id) > + return 1; > + > + devtype_data = of_id->data; > + info->cpu_type = devtype_data->type; > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return -ENOMEM; > + > + pdev->dev.platform_data = pdata; > + > + of_property_read_u32(np, "samsung,tacls", &pdata->tacls); > + of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0); > + of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1); > + > + if (of_get_property(np, "samsung,ignore_unset_ecc", NULL)) > + pdata->ignore_unset_ecc = 1; > + > + 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; > + > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > + sets->disable_ecc = 1; > + > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > + sets->flash_bbt = 1; > + These properties are automatically extracted in nand_scan_ident(), why do you need to parse them twice? > + sets++; > + } > + > + return 0; > +} > +#else > +static int s3c24xx_nand_probe_dt(struct platform_device *pdev) > +{ > + return 1; > +} > +#endif > + > +static void 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; > +} > + > /* s3c24xx_nand_probe > * > * called by device layer when it finds a device matching > @@ -956,8 +1070,7 @@ static void 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; > @@ -967,8 +1080,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; > @@ -991,6 +1102,14 @@ static int s3c24xx_nand_probe(struct platform_device *pdev) > > s3c2410_nand_clk_set_state(info, CLOCK_ENABLE); > > + err = s3c24xx_nand_probe_dt(pdev); > + if (err > 0) > + s3c24xx_nand_probe_pdata(pdev); > + else if (err < 0) > + goto exit_error; > + > + plat = to_nand_plat(pdev); > + > /* allocate and map the resource */ > > /* currently we assume we have the one resource */ > @@ -999,7 +1118,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)) { > @@ -1156,6 +1274,7 @@ static struct platform_driver s3c24xx_nand_driver = { > .id_table = s3c24xx_driver_ids, > .driver = { > .name = "s3c24xx-nand", > + .of_match_table = s3c24xx_nand_dt_ids, If you keep the #ifdef CONFIG_OF section .of_match_table = of_match_ptr(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 c55e42ee57fa..9d20871e4bbd 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 { -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Sep 17, 2016 at 12:22:40PM -0300, Sergio Prado wrote: > Tested on FriendlyARM Mini2440 > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > --- > .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ > drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- > include/linux/platform_data/mtd-nand-s3c2410.h | 1 + > 3 files changed, 195 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > new file mode 100644 > index 000000000000..1c39f6cf483b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > @@ -0,0 +1,70 @@ > +* Samsung S3C2410 and compatible NAND flash controller > + > +Required properties: > +- compatible : The possible values are: > + "samsung,s3c2410-nand" > + "samsung,s3c2412-nand" > + "samsung,s3c2440-nand" > + "samsung,s3c6400-nand" > +- reg : register's location and length. > +- #address-cells, #size-cells : see nand.txt > +- clocks : phandle to the nand controller clock > +- clock-names : must contain "nand" Is there only one clock feeding into the NAND block? Is it actually called "nand", or does the datasheet not give the input a nme? > + > +Optional properties: > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > +- samsung,twrph0 : active time for nWE/nOE > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive If these are necessary, what units are these in? > +- samsung,ignore_unset_ecc : boolean to ignore error when we have > + 0xff,0xff,0xff read ECC, on the > + assumption that it is an un-eccd page s/_/-/ in property names, please. That said, this name reads like policy rather than a HW description. Why/when is this necessary to set? [...] > + of_property_read_u32(np, "samsung,tacls", &pdata->tacls); > + of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0); > + of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1); Are all values in the range [0, 0xffffffff] valid for these? If not, it would be a good idea to sanity-check the values. > + if (of_get_property(np, "samsung,ignore_unset_ecc", NULL)) > + pdata->ignore_unset_ecc = 1; Use of_property_read_bool(); [...] > + for_each_available_child_of_node(np, child) { > + > + sets->name = (char *)child->name; > + sets->of_node = child; Strictly speaking, you should increment the child node refcount here, as you maintain a reference to it and its name field. > + sets->nr_chips = 1; > + > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > + sets->disable_ecc = 1; > + > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > + sets->flash_bbt = 1; Use of_property_read_bool(). Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/17/2016 05:22 PM, Sergio Prado wrote: > +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; > + const struct of_device_id *of_id; > + struct s3c2410_nand_set *sets; > + > + of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev); > + if (!of_id) > + return 1; > + > + devtype_data = of_id->data; You could make it a bit simpler with of_device_get_match_data(). -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Sep 17, 2016 at 07:31:23PM +0200, Boris Brezillon wrote: > Hi Sergio, > > On Sat, 17 Sep 2016 12:22:40 -0300 > Sergio Prado <sergio.prado@e-labworks.com> wrote: > > > Tested on FriendlyARM Mini2440 > > > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > > --- > > .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ > > DT maintainers usually ask people to keep the DT bindings doc in a > separate patch. Right. I'll send the DT bindings doc in a separate patch. > > > drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- > > include/linux/platform_data/mtd-nand-s3c2410.h | 1 + > > 3 files changed, 195 insertions(+), 5 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > > > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > new file mode 100644 > > index 000000000000..1c39f6cf483b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > @@ -0,0 +1,70 @@ > > +* Samsung S3C2410 and compatible NAND flash controller > > + > > +Required properties: > > +- compatible : The possible values are: > > + "samsung,s3c2410-nand" > > + "samsung,s3c2412-nand" > > + "samsung,s3c2440-nand" > > + "samsung,s3c6400-nand" > > +- reg : register's location and length. > > +- #address-cells, #size-cells : see nand.txt > > +- clocks : phandle to the nand controller clock > > +- clock-names : must contain "nand" > > + > > +Optional properties: > > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > > +- samsung,twrph0 : active time for nWE/nOE > > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive > > Can you try to extract these information from the nand_sdr_timings > struct instead of passing it in your DT? OK. Looks like it is possible to extract the timings from nand_sdr_timings. I'll try to do it. > > > +- samsung,ignore_unset_ecc : boolean to ignore error when we have > > + 0xff,0xff,0xff read ECC, on the > > + assumption that it is an un-eccd page > > + > > +Optional children nodes: > > +Children nodes representing the available nand chips. > > + > > +Optional children properties: > > +- nand-ecc-mode : see nand.txt > > +- nand-on-flash-bbt : see nand.txt > > + > > +Each children device node may optionally contain a 'partitions' sub-node, > > +which further contains sub-nodes describing the flash partition mapping. > > +See partition.txt for more detail. > > + > > +Example: > > + > > +nand@4e000000 { > > s/nand/nand-controller/ Ops. My bad. > > > + compatible = "samsung,s3c2440-nand"; > > + reg = <0x4e000000 0x40>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + clocks = <&clocks HCLK_NAND>; > > + clock-names = "nand"; > > + > > + samsung,tacls = <0>; > > + samsung,twrph0 = <25>; > > + samsung,twrph1 = <15>; > > As said above, I think these timings can be extracted from the > nand_sdr_timings struct, which is know automatically attached to > nand_chip at detection time. Right. > > > + samsung,ignore_unset_ecc; > > Just discovered this ->ignore_unset_ecc property, and I don't > understand why it's here for... > Either you don't want to have ECC, and in this case you should set > NAND_ECC_NONE, or you want to have ECC calculated, and in this case, > the only valid situation where ECC bytes are 0xff is when the page is > erased. > > If I'm right, please fix the driver instead of adding this DT property. > If I'm wrong, could you explain in more detail when you have ECC bytes > set to 0xff? That's what I initially thought, but I must confess I just focused on keeping the same interface. I'll try to understand better if this check is really necessary. > > > + > > + nand@0 { > > @0 implies having a reg property. I don't see any in your example, and > it's not document in the required property list. > > Is your controller able to connect to different CS? No, it is not necessary. I'll remove @0. > > > + nand-ecc-mode = "soft"; > > + nand-on-flash-bbt; > > + > > + partitions { > > + compatible = "fixed-partitions"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + partition@0 { > > + label = "u-boot"; > > + reg = <0 0x040000>; > > + }; > > + > > + partition@40000 { > > + label = "kernel"; > > + reg = <0x040000 0x500000>; > > + }; > > + }; > > + }; > > +}; > > diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c > > index d9309cf0ce2e..ecbb9c9c1e9a 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,26 @@ struct s3c2410_nand_info { > > #endif > > }; > > > > +struct s3c24XX_nand_devtype_data { > > + enum s3c_cpu_type type; > > +}; > > + > > +struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = { > > + .type = TYPE_S3C2410, > > +}; > > + > > +struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = { > > + .type = TYPE_S3C2412, > > +}; > > + > > +struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = { > > + .type = TYPE_S3C2440, > > +}; > > + > > +struct s3c24XX_nand_devtype_data s3c6400_nand_devtype_data = { > > + .type = TYPE_S3C2412, > > +}; > > + > > /* conversion functions */ > > > > static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd) > > @@ -813,6 +835,8 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info, > > 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; > > @@ -947,6 +971,96 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info, > > } > > } > > > > +#ifdef CONFIG_OF_MTD > > Hm, I thought this symbol had been dropped, but apparently I forgot to > remove it. You should make it dependent on CONFIG_OF, not CONFIG_OF_MTD. > > Anyway, I'm not even sure this ifdef is needed. Just test if > pdev->dev.of_node is NULL in s3c24xx_nand_probe_dt() and return -1 in > this case. Right. I'll remove this ifdef. > > > +static const struct of_device_id s3c24xx_nand_dt_ids[] = { > > + { > > + .compatible = "samsung,s3c2410-nand", > > + .data = &s3c2410_nand_devtype_data, > > + }, { > > + .compatible = "samsung,s3c2412-nand", > > + .data = &s3c2412_nand_devtype_data, > > + }, { > > + .compatible = "samsung,s3c2440-nand", > > + .data = &s3c2440_nand_devtype_data, > > + }, { > > + .compatible = "samsung,s3c6400-nand", > > + .data = &s3c6400_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; > > + const struct of_device_id *of_id; > > + struct s3c2410_nand_set *sets; > > + > > + of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev); > > + if (!of_id) > > + return 1; > > + > > + devtype_data = of_id->data; > > + info->cpu_type = devtype_data->type; > > + > > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) > > + return -ENOMEM; > > + > > + pdev->dev.platform_data = pdata; > > + > > + of_property_read_u32(np, "samsung,tacls", &pdata->tacls); > > + of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0); > > + of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1); > > + > > + if (of_get_property(np, "samsung,ignore_unset_ecc", NULL)) > > + pdata->ignore_unset_ecc = 1; > > + > > + 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; > > + > > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > > + sets->disable_ecc = 1; > > + > > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > > + sets->flash_bbt = 1; > > + > > These properties are automatically extracted in nand_scan_ident(), why > do you need to parse them twice? Looks like the driver uses this properties before they are extracted in nand_scan_ident(). But I'll try to understand better what the driver is doing and prevent from parsing these properties twice. > > > + sets++; > > + } > > + > > + return 0; > > +} > > +#else > > +static int s3c24xx_nand_probe_dt(struct platform_device *pdev) > > +{ > > + return 1; > > +} > > +#endif > > + > > +static void 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; > > +} > > + > > /* s3c24xx_nand_probe > > * > > * called by device layer when it finds a device matching > > @@ -956,8 +1070,7 @@ static void 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; > > @@ -967,8 +1080,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; > > @@ -991,6 +1102,14 @@ static int s3c24xx_nand_probe(struct platform_device *pdev) > > > > s3c2410_nand_clk_set_state(info, CLOCK_ENABLE); > > > > + err = s3c24xx_nand_probe_dt(pdev); > > + if (err > 0) > > + s3c24xx_nand_probe_pdata(pdev); > > + else if (err < 0) > > + goto exit_error; > > + > > + plat = to_nand_plat(pdev); > > + > > /* allocate and map the resource */ > > > > /* currently we assume we have the one resource */ > > @@ -999,7 +1118,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)) { > > @@ -1156,6 +1274,7 @@ static struct platform_driver s3c24xx_nand_driver = { > > .id_table = s3c24xx_driver_ids, > > .driver = { > > .name = "s3c24xx-nand", > > + .of_match_table = s3c24xx_nand_dt_ids, > > If you keep the #ifdef CONFIG_OF section > > .of_match_table = of_match_ptr(s3c24xx_nand_dt_ids), I'll remove the ifdef. > > > }, > > }; > > > > diff --git a/include/linux/platform_data/mtd-nand-s3c2410.h b/include/linux/platform_data/mtd-nand-s3c2410.h > > index c55e42ee57fa..9d20871e4bbd 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 { >
On Mon, Sep 19, 2016 at 10:11:55AM +0100, Mark Rutland wrote: > On Sat, Sep 17, 2016 at 12:22:40PM -0300, Sergio Prado wrote: > > Tested on FriendlyARM Mini2440 > > > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > > --- > > .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ > > drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- > > include/linux/platform_data/mtd-nand-s3c2410.h | 1 + > > 3 files changed, 195 insertions(+), 5 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > > > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > new file mode 100644 > > index 000000000000..1c39f6cf483b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > @@ -0,0 +1,70 @@ > > +* Samsung S3C2410 and compatible NAND flash controller > > + > > +Required properties: > > +- compatible : The possible values are: > > + "samsung,s3c2410-nand" > > + "samsung,s3c2412-nand" > > + "samsung,s3c2440-nand" > > + "samsung,s3c6400-nand" > > +- reg : register's location and length. > > +- #address-cells, #size-cells : see nand.txt > > +- clocks : phandle to the nand controller clock > > +- clock-names : must contain "nand" > > Is there only one clock feeding into the NAND block? Is it actually > called "nand", or does the datasheet not give the input a nme? Yes, there is only one clock associated with the NAND controller. The clock is called "NAND Flash Controller" clock by the datasheet. > > > + > > +Optional properties: > > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > > +- samsung,twrph0 : active time for nWE/nOE > > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive > > If these are necessary, what units are these in? They are probably not necessary and will be removed as suggested by Boris Brezillon. > > > +- samsung,ignore_unset_ecc : boolean to ignore error when we have > > + 0xff,0xff,0xff read ECC, on the > > + assumption that it is an un-eccd page > > s/_/-/ in property names, please. OK. If I keep this property, I will change it to samsung,ignore-unset-ecc. > > That said, this name reads like policy rather than a HW description. > Why/when is this necessary to set? I'll evaluate if this property is really necessary, as suggested by Boris Brezillon. > > [...] > > > + of_property_read_u32(np, "samsung,tacls", &pdata->tacls); > > + of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0); > > + of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1); > > Are all values in the range [0, 0xffffffff] valid for these? If not, it > would be a good idea to sanity-check the values. OK. > > > + if (of_get_property(np, "samsung,ignore_unset_ecc", NULL)) > > + pdata->ignore_unset_ecc = 1; > > Use of_property_read_bool(); Right. > > [...] > > > + for_each_available_child_of_node(np, child) { > > + > > + sets->name = (char *)child->name; > > + sets->of_node = child; > > Strictly speaking, you should increment the child node refcount here, as > you maintain a reference to it and its name field. OK. > > > + sets->nr_chips = 1; > > + > > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > > + sets->disable_ecc = 1; > > + > > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > > + sets->flash_bbt = 1; > > Use of_property_read_bool(). OK. > > Thanks, > Mark.
On Mon, Sep 19, 2016 at 12:44:07PM +0200, Sylwester Nawrocki wrote: > On 09/17/2016 05:22 PM, Sergio Prado wrote: > > +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; > > + const struct of_device_id *of_id; > > + struct s3c2410_nand_set *sets; > > + > > + of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev); > > + if (!of_id) > > + return 1; > > + > > + devtype_data = of_id->data; > > You could make it a bit simpler with of_device_get_match_data(). I wasn't aware of this interface. I'll use it. Thanks! > > -- > Thanks, > Sylwester
On Sat, Sep 17, 2016 at 12:22:40PM -0300, Sergio Prado wrote: > Tested on FriendlyARM Mini2440 > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > --- > .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ > drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- > include/linux/platform_data/mtd-nand-s3c2410.h | 1 + > 3 files changed, 195 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > new file mode 100644 > index 000000000000..1c39f6cf483b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt > @@ -0,0 +1,70 @@ > +* Samsung S3C2410 and compatible NAND flash controller > + > +Required properties: > +- compatible : The possible values are: > + "samsung,s3c2410-nand" > + "samsung,s3c2412-nand" > + "samsung,s3c2440-nand" > + "samsung,s3c6400-nand" > +- reg : register's location and length. > +- #address-cells, #size-cells : see nand.txt > +- clocks : phandle to the nand controller clock > +- clock-names : must contain "nand" > + > +Optional properties: > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > +- samsung,twrph0 : active time for nWE/nOE > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive > +- samsung,ignore_unset_ecc : boolean to ignore error when we have Use '-', not '_'. > + 0xff,0xff,0xff read ECC, on the > + assumption that it is an un-eccd page > + > +Optional children nodes: > +Children nodes representing the available nand chips. s/Children/Child/ > + > +Optional children properties: > +- nand-ecc-mode : see nand.txt > +- nand-on-flash-bbt : see nand.txt > + > +Each children device node may optionally contain a 'partitions' sub-node, s/children/child/ > +which further contains sub-nodes describing the flash partition mapping. > +See partition.txt for more detail. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Boris, > > +Optional properties: > > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > > +- samsung,twrph0 : active time for nWE/nOE > > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive > > Can you try to extract these information from the nand_sdr_timings > struct instead of passing it in your DT? I've tested and the nand chip I'm working on is not ONFI or JEDEC compatible, so looks like it is not possible to extract the timing information from nand_sdr_timings. Am I right? > > + samsung,ignore_unset_ecc; > > Just discovered this ->ignore_unset_ecc property, and I don't > understand why it's here for... > Either you don't want to have ECC, and in this case you should set > NAND_ECC_NONE, or you want to have ECC calculated, and in this case, > the only valid situation where ECC bytes are 0xff is when the page is > erased. > > If I'm right, please fix the driver instead of adding this DT property. > If I'm wrong, could you explain in more detail when you have ECC bytes > set to 0xff? I think you are right but I am not an expert on flash devices and the MTD sub-system. The commit message of this code says "If a block's ecc field is all 0xff, then ignore the ECC correction. This is for systems where some of the blocks, such as the initial cramfs are written without ECC and need to be loaded on start.". Does it make sense? > > + for_each_available_child_of_node(np, child) { > > + > > + sets->name = (char *)child->name; > > + sets->of_node = child; > > + sets->nr_chips = 1; > > + > > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > > + sets->disable_ecc = 1; > > + > > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > > + sets->flash_bbt = 1; > > + > > These properties are automatically extracted in nand_scan_ident(), why > do you need to parse them twice? > You are right, there is no need to parse them twice. But taking a look at the code I found a problem. Right now enabling hardware ECC is done at compile time by enabling CONFIG_MTD_NAND_S3C2410_HWECC in the menuconfig. Looks like this config option should be removed so we can select ECC mode using nand-ecc-mode property in the device tree. But this would break current boards that are not using a device tree. So it would be necessary to add a ecc_mode field in the platform_data of these boards and set them all to the default ECC mode (NAND_ECC_SOFT). Is this the right approach? Thanks!
Hi Sergio, On Sun, 25 Sep 2016 14:42:57 -0300 Sergio Prado <sergio.prado@e-labworks.com> wrote: > Hi Boris, > > > > +Optional properties: > > > +- samsung,tacls : time for active CLE/ALE to nWE/nOE > > > +- samsung,twrph0 : active time for nWE/nOE > > > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive > > > > Can you try to extract these information from the nand_sdr_timings > > struct instead of passing it in your DT? > > I've tested and the nand chip I'm working on is not ONFI or JEDEC > compatible, so looks like it is not possible to extract the timing > information from nand_sdr_timings. Am I right? There's an default_onfi_timing_mode in the nand_flash_dev struct. You can define a full-id for your NAND chip in the nand-ids table if you want. Otherwise, this means you'll have to use ONFI timing mode 0 (which should work for all NANDs). Note that we've recently introduced a generic interface [1] to let NAND controllers configure the timings, and let the core select the best timings based on the information it has (ONFI, JEDEC or the nand-ids table). > > > > + samsung,ignore_unset_ecc; > > > > Just discovered this ->ignore_unset_ecc property, and I don't > > understand why it's here for... > > Either you don't want to have ECC, and in this case you should set > > NAND_ECC_NONE, or you want to have ECC calculated, and in this case, > > the only valid situation where ECC bytes are 0xff is when the page is > > erased. > > > > If I'm right, please fix the driver instead of adding this DT property. > > If I'm wrong, could you explain in more detail when you have ECC bytes > > set to 0xff? > > I think you are right but I am not an expert on flash devices and the > MTD sub-system. The commit message of this code says "If a block's ecc > field is all 0xff, then ignore the ECC correction. This is for systems > where some of the blocks, such as the initial cramfs are written without > ECC and need to be loaded on start.". Does it make sense? Well, no, it does not make any sense. What could have a sense is enabling ECC on some partitions, but not on others, but that's not supported right now (actually, I tried to add support for that once, but it was not accepted). Do you really need that? I mean, is the platform you're trying to convert to DT using this property in its platform_data objects? I'd recommend to drop this property until we figure what it's really used for. > > > > + for_each_available_child_of_node(np, child) { > > > + > > > + sets->name = (char *)child->name; > > > + sets->of_node = child; > > > + sets->nr_chips = 1; > > > + > > > + if (!of_property_match_string(child, "nand-ecc-mode", "none")) > > > + sets->disable_ecc = 1; > > > + > > > + if (of_get_property(child, "nand-on-flash-bbt", NULL)) > > > + sets->flash_bbt = 1; > > > + > > > > These properties are automatically extracted in nand_scan_ident(), why > > do you need to parse them twice? > > > > You are right, there is no need to parse them twice. But taking a look > at the code I found a problem. Right now enabling hardware ECC is done > at compile time by enabling CONFIG_MTD_NAND_S3C2410_HWECC in the > menuconfig. Looks like this config option should be removed so we can > select ECC mode using nand-ecc-mode property in the device tree. But > this would break current boards that are not using a device tree. So it > would be necessary to add a ecc_mode field in the platform_data of these > boards and set them all to the default ECC mode (NAND_ECC_SOFT). Is this > the right approach? It's the right approach, indeed. Thanks, Boris [1]http://www.spinics.net/lists/arm-kernel/msg532007.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt new file mode 100644 index 000000000000..1c39f6cf483b --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt @@ -0,0 +1,70 @@ +* Samsung S3C2410 and compatible NAND flash controller + +Required properties: +- compatible : The possible values are: + "samsung,s3c2410-nand" + "samsung,s3c2412-nand" + "samsung,s3c2440-nand" + "samsung,s3c6400-nand" +- reg : register's location and length. +- #address-cells, #size-cells : see nand.txt +- clocks : phandle to the nand controller clock +- clock-names : must contain "nand" + +Optional properties: +- samsung,tacls : time for active CLE/ALE to nWE/nOE +- samsung,twrph0 : active time for nWE/nOE +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive +- samsung,ignore_unset_ecc : boolean to ignore error when we have + 0xff,0xff,0xff read ECC, on the + assumption that it is an un-eccd page + +Optional children nodes: +Children nodes representing the available nand chips. + +Optional children properties: +- nand-ecc-mode : see nand.txt +- nand-on-flash-bbt : see nand.txt + +Each children device node may optionally contain a 'partitions' sub-node, +which further contains sub-nodes describing the flash partition mapping. +See partition.txt for more detail. + +Example: + +nand@4e000000 { + compatible = "samsung,s3c2440-nand"; + reg = <0x4e000000 0x40>; + + #address-cells = <1>; + #size-cells = <0>; + + clocks = <&clocks HCLK_NAND>; + clock-names = "nand"; + + samsung,tacls = <0>; + samsung,twrph0 = <25>; + samsung,twrph1 = <15>; + samsung,ignore_unset_ecc; + + nand@0 { + nand-ecc-mode = "soft"; + nand-on-flash-bbt; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "u-boot"; + reg = <0 0x040000>; + }; + + partition@40000 { + label = "kernel"; + reg = <0x040000 0x500000>; + }; + }; + }; +}; diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c index d9309cf0ce2e..ecbb9c9c1e9a 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,26 @@ struct s3c2410_nand_info { #endif }; +struct s3c24XX_nand_devtype_data { + enum s3c_cpu_type type; +}; + +struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = { + .type = TYPE_S3C2410, +}; + +struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = { + .type = TYPE_S3C2412, +}; + +struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = { + .type = TYPE_S3C2440, +}; + +struct s3c24XX_nand_devtype_data s3c6400_nand_devtype_data = { + .type = TYPE_S3C2412, +}; + /* conversion functions */ static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd) @@ -813,6 +835,8 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info, 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; @@ -947,6 +971,96 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info, } } +#ifdef CONFIG_OF_MTD +static const struct of_device_id s3c24xx_nand_dt_ids[] = { + { + .compatible = "samsung,s3c2410-nand", + .data = &s3c2410_nand_devtype_data, + }, { + .compatible = "samsung,s3c2412-nand", + .data = &s3c2412_nand_devtype_data, + }, { + .compatible = "samsung,s3c2440-nand", + .data = &s3c2440_nand_devtype_data, + }, { + .compatible = "samsung,s3c6400-nand", + .data = &s3c6400_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; + const struct of_device_id *of_id; + struct s3c2410_nand_set *sets; + + of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev); + if (!of_id) + return 1; + + devtype_data = of_id->data; + info->cpu_type = devtype_data->type; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + pdev->dev.platform_data = pdata; + + of_property_read_u32(np, "samsung,tacls", &pdata->tacls); + of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0); + of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1); + + if (of_get_property(np, "samsung,ignore_unset_ecc", NULL)) + pdata->ignore_unset_ecc = 1; + + 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; + + if (!of_property_match_string(child, "nand-ecc-mode", "none")) + sets->disable_ecc = 1; + + if (of_get_property(child, "nand-on-flash-bbt", NULL)) + sets->flash_bbt = 1; + + sets++; + } + + return 0; +} +#else +static int s3c24xx_nand_probe_dt(struct platform_device *pdev) +{ + return 1; +} +#endif + +static void 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; +} + /* s3c24xx_nand_probe * * called by device layer when it finds a device matching @@ -956,8 +1070,7 @@ static void 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; @@ -967,8 +1080,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; @@ -991,6 +1102,14 @@ static int s3c24xx_nand_probe(struct platform_device *pdev) s3c2410_nand_clk_set_state(info, CLOCK_ENABLE); + err = s3c24xx_nand_probe_dt(pdev); + if (err > 0) + s3c24xx_nand_probe_pdata(pdev); + else if (err < 0) + goto exit_error; + + plat = to_nand_plat(pdev); + /* allocate and map the resource */ /* currently we assume we have the one resource */ @@ -999,7 +1118,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)) { @@ -1156,6 +1274,7 @@ static struct platform_driver s3c24xx_nand_driver = { .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 c55e42ee57fa..9d20871e4bbd 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 {
Tested on FriendlyARM Mini2440 Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> --- .../devicetree/bindings/mtd/samsung-s3c2410.txt | 70 +++++++++++ drivers/mtd/nand/s3c2410.c | 129 ++++++++++++++++++++- include/linux/platform_data/mtd-nand-s3c2410.h | 1 + 3 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt