| Message ID | 20171015232049.rzz7p2mtpmyn452k@lenoch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 16/10/17 02:20, Ladislav Michl wrote: > Generic probe function can deal with OneNAND node, remove > now useless gpmc_probe_onenand_child function. > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > --- > drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- > 1 file changed, 69 insertions(+), 42 deletions(-) > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > index f2aef0b87bc6..fc7c3c2a0b04 100644 > --- a/drivers/memory/omap-gpmc.c > +++ b/drivers/memory/omap-gpmc.c > @@ -1921,52 +1921,22 @@ 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_ONENAND) > -static int gpmc_probe_onenand_child(struct platform_device *pdev, > - struct device_node *child) > -{ > - u32 val; > - struct omap_onenand_platform_data *gpmc_onenand_data; > - > - if (of_property_read_u32(child, "reg", &val) < 0) { > - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", > - child); > - return -ENODEV; > - } > - > - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), > - GFP_KERNEL); > - if (!gpmc_onenand_data) > - return -ENOMEM; > - > - gpmc_onenand_data->cs = val; > - gpmc_onenand_data->of_node = child; > - gpmc_onenand_data->dma_channel = -1; > - > - if (!of_property_read_u32(child, "dma-channel", &val)) > - gpmc_onenand_data->dma_channel = val; > - > - return gpmc_onenand_init(gpmc_onenand_data); > -} > -#else > -static int gpmc_probe_onenand_child(struct platform_device *pdev, > - struct device_node *child) > -{ > - return 0; > -} > -#endif > +extern int > +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); > > /** > - * gpmc_probe_generic_child - configures the gpmc for a child device > + * gpmc_probe_child - configures the gpmc for a child device Let's not do this name change it is not really required. > * @pdev: pointer to gpmc platform device > * @child: pointer to device-tree node for child device > * > * Allocates and configures a GPMC chip-select for a child device. > * Returns 0 on success and appropriate negative error code on failure. > */ > -static int gpmc_probe_generic_child(struct platform_device *pdev, > +static int gpmc_probe_child(struct platform_device *pdev, > struct device_node *child) > { > + struct omap_onenand_platform_data *gpmc_onenand_data; > + struct platform_device *child_dev; > struct gpmc_settings gpmc_s; > struct gpmc_timings gpmc_t; > struct resource res; > @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > } > } > > + if (of_node_cmp(child->name, "onenand") == 0) { > + /* > + * Warn about older DT blobs with no compatible property > + * and fix them for now > + */ > + if (!of_property_read_bool(child, "compatible")) { > + struct property *prop; > + > + dev_warn(&pdev->dev, > + "Incompatible OneNAND node, fixing."); > + > + prop = kzalloc(sizeof(*prop), GFP_KERNEL); > + if (!prop) { > + ret = -ENOMEM; > + goto err; > + } > + > + prop->name = "compatible"; > + /* See FIXME about device-tree migration bellow */ > + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? > + "ti,omap3-onenand" : "ti,omap2-onenand"; > + prop->length = 14; > + > + of_update_property(child, prop); > + } I don't want to fixup broken DTs like this as they will go silently without being fixed. Let's instead just error out after the dev_warn(). This will force us to fix all DT nodes. > + } > + > if (of_device_is_compatible(child, "ti,omap2-nand")) { > /* NAND specific setup */ > val = 8; > @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > /* disable write protect */ > gpmc_configure(GPMC_CONFIG_WP, 0); > gpmc_s.device_nand = true; > + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || > + of_device_is_compatible(child, "ti,omap3-onenand")) { Could just be if() instead of else if() as we don't have any dependency on nand. > + gpmc_onenand_data = devm_kzalloc(&pdev->dev, > + sizeof(*gpmc_onenand_data), > + GFP_KERNEL); > + if (!gpmc_onenand_data) { > + ret = -ENOMEM; > + goto err; > + } > + > + gpmc_onenand_data->cs = cs; > + gpmc_onenand_data->of_node = child; > + if (!of_property_read_u32(child, "dma-channel", &val)) > + gpmc_onenand_data->dma_channel = val; > + else > + gpmc_onenand_data->dma_channel = -1; > + > + ret = gpmc_omap_onenand_init(gpmc_onenand_data); > + if (ret) > + goto err; this gpmc_onenand_data should go away if we squash patch 10 here. > + > + /* OneNAND driver handles timings on its own */ > + gpmc_cs_enable_mem(cs); > + goto no_timings; Actually ASYNC timings should be handled here just as the other children. Only SYNC timings will be triggered by the OneNAND driver. > } else { > ret = of_property_read_u32(child, "bank-width", > &gpmc_s.device_width); > @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > no_timings: > > /* create platform device, NULL on error or when disabled */ > - if (!of_platform_device_create(child, NULL, &pdev->dev)) > + child_dev = of_platform_device_create(child, NULL, &pdev->dev); > + if (!child_dev) > goto err_child_fail; > > + /* Use platform data until OneNAND driver is DT aware */ > + if (gpmc_onenand_data) { > + child_dev->name = "omap2-onenand"; > + child_dev->dev.platform_data = gpmc_onenand_data; > + ret = platform_device_add_resources(child_dev, &res, 1); > + if (ret) > + goto err_child_fail; > + } > + This bit will go away on squashing patch 10. > /* is child a common bus? */ > if (of_match_node(of_default_bus_match_table, child)) > /* create children and other common bus children */ > @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) > if (!child->name) > continue; > > - if (of_node_cmp(child->name, "onenand") == 0) > - ret = gpmc_probe_onenand_child(pdev, child); > - else > - ret = gpmc_probe_generic_child(pdev, child); > - > + ret = gpmc_probe_child(pdev, child); > if (ret) { > dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", > child->name, ret); >
On Tue, Oct 17, 2017 at 11:46:51AM +0300, Roger Quadros wrote: > Hi, > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 16/10/17 02:20, Ladislav Michl wrote: > > Generic probe function can deal with OneNAND node, remove > > now useless gpmc_probe_onenand_child function. > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > --- > > drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- > > 1 file changed, 69 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > > index f2aef0b87bc6..fc7c3c2a0b04 100644 > > --- a/drivers/memory/omap-gpmc.c > > +++ b/drivers/memory/omap-gpmc.c > > @@ -1921,52 +1921,22 @@ 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_ONENAND) > > -static int gpmc_probe_onenand_child(struct platform_device *pdev, > > - struct device_node *child) > > -{ > > - u32 val; > > - struct omap_onenand_platform_data *gpmc_onenand_data; > > - > > - if (of_property_read_u32(child, "reg", &val) < 0) { > > - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", > > - child); > > - return -ENODEV; > > - } > > - > > - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), > > - GFP_KERNEL); > > - if (!gpmc_onenand_data) > > - return -ENOMEM; > > - > > - gpmc_onenand_data->cs = val; > > - gpmc_onenand_data->of_node = child; > > - gpmc_onenand_data->dma_channel = -1; > > - > > - if (!of_property_read_u32(child, "dma-channel", &val)) > > - gpmc_onenand_data->dma_channel = val; > > - > > - return gpmc_onenand_init(gpmc_onenand_data); > > -} > > -#else > > -static int gpmc_probe_onenand_child(struct platform_device *pdev, > > - struct device_node *child) > > -{ > > - return 0; > > -} > > -#endif > > +extern int > > +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); > > > > /** > > - * gpmc_probe_generic_child - configures the gpmc for a child device > > + * gpmc_probe_child - configures the gpmc for a child device > > Let's not do this name change it is not really required. But it is nice, right? :) Will drop that change. > > * @pdev: pointer to gpmc platform device > > * @child: pointer to device-tree node for child device > > * > > * Allocates and configures a GPMC chip-select for a child device. > > * Returns 0 on success and appropriate negative error code on failure. > > */ > > -static int gpmc_probe_generic_child(struct platform_device *pdev, > > +static int gpmc_probe_child(struct platform_device *pdev, > > struct device_node *child) > > { > > + struct omap_onenand_platform_data *gpmc_onenand_data; > > + struct platform_device *child_dev; > > struct gpmc_settings gpmc_s; > > struct gpmc_timings gpmc_t; > > struct resource res; > > @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > > } > > } > > > > + if (of_node_cmp(child->name, "onenand") == 0) { > > + /* > > + * Warn about older DT blobs with no compatible property > > + * and fix them for now > > + */ > > + if (!of_property_read_bool(child, "compatible")) { > > + struct property *prop; > > + > > + dev_warn(&pdev->dev, > > + "Incompatible OneNAND node, fixing."); > > + > > + prop = kzalloc(sizeof(*prop), GFP_KERNEL); > > + if (!prop) { > > + ret = -ENOMEM; > > + goto err; > > + } > > + > > + prop->name = "compatible"; > > + /* See FIXME about device-tree migration bellow */ > > + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? > > + "ti,omap3-onenand" : "ti,omap2-onenand"; > > + prop->length = 14; > > + > > + of_update_property(child, prop); > > + } > > I don't want to fixup broken DTs like this as they will go silently without being fixed. > > Let's instead just error out after the dev_warn(). > This will force us to fix all DT nodes. Is it really right thing to do? A lot of people scream loudly when you break their boot. > > + } > > + > > if (of_device_is_compatible(child, "ti,omap2-nand")) { > > /* NAND specific setup */ > > val = 8; > > @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > > /* disable write protect */ > > gpmc_configure(GPMC_CONFIG_WP, 0); > > gpmc_s.device_nand = true; > > + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || > > + of_device_is_compatible(child, "ti,omap3-onenand")) { > > Could just be if() instead of else if() as we don't have any dependency on nand. It could, but as node is eithrt nand or onenand, this saves few cycles. > > + gpmc_onenand_data = devm_kzalloc(&pdev->dev, > > + sizeof(*gpmc_onenand_data), > > + GFP_KERNEL); > > + if (!gpmc_onenand_data) { > > + ret = -ENOMEM; > > + goto err; > > + } > > + > > + gpmc_onenand_data->cs = cs; > > + gpmc_onenand_data->of_node = child; > > + if (!of_property_read_u32(child, "dma-channel", &val)) > > + gpmc_onenand_data->dma_channel = val; > > + else > > + gpmc_onenand_data->dma_channel = -1; > > + > > + ret = gpmc_omap_onenand_init(gpmc_onenand_data); > > + if (ret) > > + goto err; > > this gpmc_onenand_data should go away if we squash patch 10 here. But driver will stop working then as setup function will be NULL. > > + > > + /* OneNAND driver handles timings on its own */ > > + gpmc_cs_enable_mem(cs); > > + goto no_timings; > > Actually ASYNC timings should be handled here just as the other children. > Only SYNC timings will be triggered by the OneNAND driver. It is handled here, it just cannot be done in one patch as MTD driver needs to be modified. > > } else { > > ret = of_property_read_u32(child, "bank-width", > > &gpmc_s.device_width); > > @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > > no_timings: > > > > /* create platform device, NULL on error or when disabled */ > > - if (!of_platform_device_create(child, NULL, &pdev->dev)) > > + child_dev = of_platform_device_create(child, NULL, &pdev->dev); > > + if (!child_dev) > > goto err_child_fail; > > > > + /* Use platform data until OneNAND driver is DT aware */ > > + if (gpmc_onenand_data) { > > + child_dev->name = "omap2-onenand"; > > + child_dev->dev.platform_data = gpmc_onenand_data; > > + ret = platform_device_add_resources(child_dev, &res, 1); > > + if (ret) > > + goto err_child_fail; > > + } > > + > > This bit will go away on squashing patch 10. I'm not sure how to do this and merge changes using two separate trees. > > /* is child a common bus? */ > > if (of_match_node(of_default_bus_match_table, child)) > > /* create children and other common bus children */ > > @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) > > if (!child->name) > > continue; > > > > - if (of_node_cmp(child->name, "onenand") == 0) > > - ret = gpmc_probe_onenand_child(pdev, child); > > - else > > - ret = gpmc_probe_generic_child(pdev, child); > > - > > + ret = gpmc_probe_child(pdev, child); > > if (ret) { > > dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", > > child->name, ret); > > > > -- > cheers, > -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 17/10/17 12:04, Ladislav Michl wrote: > On Tue, Oct 17, 2017 at 11:46:51AM +0300, Roger Quadros wrote: >> Hi, >> >> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> >> On 16/10/17 02:20, Ladislav Michl wrote: >>> Generic probe function can deal with OneNAND node, remove >>> now useless gpmc_probe_onenand_child function. >>> >>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org> >>> --- >>> drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- >>> 1 file changed, 69 insertions(+), 42 deletions(-) >>> >>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >>> index f2aef0b87bc6..fc7c3c2a0b04 100644 >>> --- a/drivers/memory/omap-gpmc.c >>> +++ b/drivers/memory/omap-gpmc.c >>> @@ -1921,52 +1921,22 @@ 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_ONENAND) >>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, >>> - struct device_node *child) >>> -{ >>> - u32 val; >>> - struct omap_onenand_platform_data *gpmc_onenand_data; >>> - >>> - if (of_property_read_u32(child, "reg", &val) < 0) { >>> - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >>> - child); >>> - return -ENODEV; >>> - } >>> - >>> - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), >>> - GFP_KERNEL); >>> - if (!gpmc_onenand_data) >>> - return -ENOMEM; >>> - >>> - gpmc_onenand_data->cs = val; >>> - gpmc_onenand_data->of_node = child; >>> - gpmc_onenand_data->dma_channel = -1; >>> - >>> - if (!of_property_read_u32(child, "dma-channel", &val)) >>> - gpmc_onenand_data->dma_channel = val; >>> - >>> - return gpmc_onenand_init(gpmc_onenand_data); >>> -} >>> -#else >>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, >>> - struct device_node *child) >>> -{ >>> - return 0; >>> -} >>> -#endif >>> +extern int >>> +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); >>> >>> /** >>> - * gpmc_probe_generic_child - configures the gpmc for a child device >>> + * gpmc_probe_child - configures the gpmc for a child device >> >> Let's not do this name change it is not really required. > > But it is nice, right? :) Will drop that change.> :) >>> * @pdev: pointer to gpmc platform device >>> * @child: pointer to device-tree node for child device >>> * >>> * Allocates and configures a GPMC chip-select for a child device. >>> * Returns 0 on success and appropriate negative error code on failure. >>> */ >>> -static int gpmc_probe_generic_child(struct platform_device *pdev, >>> +static int gpmc_probe_child(struct platform_device *pdev, >>> struct device_node *child) >>> { >>> + struct omap_onenand_platform_data *gpmc_onenand_data; >>> + struct platform_device *child_dev; >>> struct gpmc_settings gpmc_s; >>> struct gpmc_timings gpmc_t; >>> struct resource res; >>> @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>> } >>> } >>> >>> + if (of_node_cmp(child->name, "onenand") == 0) { >>> + /* >>> + * Warn about older DT blobs with no compatible property >>> + * and fix them for now >>> + */ >>> + if (!of_property_read_bool(child, "compatible")) { >>> + struct property *prop; >>> + >>> + dev_warn(&pdev->dev, >>> + "Incompatible OneNAND node, fixing."); >>> + >>> + prop = kzalloc(sizeof(*prop), GFP_KERNEL); >>> + if (!prop) { >>> + ret = -ENOMEM; >>> + goto err; >>> + } >>> + >>> + prop->name = "compatible"; >>> + /* See FIXME about device-tree migration bellow */ >>> + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? >>> + "ti,omap3-onenand" : "ti,omap2-onenand"; >>> + prop->length = 14; >>> + >>> + of_update_property(child, prop); >>> + } >> >> I don't want to fixup broken DTs like this as they will go silently without being fixed. >> >> Let's instead just error out after the dev_warn(). >> This will force us to fix all DT nodes. > > Is it really right thing to do? A lot of people scream loudly when you break > their boot. > And it is our job to fix it if it does :). IMO this is way better than things going unfixed for years together. >>> + } >>> + >>> if (of_device_is_compatible(child, "ti,omap2-nand")) { >>> /* NAND specific setup */ >>> val = 8; >>> @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>> /* disable write protect */ >>> gpmc_configure(GPMC_CONFIG_WP, 0); >>> gpmc_s.device_nand = true; >>> + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || >>> + of_device_is_compatible(child, "ti,omap3-onenand")) { >> >> Could just be if() instead of else if() as we don't have any dependency on nand. > > It could, but as node is eithrt nand or onenand, this saves few cycles. > I don't think the few cycles matter that much. I'd choose an option which has better readability. Choice is yours :) >>> + gpmc_onenand_data = devm_kzalloc(&pdev->dev, >>> + sizeof(*gpmc_onenand_data), >>> + GFP_KERNEL); >>> + if (!gpmc_onenand_data) { >>> + ret = -ENOMEM; >>> + goto err; >>> + } >>> + >>> + gpmc_onenand_data->cs = cs; >>> + gpmc_onenand_data->of_node = child; >>> + if (!of_property_read_u32(child, "dma-channel", &val)) >>> + gpmc_onenand_data->dma_channel = val; >>> + else >>> + gpmc_onenand_data->dma_channel = -1; >>> + >>> + ret = gpmc_omap_onenand_init(gpmc_onenand_data); >>> + if (ret) >>> + goto err; >> >> this gpmc_onenand_data should go away if we squash patch 10 here. > > But driver will stop working then as setup function will be NULL. > Which is fine. It will start working again when mtd parts get merged via mtd tree. >>> + >>> + /* OneNAND driver handles timings on its own */ >>> + gpmc_cs_enable_mem(cs); >>> + goto no_timings; >> >> Actually ASYNC timings should be handled here just as the other children. >> Only SYNC timings will be triggered by the OneNAND driver. > > It is handled here, it just cannot be done in one patch as MTD driver > needs to be modified. > >>> } else { >>> ret = of_property_read_u32(child, "bank-width", >>> &gpmc_s.device_width); >>> @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>> no_timings: >>> >>> /* create platform device, NULL on error or when disabled */ >>> - if (!of_platform_device_create(child, NULL, &pdev->dev)) >>> + child_dev = of_platform_device_create(child, NULL, &pdev->dev); >>> + if (!child_dev) >>> goto err_child_fail; >>> >>> + /* Use platform data until OneNAND driver is DT aware */ >>> + if (gpmc_onenand_data) { >>> + child_dev->name = "omap2-onenand"; >>> + child_dev->dev.platform_data = gpmc_onenand_data; >>> + ret = platform_device_add_resources(child_dev, &res, 1); >>> + if (ret) >>> + goto err_child_fail; >>> + } >>> + >> >> This bit will go away on squashing patch 10. > > I'm not sure how to do this and merge changes using two separate trees. - make sure build doesn't break in each tree. I'm OK with oneNAND functionality broken in individual trees for this case. As there is no other way out and make your life easier. - make sure everything works after trees are merged. > >>> /* is child a common bus? */ >>> if (of_match_node(of_default_bus_match_table, child)) >>> /* create children and other common bus children */ >>> @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) >>> if (!child->name) >>> continue; >>> >>> - if (of_node_cmp(child->name, "onenand") == 0) >>> - ret = gpmc_probe_onenand_child(pdev, child); >>> - else >>> - ret = gpmc_probe_generic_child(pdev, child); >>> - >>> + ret = gpmc_probe_child(pdev, child); >>> if (ret) { >>> dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", >>> child->name, ret); >>> >>
On Tue, Oct 17, 2017 at 12:12:15PM +0300, Roger Quadros wrote: > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 17/10/17 12:04, Ladislav Michl wrote: > > On Tue, Oct 17, 2017 at 11:46:51AM +0300, Roger Quadros wrote: > >> Hi, > >> > >> > >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > >> > >> On 16/10/17 02:20, Ladislav Michl wrote: > >>> Generic probe function can deal with OneNAND node, remove > >>> now useless gpmc_probe_onenand_child function. > >>> > >>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > >>> --- > >>> drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- > >>> 1 file changed, 69 insertions(+), 42 deletions(-) > >>> > >>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > >>> index f2aef0b87bc6..fc7c3c2a0b04 100644 > >>> --- a/drivers/memory/omap-gpmc.c > >>> +++ b/drivers/memory/omap-gpmc.c > >>> @@ -1921,52 +1921,22 @@ 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_ONENAND) > >>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, > >>> - struct device_node *child) > >>> -{ > >>> - u32 val; > >>> - struct omap_onenand_platform_data *gpmc_onenand_data; > >>> - > >>> - if (of_property_read_u32(child, "reg", &val) < 0) { > >>> - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", > >>> - child); > >>> - return -ENODEV; > >>> - } > >>> - > >>> - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), > >>> - GFP_KERNEL); > >>> - if (!gpmc_onenand_data) > >>> - return -ENOMEM; > >>> - > >>> - gpmc_onenand_data->cs = val; > >>> - gpmc_onenand_data->of_node = child; > >>> - gpmc_onenand_data->dma_channel = -1; > >>> - > >>> - if (!of_property_read_u32(child, "dma-channel", &val)) > >>> - gpmc_onenand_data->dma_channel = val; > >>> - > >>> - return gpmc_onenand_init(gpmc_onenand_data); > >>> -} > >>> -#else > >>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, > >>> - struct device_node *child) > >>> -{ > >>> - return 0; > >>> -} > >>> -#endif > >>> +extern int > >>> +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); > >>> > >>> /** > >>> - * gpmc_probe_generic_child - configures the gpmc for a child device > >>> + * gpmc_probe_child - configures the gpmc for a child device > >> > >> Let's not do this name change it is not really required. > > > > But it is nice, right? :) Will drop that change.> > > :) > > >>> * @pdev: pointer to gpmc platform device > >>> * @child: pointer to device-tree node for child device > >>> * > >>> * Allocates and configures a GPMC chip-select for a child device. > >>> * Returns 0 on success and appropriate negative error code on failure. > >>> */ > >>> -static int gpmc_probe_generic_child(struct platform_device *pdev, > >>> +static int gpmc_probe_child(struct platform_device *pdev, > >>> struct device_node *child) > >>> { > >>> + struct omap_onenand_platform_data *gpmc_onenand_data; > >>> + struct platform_device *child_dev; > >>> struct gpmc_settings gpmc_s; > >>> struct gpmc_timings gpmc_t; > >>> struct resource res; > >>> @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > >>> } > >>> } > >>> > >>> + if (of_node_cmp(child->name, "onenand") == 0) { > >>> + /* > >>> + * Warn about older DT blobs with no compatible property > >>> + * and fix them for now > >>> + */ > >>> + if (!of_property_read_bool(child, "compatible")) { > >>> + struct property *prop; > >>> + > >>> + dev_warn(&pdev->dev, > >>> + "Incompatible OneNAND node, fixing."); > >>> + > >>> + prop = kzalloc(sizeof(*prop), GFP_KERNEL); > >>> + if (!prop) { > >>> + ret = -ENOMEM; > >>> + goto err; > >>> + } > >>> + > >>> + prop->name = "compatible"; > >>> + /* See FIXME about device-tree migration bellow */ > >>> + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? > >>> + "ti,omap3-onenand" : "ti,omap2-onenand"; > >>> + prop->length = 14; > >>> + > >>> + of_update_property(child, prop); > >>> + } > >> > >> I don't want to fixup broken DTs like this as they will go silently without being fixed. > >> > >> Let's instead just error out after the dev_warn(). > >> This will force us to fix all DT nodes. > > > > Is it really right thing to do? A lot of people scream loudly when you break > > their boot. > > > And it is our job to fix it if it does :). Well, we can't really fix this. DT is expected to work for various kernels long term. At least that's what I have read long time ago - it is part of kernel ABI. I have many OMAP3 devices with OneNAND in field. Those are booting in Falcon mode: https://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf FDT blob is updated only when servicing device (full reflash). In this case full U-Boot is run and FDT is modified - RAM size, MTD partitioning, SoC type, etc. is stored in DT and flashed. U-Boot SPL the just reads it and passes to kernel. Now this approach means I cannot do remote kernel upgrade as it will not boot. I'd have to write utility to fix device tree before updating and I do not think I'm alone using similar approach. > IMO this is way better than things going unfixed for years together. I'd say print annoying warning is enough. > >>> + } > >>> + > >>> if (of_device_is_compatible(child, "ti,omap2-nand")) { > >>> /* NAND specific setup */ > >>> val = 8; > >>> @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > >>> /* disable write protect */ > >>> gpmc_configure(GPMC_CONFIG_WP, 0); > >>> gpmc_s.device_nand = true; > >>> + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || > >>> + of_device_is_compatible(child, "ti,omap3-onenand")) { > >> > >> Could just be if() instead of else if() as we don't have any dependency on nand. > > > > It could, but as node is eithrt nand or onenand, this saves few cycles. > > > > I don't think the few cycles matter that much. I'd choose an option which > has better readability. Choice is yours :) This few cycles/bytes approach lead to unbootable kernel during past 15 years on my omap1 devices ;-) > >>> + gpmc_onenand_data = devm_kzalloc(&pdev->dev, > >>> + sizeof(*gpmc_onenand_data), > >>> + GFP_KERNEL); > >>> + if (!gpmc_onenand_data) { > >>> + ret = -ENOMEM; > >>> + goto err; > >>> + } > >>> + > >>> + gpmc_onenand_data->cs = cs; > >>> + gpmc_onenand_data->of_node = child; > >>> + if (!of_property_read_u32(child, "dma-channel", &val)) > >>> + gpmc_onenand_data->dma_channel = val; > >>> + else > >>> + gpmc_onenand_data->dma_channel = -1; > >>> + > >>> + ret = gpmc_omap_onenand_init(gpmc_onenand_data); > >>> + if (ret) > >>> + goto err; > >> > >> this gpmc_onenand_data should go away if we squash patch 10 here. > > > > But driver will stop working then as setup function will be NULL. > > > Which is fine. It will start working again when mtd parts get merged via mtd tree. > > >>> + > >>> + /* OneNAND driver handles timings on its own */ > >>> + gpmc_cs_enable_mem(cs); > >>> + goto no_timings; > >> > >> Actually ASYNC timings should be handled here just as the other children. > >> Only SYNC timings will be triggered by the OneNAND driver. > > > > It is handled here, it just cannot be done in one patch as MTD driver > > needs to be modified. > > > >>> } else { > >>> ret = of_property_read_u32(child, "bank-width", > >>> &gpmc_s.device_width); > >>> @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > >>> no_timings: > >>> > >>> /* create platform device, NULL on error or when disabled */ > >>> - if (!of_platform_device_create(child, NULL, &pdev->dev)) > >>> + child_dev = of_platform_device_create(child, NULL, &pdev->dev); > >>> + if (!child_dev) > >>> goto err_child_fail; > >>> > >>> + /* Use platform data until OneNAND driver is DT aware */ > >>> + if (gpmc_onenand_data) { > >>> + child_dev->name = "omap2-onenand"; > >>> + child_dev->dev.platform_data = gpmc_onenand_data; > >>> + ret = platform_device_add_resources(child_dev, &res, 1); > >>> + if (ret) > >>> + goto err_child_fail; > >>> + } > >>> + > >> > >> This bit will go away on squashing patch 10. > > > > I'm not sure how to do this and merge changes using two separate trees. > > - make sure build doesn't break in each tree. I'm OK with oneNAND functionality broken in > individual trees for this case. As there is no other way out and make your life easier. > - make sure everything works after trees are merged. > > > >>> /* is child a common bus? */ > >>> if (of_match_node(of_default_bus_match_table, child)) > >>> /* create children and other common bus children */ > >>> @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) > >>> if (!child->name) > >>> continue; > >>> > >>> - if (of_node_cmp(child->name, "onenand") == 0) > >>> - ret = gpmc_probe_onenand_child(pdev, child); > >>> - else > >>> - ret = gpmc_probe_generic_child(pdev, child); > >>> - > >>> + ret = gpmc_probe_child(pdev, child); > >>> if (ret) { > >>> dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", > >>> child->name, ret); > >>> > >> > > -- > cheers, > -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 17/10/17 12:46, Ladislav Michl wrote: > On Tue, Oct 17, 2017 at 12:12:15PM +0300, Roger Quadros wrote: >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> >> On 17/10/17 12:04, Ladislav Michl wrote: >>> On Tue, Oct 17, 2017 at 11:46:51AM +0300, Roger Quadros wrote: >>>> Hi, >>>> >>>> >>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >>>> >>>> On 16/10/17 02:20, Ladislav Michl wrote: >>>>> Generic probe function can deal with OneNAND node, remove >>>>> now useless gpmc_probe_onenand_child function. >>>>> >>>>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org> >>>>> --- >>>>> drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- >>>>> 1 file changed, 69 insertions(+), 42 deletions(-) >>>>> >>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c >>>>> index f2aef0b87bc6..fc7c3c2a0b04 100644 >>>>> --- a/drivers/memory/omap-gpmc.c >>>>> +++ b/drivers/memory/omap-gpmc.c >>>>> @@ -1921,52 +1921,22 @@ 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_ONENAND) >>>>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, >>>>> - struct device_node *child) >>>>> -{ >>>>> - u32 val; >>>>> - struct omap_onenand_platform_data *gpmc_onenand_data; >>>>> - >>>>> - if (of_property_read_u32(child, "reg", &val) < 0) { >>>>> - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", >>>>> - child); >>>>> - return -ENODEV; >>>>> - } >>>>> - >>>>> - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), >>>>> - GFP_KERNEL); >>>>> - if (!gpmc_onenand_data) >>>>> - return -ENOMEM; >>>>> - >>>>> - gpmc_onenand_data->cs = val; >>>>> - gpmc_onenand_data->of_node = child; >>>>> - gpmc_onenand_data->dma_channel = -1; >>>>> - >>>>> - if (!of_property_read_u32(child, "dma-channel", &val)) >>>>> - gpmc_onenand_data->dma_channel = val; >>>>> - >>>>> - return gpmc_onenand_init(gpmc_onenand_data); >>>>> -} >>>>> -#else >>>>> -static int gpmc_probe_onenand_child(struct platform_device *pdev, >>>>> - struct device_node *child) >>>>> -{ >>>>> - return 0; >>>>> -} >>>>> -#endif >>>>> +extern int >>>>> +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); >>>>> >>>>> /** >>>>> - * gpmc_probe_generic_child - configures the gpmc for a child device >>>>> + * gpmc_probe_child - configures the gpmc for a child device >>>> >>>> Let's not do this name change it is not really required. >>> >>> But it is nice, right? :) Will drop that change.> >> >> :) >> >>>>> * @pdev: pointer to gpmc platform device >>>>> * @child: pointer to device-tree node for child device >>>>> * >>>>> * Allocates and configures a GPMC chip-select for a child device. >>>>> * Returns 0 on success and appropriate negative error code on failure. >>>>> */ >>>>> -static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> +static int gpmc_probe_child(struct platform_device *pdev, >>>>> struct device_node *child) >>>>> { >>>>> + struct omap_onenand_platform_data *gpmc_onenand_data; >>>>> + struct platform_device *child_dev; >>>>> struct gpmc_settings gpmc_s; >>>>> struct gpmc_timings gpmc_t; >>>>> struct resource res; >>>>> @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> } >>>>> } >>>>> >>>>> + if (of_node_cmp(child->name, "onenand") == 0) { >>>>> + /* >>>>> + * Warn about older DT blobs with no compatible property >>>>> + * and fix them for now >>>>> + */ >>>>> + if (!of_property_read_bool(child, "compatible")) { >>>>> + struct property *prop; >>>>> + >>>>> + dev_warn(&pdev->dev, >>>>> + "Incompatible OneNAND node, fixing."); >>>>> + >>>>> + prop = kzalloc(sizeof(*prop), GFP_KERNEL); >>>>> + if (!prop) { >>>>> + ret = -ENOMEM; >>>>> + goto err; >>>>> + } >>>>> + >>>>> + prop->name = "compatible"; >>>>> + /* See FIXME about device-tree migration bellow */ >>>>> + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? >>>>> + "ti,omap3-onenand" : "ti,omap2-onenand"; >>>>> + prop->length = 14; >>>>> + >>>>> + of_update_property(child, prop); >>>>> + } >>>> >>>> I don't want to fixup broken DTs like this as they will go silently without being fixed. >>>> >>>> Let's instead just error out after the dev_warn(). >>>> This will force us to fix all DT nodes. >>> >>> Is it really right thing to do? A lot of people scream loudly when you break >>> their boot. >>> >> And it is our job to fix it if it does :). > > Well, we can't really fix this. DT is expected to work for various kernels > long term. At least that's what I have read long time ago - it is part of > kernel ABI. But gpmc,onenand doesn't even have a compatible ID. So I'm not sure if it is a qualified ABI yet. > > I have many OMAP3 devices with OneNAND in field. > Those are booting in Falcon mode: > https://www.denx.de/wiki/pub/U-Boot/MiniSummitELCE2013/2013-ELCE-U-Boot-Falcon-Boot.pdf > FDT blob is updated only when servicing device (full reflash). In this > case full U-Boot is run and FDT is modified - RAM size, MTD partitioning, > SoC type, etc. is stored in DT and flashed. U-Boot SPL the just reads it > and passes to kernel. > > Now this approach means I cannot do remote kernel upgrade as it will not > boot. I'd have to write utility to fix device tree before updating and > I do not think I'm alone using similar approach. Oh, OK. > >> IMO this is way better than things going unfixed for years together. > > I'd say print annoying warning is enough. Maintaining backward compatibility will only work if we leave other parts of the node as it is. But what timings/settings are used in DT in the devices you use in the field? Do they provide ASYNC timings or SYNC timings? is sync-read/sync-write set? is sync-clk-ls set? Are the timings self sufficient or they still need patching up by driver. Maybe you can carry the DT on the fly patch up code in your customer support tree? > >>>>> + } >>>>> + >>>>> if (of_device_is_compatible(child, "ti,omap2-nand")) { >>>>> /* NAND specific setup */ >>>>> val = 8; >>>>> @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> /* disable write protect */ >>>>> gpmc_configure(GPMC_CONFIG_WP, 0); >>>>> gpmc_s.device_nand = true; >>>>> + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || >>>>> + of_device_is_compatible(child, "ti,omap3-onenand")) { >>>> >>>> Could just be if() instead of else if() as we don't have any dependency on nand. >>> >>> It could, but as node is eithrt nand or onenand, this saves few cycles. >>> >> >> I don't think the few cycles matter that much. I'd choose an option which >> has better readability. Choice is yours :) > > This few cycles/bytes approach lead to unbootable kernel during past 15 years > on my omap1 devices ;-) > >>>>> + gpmc_onenand_data = devm_kzalloc(&pdev->dev, >>>>> + sizeof(*gpmc_onenand_data), >>>>> + GFP_KERNEL); >>>>> + if (!gpmc_onenand_data) { >>>>> + ret = -ENOMEM; >>>>> + goto err; >>>>> + } >>>>> + >>>>> + gpmc_onenand_data->cs = cs; >>>>> + gpmc_onenand_data->of_node = child; >>>>> + if (!of_property_read_u32(child, "dma-channel", &val)) >>>>> + gpmc_onenand_data->dma_channel = val; >>>>> + else >>>>> + gpmc_onenand_data->dma_channel = -1; >>>>> + >>>>> + ret = gpmc_omap_onenand_init(gpmc_onenand_data); >>>>> + if (ret) >>>>> + goto err; >>>> >>>> this gpmc_onenand_data should go away if we squash patch 10 here. >>> >>> But driver will stop working then as setup function will be NULL. >>> >> Which is fine. It will start working again when mtd parts get merged via mtd tree. >> >>>>> + >>>>> + /* OneNAND driver handles timings on its own */ >>>>> + gpmc_cs_enable_mem(cs); >>>>> + goto no_timings; >>>> >>>> Actually ASYNC timings should be handled here just as the other children. >>>> Only SYNC timings will be triggered by the OneNAND driver. >>> >>> It is handled here, it just cannot be done in one patch as MTD driver >>> needs to be modified. >>> >>>>> } else { >>>>> ret = of_property_read_u32(child, "bank-width", >>>>> &gpmc_s.device_width); >>>>> @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, >>>>> no_timings: >>>>> >>>>> /* create platform device, NULL on error or when disabled */ >>>>> - if (!of_platform_device_create(child, NULL, &pdev->dev)) >>>>> + child_dev = of_platform_device_create(child, NULL, &pdev->dev); >>>>> + if (!child_dev) >>>>> goto err_child_fail; >>>>> >>>>> + /* Use platform data until OneNAND driver is DT aware */ >>>>> + if (gpmc_onenand_data) { >>>>> + child_dev->name = "omap2-onenand"; >>>>> + child_dev->dev.platform_data = gpmc_onenand_data; >>>>> + ret = platform_device_add_resources(child_dev, &res, 1); >>>>> + if (ret) >>>>> + goto err_child_fail; >>>>> + } >>>>> + >>>> >>>> This bit will go away on squashing patch 10. >>> >>> I'm not sure how to do this and merge changes using two separate trees. >> >> - make sure build doesn't break in each tree. I'm OK with oneNAND functionality broken in >> individual trees for this case. As there is no other way out and make your life easier. >> - make sure everything works after trees are merged. >>> >>>>> /* is child a common bus? */ >>>>> if (of_match_node(of_default_bus_match_table, child)) >>>>> /* create children and other common bus children */ >>>>> @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) >>>>> if (!child->name) >>>>> continue; >>>>> >>>>> - if (of_node_cmp(child->name, "onenand") == 0) >>>>> - ret = gpmc_probe_onenand_child(pdev, child); >>>>> - else >>>>> - ret = gpmc_probe_generic_child(pdev, child); >>>>> - >>>>> + ret = gpmc_probe_child(pdev, child); >>>>> if (ret) { >>>>> dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", >>>>> child->name, ret); >>>>> >>>>
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c index f2aef0b87bc6..fc7c3c2a0b04 100644 --- a/drivers/memory/omap-gpmc.c +++ b/drivers/memory/omap-gpmc.c @@ -1921,52 +1921,22 @@ 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_ONENAND) -static int gpmc_probe_onenand_child(struct platform_device *pdev, - struct device_node *child) -{ - u32 val; - struct omap_onenand_platform_data *gpmc_onenand_data; - - if (of_property_read_u32(child, "reg", &val) < 0) { - dev_err(&pdev->dev, "%pOF has no 'reg' property\n", - child); - return -ENODEV; - } - - gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data), - GFP_KERNEL); - if (!gpmc_onenand_data) - return -ENOMEM; - - gpmc_onenand_data->cs = val; - gpmc_onenand_data->of_node = child; - gpmc_onenand_data->dma_channel = -1; - - if (!of_property_read_u32(child, "dma-channel", &val)) - gpmc_onenand_data->dma_channel = val; - - return gpmc_onenand_init(gpmc_onenand_data); -} -#else -static int gpmc_probe_onenand_child(struct platform_device *pdev, - struct device_node *child) -{ - return 0; -} -#endif +extern int +gpmc_omap_onenand_init(struct omap_onenand_platform_data *_onenand_data); /** - * gpmc_probe_generic_child - configures the gpmc for a child device + * gpmc_probe_child - configures the gpmc for a child device * @pdev: pointer to gpmc platform device * @child: pointer to device-tree node for child device * * Allocates and configures a GPMC chip-select for a child device. * Returns 0 on success and appropriate negative error code on failure. */ -static int gpmc_probe_generic_child(struct platform_device *pdev, +static int gpmc_probe_child(struct platform_device *pdev, struct device_node *child) { + struct omap_onenand_platform_data *gpmc_onenand_data; + struct platform_device *child_dev; struct gpmc_settings gpmc_s; struct gpmc_timings gpmc_t; struct resource res; @@ -2058,6 +2028,33 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, } } + if (of_node_cmp(child->name, "onenand") == 0) { + /* + * Warn about older DT blobs with no compatible property + * and fix them for now + */ + if (!of_property_read_bool(child, "compatible")) { + struct property *prop; + + dev_warn(&pdev->dev, + "Incompatible OneNAND node, fixing."); + + prop = kzalloc(sizeof(*prop), GFP_KERNEL); + if (!prop) { + ret = -ENOMEM; + goto err; + } + + prop->name = "compatible"; + /* See FIXME about device-tree migration bellow */ + prop->value = (gpmc_capability & GPMC_HAS_WR_ACCESS) ? + "ti,omap3-onenand" : "ti,omap2-onenand"; + prop->length = 14; + + of_update_property(child, prop); + } + } + if (of_device_is_compatible(child, "ti,omap2-nand")) { /* NAND specific setup */ val = 8; @@ -2079,6 +2076,30 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, /* disable write protect */ gpmc_configure(GPMC_CONFIG_WP, 0); gpmc_s.device_nand = true; + } else if (of_device_is_compatible(child, "ti,omap2-onenand") || + of_device_is_compatible(child, "ti,omap3-onenand")) { + gpmc_onenand_data = devm_kzalloc(&pdev->dev, + sizeof(*gpmc_onenand_data), + GFP_KERNEL); + if (!gpmc_onenand_data) { + ret = -ENOMEM; + goto err; + } + + gpmc_onenand_data->cs = cs; + gpmc_onenand_data->of_node = child; + if (!of_property_read_u32(child, "dma-channel", &val)) + gpmc_onenand_data->dma_channel = val; + else + gpmc_onenand_data->dma_channel = -1; + + ret = gpmc_omap_onenand_init(gpmc_onenand_data); + if (ret) + goto err; + + /* OneNAND driver handles timings on its own */ + gpmc_cs_enable_mem(cs); + goto no_timings; } else { ret = of_property_read_u32(child, "bank-width", &gpmc_s.device_width); @@ -2126,9 +2147,19 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, no_timings: /* create platform device, NULL on error or when disabled */ - if (!of_platform_device_create(child, NULL, &pdev->dev)) + child_dev = of_platform_device_create(child, NULL, &pdev->dev); + if (!child_dev) goto err_child_fail; + /* Use platform data until OneNAND driver is DT aware */ + if (gpmc_onenand_data) { + child_dev->name = "omap2-onenand"; + child_dev->dev.platform_data = gpmc_onenand_data; + ret = platform_device_add_resources(child_dev, &res, 1); + if (ret) + goto err_child_fail; + } + /* is child a common bus? */ if (of_match_node(of_default_bus_match_table, child)) /* create children and other common bus children */ @@ -2193,11 +2224,7 @@ static void gpmc_probe_dt_children(struct platform_device *pdev) if (!child->name) continue; - if (of_node_cmp(child->name, "onenand") == 0) - ret = gpmc_probe_onenand_child(pdev, child); - else - ret = gpmc_probe_generic_child(pdev, child); - + ret = gpmc_probe_child(pdev, child); if (ret) { dev_err(&pdev->dev, "failed to probe DT child '%s': %d\n", child->name, ret);
Generic probe function can deal with OneNAND node, remove now useless gpmc_probe_onenand_child function. Signed-off-by: Ladislav Michl <ladis@linux-mips.org> --- drivers/memory/omap-gpmc.c | 111 ++++++++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 42 deletions(-)