Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

Jamie Iles
2011-08-19T21:18:13Z

On Fri, Aug 19, 2011 at 10:51:46PM +0300, Artem Bityutskiy wrote:
> On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> > @@ -178,7 +249,7 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
> >
> > nand_release(&gpiomtd->mtd_info);
> >
> > - res = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > + res = gpio_nand_get_io_sync(dev);
>
> Why do you call 'gpio_nand_get_io_sync(dev)' here, in
> 'gpio_nand_remove()' function? You should have it in gpiomtd->io_sync.
> Right?
>
> If this is the case, then you do not need a separate
> 'gpio_nand_get_io_sync()' function at all, you can make
> 'gpio_nand_get_config()' to fetch the io_sync information from the DT.
> And then you will have one single function which gets data from DT, not
> 2 -> simpler code.
>
> Do I miss something?

gpiomtd->io_sync is a void __iomem *, but we need a struct resource here
so that we can do the release_mem_region(). I could store the struct
resource pointer in gpiomtd rather than calling gpio_nand_get_io_sync()
twice though, I'm happy to change if you prefer.

Note that for the device tree case, the iosync register isn't in the reg
property so we can't do platform_get_resource() to get it. We do this
because the io_sync address isn't actually a gpio nand resource and
can't always be expressed as such in the device tree.

Jamie