* [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF @ 2013-12-07 15:36 Ezequiel Garcia 2013-12-09 20:31 ` Brian Norris 0 siblings, 1 reply; 5+ messages in thread From: Ezequiel Garcia @ 2013-12-07 15:36 UTC (permalink / raw) To: linux-mtd; +Cc: Alexander Shiyan, Ezequiel Garcia Since the of_mtd header provides dummy stubs for !CONFIG_OF, it's safe to remove the #ifdef CONFIG_OF. Also remove the of_match_ptr guard as it's no longer required. Build tested only. Cc: Alexander Shiyan <shc_work@mail.ru> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/mtd/nand/gpio.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c index e826f89..c966f86 100644 --- a/drivers/mtd/nand/gpio.c +++ b/drivers/mtd/nand/gpio.c @@ -94,7 +94,6 @@ static int gpio_nand_devready(struct mtd_info *mtd) return gpio_get_value(gpiomtd->plat.gpio_rdy); } -#ifdef CONFIG_OF static const struct of_device_id gpio_nand_id_table[] = { { .compatible = "gpio-control-nand" }, {} @@ -145,19 +144,6 @@ static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev) return r; } -#else /* CONFIG_OF */ -static inline int gpio_nand_get_config_of(const struct device *dev, - struct gpio_nand_platdata *plat) -{ - return -ENOSYS; -} - -static inline struct resource * -gpio_nand_get_io_sync_of(struct platform_device *pdev) -{ - return NULL; -} -#endif /* CONFIG_OF */ static inline int gpio_nand_get_config(const struct device *dev, struct gpio_nand_platdata *plat) @@ -308,7 +294,7 @@ static struct platform_driver gpio_nand_driver = { .driver = { .name = "gpio-nand", .owner = THIS_MODULE, - .of_match_table = of_match_ptr(gpio_nand_id_table), + .of_match_table = gpio_nand_id_table, }, }; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF 2013-12-07 15:36 [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF Ezequiel Garcia @ 2013-12-09 20:31 ` Brian Norris 2013-12-09 21:17 ` Ezequiel Garcia 0 siblings, 1 reply; 5+ messages in thread From: Brian Norris @ 2013-12-09 20:31 UTC (permalink / raw) To: Ezequiel Garcia; +Cc: linux-mtd, Alexander Shiyan On Sat, Dec 07, 2013 at 12:36:49PM -0300, Ezequiel Garcia wrote: > Since the of_mtd header provides dummy stubs for !CONFIG_OF, it's safe > to remove the #ifdef CONFIG_OF. Also remove the of_match_ptr guard as > it's no longer required. Build tested only. > > Cc: Alexander Shiyan <shc_work@mail.ru> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Just because we can compile with and without CONFIG_OF doesn't mean the behavior is equivalent now. Take a look at gpio_nand_get_io_sync_of(): we allocate and throw away memory (it's only cleaned up at device removal time): static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev) { struct resource *r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); u64 addr; if (!r || of_property_read_u64(pdev->dev.of_node, "gpio-control-nand,io-sync-reg", &addr)) return NULL; ... But I guess this should be fixed anyway, to do this: static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev) { struct resource *r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); u64 addr; if (of_property_read_u64(pdev->dev.of_node, "gpio-control-nand,io-sync-reg", &addr)) return NULL; r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); if (!r) return NULL; /* Probably should be PTR_ERR(-ENOMEM), with callee-error-checking */ ... I think we might as well fix this before forcing this quirk onto !OF builds. Brian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF 2013-12-09 20:31 ` Brian Norris @ 2013-12-09 21:17 ` Ezequiel Garcia 2013-12-09 21:22 ` Brian Norris 0 siblings, 1 reply; 5+ messages in thread From: Ezequiel Garcia @ 2013-12-09 21:17 UTC (permalink / raw) To: Brian Norris; +Cc: linux-mtd, Alexander Shiyan Brian, On Mon, Dec 09, 2013 at 12:31:05PM -0800, Brian Norris wrote: > On Sat, Dec 07, 2013 at 12:36:49PM -0300, Ezequiel Garcia wrote: > > Since the of_mtd header provides dummy stubs for !CONFIG_OF, it's safe > > to remove the #ifdef CONFIG_OF. Also remove the of_match_ptr guard as > > it's no longer required. Build tested only. > > > > Cc: Alexander Shiyan <shc_work@mail.ru> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > Just because we can compile with and without CONFIG_OF doesn't mean the > behavior is equivalent now. > > Take a look at gpio_nand_get_io_sync_of(): we allocate and throw away > memory (it's only cleaned up at device removal time): > > static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev) > { > struct resource *r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); > u64 addr; > > if (!r || of_property_read_u64(pdev->dev.of_node, > "gpio-control-nand,io-sync-reg", &addr)) > return NULL; > ... > > But I guess this should be fixed anyway, to do this: > > static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev) > { > struct resource *r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); > u64 addr; > > if (of_property_read_u64(pdev->dev.of_node, > "gpio-control-nand,io-sync-reg", &addr)) > return NULL; > > r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL); > if (!r) > return NULL; /* Probably should be PTR_ERR(-ENOMEM), with callee-error-checking */ > ... > > I think we might as well fix this before forcing this quirk onto !OF > builds. > Ah, I missed that spot! Yes, I think you're right, so let's hold this one until the above gets fixed. By the way, has anyone actually *tried* this driver with !OF lately? I guess I'm missing something but it seems to me it won't probe: static inline int gpio_nand_get_config_of(const struct device *dev, struct gpio_nand_platdata *plat) { return -ENOSYS; } static inline int gpio_nand_get_config(const struct device *dev, struct gpio_nand_platdata *plat) { int ret = gpio_nand_get_config_of(dev, plat); if (!ret) return ret; [..] } static int gpio_nand_probe(struct platform_device *pdev) { [..] ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); if (ret) return ret; } -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF 2013-12-09 21:17 ` Ezequiel Garcia @ 2013-12-09 21:22 ` Brian Norris 2013-12-09 21:39 ` Ezequiel Garcia 0 siblings, 1 reply; 5+ messages in thread From: Brian Norris @ 2013-12-09 21:22 UTC (permalink / raw) To: Ezequiel Garcia; +Cc: linux-mtd, Alexander Shiyan On Mon, Dec 09, 2013 at 06:17:51PM -0300, Ezequiel Garcia wrote: > By the way, has anyone actually *tried* this driver with !OF lately? > I guess I'm missing something but it seems to me it won't probe: > > static inline int gpio_nand_get_config_of(const struct device *dev, > struct gpio_nand_platdata *plat) > { > return -ENOSYS; > } > > static inline int gpio_nand_get_config(const struct device *dev, > struct gpio_nand_platdata *plat) > { > int ret = gpio_nand_get_config_of(dev, plat); > if (!ret) ^^^ if (! (-ENOSYS)) ====> if (0) > return ret; > [..] > } > > static int gpio_nand_probe(struct platform_device *pdev) > { > [..] > ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); > if (ret) > return ret; [..] > } Looks OK to me, but I haven't tested it. What do you see? Perhaps you were incorrectly reading the line I pointed to? Brian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF 2013-12-09 21:22 ` Brian Norris @ 2013-12-09 21:39 ` Ezequiel Garcia 0 siblings, 0 replies; 5+ messages in thread From: Ezequiel Garcia @ 2013-12-09 21:39 UTC (permalink / raw) To: Brian Norris; +Cc: linux-mtd, Alexander Shiyan On Mon, Dec 09, 2013 at 01:22:36PM -0800, Brian Norris wrote: > On Mon, Dec 09, 2013 at 06:17:51PM -0300, Ezequiel Garcia wrote: > > By the way, has anyone actually *tried* this driver with !OF lately? > > I guess I'm missing something but it seems to me it won't probe: > > > > static inline int gpio_nand_get_config_of(const struct device *dev, > > struct gpio_nand_platdata *plat) > > { > > return -ENOSYS; > > } > > > > static inline int gpio_nand_get_config(const struct device *dev, > > struct gpio_nand_platdata *plat) > > { > > int ret = gpio_nand_get_config_of(dev, plat); > > if (!ret) > > ^^^ if (! (-ENOSYS)) ====> if (0) > Ah.. yes... > > return ret; > > [..] > > } > > > > static int gpio_nand_probe(struct platform_device *pdev) > > { > > [..] > > ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); > > if (ret) > > return ret; > [..] > > } > > Looks OK to me, but I haven't tested it. What do you see? Perhaps you > were incorrectly reading the line I pointed to? > I knew I was missing something! Yes, that line confused me. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-09 21:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-07 15:36 [PATCH v2] mtd: nand: gpio: Remove unneeded CONFIG_OF Ezequiel Garcia 2013-12-09 20:31 ` Brian Norris 2013-12-09 21:17 ` Ezequiel Garcia 2013-12-09 21:22 ` Brian Norris 2013-12-09 21:39 ` Ezequiel Garcia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).