Message ID | 1376637583-12485-1-git-send-email-shc_work@mail.ru |
---|---|
State | New, archived |
Headers | show |
On Fri, Aug 16, 2013 at 11:19:42AM +0400, Alexander Shiyan wrote: > This patch adds a property to automatically determine the NAND > bus width. This property works if the bus width is not specified > explicitly. > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > --- > .../devicetree/bindings/mtd/gpio-control-nand.txt | 2 ++ > drivers/mtd/nand/gpio.c | 16 ++++++++++++---- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > index 36ef07d..91070d0 100644 > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > @@ -19,6 +19,8 @@ Optional properties: > defaults to 1 byte. > - chip-delay : chip dependent delay for transferring data from array to > read registers (tR). If not present then a default of 20us is used. > +- gpio-control-nand,bank-width-auto : Device bus width is determined > + automatically if "bank-width" is omitted (Boolean). > - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read > location used to guard against bus reordering with regards to accesses to > the GPIO's and the NAND flash data bus. If present, then after changing > diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c > index e826f89..8eea181 100644 > --- a/drivers/mtd/nand/gpio.c > +++ b/drivers/mtd/nand/gpio.c > @@ -116,6 +116,10 @@ static int gpio_nand_get_config_of(const struct device *dev, > dev_err(dev, "invalid bank-width %u\n", val); > return -EINVAL; > } > + } else { Combine with the 'if' and make it 'else if'. > + if (of_get_property(dev->of_node, > + "gpio-control-nand,bank-width-auto", NULL)) of_property_read_bool() > + plat->options |= NAND_BUSWIDTH_AUTO; > } > > plat->gpio_rdy = of_get_gpio(dev->of_node, 0); > @@ -223,6 +227,14 @@ static int gpio_nand_probe(struct platform_device *pdev) > if (IS_ERR(chip->IO_ADDR_R)) > return PTR_ERR(chip->IO_ADDR_R); > > + ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); > + if (ret) > + return ret; > + > + if (resource_size(res) < 2) > + gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | > + NAND_BUSWIDTH_AUTO); I can't quite figure out what the significance of resource_size(res) < 2 is. It doesn't quite match my reading of the device-tree documentation. Maybe I'm being dense. Is the resource provided as either an 8-bit IO address (length=1) or 16-bit IO address (length=2)? > + > res = gpio_nand_get_io_sync(pdev); > if (res) { > gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res); > @@ -230,10 +242,6 @@ static int gpio_nand_probe(struct platform_device *pdev) > return PTR_ERR(gpiomtd->io_sync); > } > > - ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); > - if (ret) > - return ret; > - > ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nce, "NAND NCE"); > if (ret) > return ret; I can fixup the first 2 without a resend. Depending on your response to the third comment, I'll apply this. Thanks, Brian
> On Fri, Aug 16, 2013 at 11:19:42AM +0400, Alexander Shiyan wrote: > > This patch adds a property to automatically determine the NAND > > bus width. This property works if the bus width is not specified > > explicitly. > > > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> [...] > > + if (resource_size(res) < 2) > > + gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | > > + NAND_BUSWIDTH_AUTO); > > I can't quite figure out what the significance of resource_size(res) < 2 > is. It doesn't quite match my reading of the device-tree documentation. > Maybe I'm being dense. Is the resource provided as either an 8-bit IO > address (length=1) or 16-bit IO address (length=2)? Size in bytes, so this is just a fault protect for length==1 && (busw==16 || busw==auto). Do you think it's not necessary? [...] > I can fixup the first 2 without a resend. Depending on your response to > the third comment, I'll apply this. I'll prefer to resend all in next few days. Thanks. ---
On 08/16/2013 01:19 AM, Alexander Shiyan wrote: > This patch adds a property to automatically determine the NAND > bus width. This property works if the bus width is not specified > explicitly. > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > +- gpio-control-nand,bank-width-auto : Device bus width is determined > + automatically if "bank-width" is omitted (Boolean). How would a driver determine the bank width automatically; is there some obvious way a driver should do this based on the/a NAND specification? Is there more than one way this auto-detection could occur, and if so, which of those methods is this property indicating will work? I'd rather expect the property description include details of the auto-detection mechanism that the DT states will work.
diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt index 36ef07d..91070d0 100644 --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt @@ -19,6 +19,8 @@ Optional properties: defaults to 1 byte. - chip-delay : chip dependent delay for transferring data from array to read registers (tR). If not present then a default of 20us is used. +- gpio-control-nand,bank-width-auto : Device bus width is determined + automatically if "bank-width" is omitted (Boolean). - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read location used to guard against bus reordering with regards to accesses to the GPIO's and the NAND flash data bus. If present, then after changing diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c index e826f89..8eea181 100644 --- a/drivers/mtd/nand/gpio.c +++ b/drivers/mtd/nand/gpio.c @@ -116,6 +116,10 @@ static int gpio_nand_get_config_of(const struct device *dev, dev_err(dev, "invalid bank-width %u\n", val); return -EINVAL; } + } else { + if (of_get_property(dev->of_node, + "gpio-control-nand,bank-width-auto", NULL)) + plat->options |= NAND_BUSWIDTH_AUTO; } plat->gpio_rdy = of_get_gpio(dev->of_node, 0); @@ -223,6 +227,14 @@ static int gpio_nand_probe(struct platform_device *pdev) if (IS_ERR(chip->IO_ADDR_R)) return PTR_ERR(chip->IO_ADDR_R); + ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); + if (ret) + return ret; + + if (resource_size(res) < 2) + gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | + NAND_BUSWIDTH_AUTO); + res = gpio_nand_get_io_sync(pdev); if (res) { gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res); @@ -230,10 +242,6 @@ static int gpio_nand_probe(struct platform_device *pdev) return PTR_ERR(gpiomtd->io_sync); } - ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat); - if (ret) - return ret; - ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nce, "NAND NCE"); if (ret) return ret;
This patch adds a property to automatically determine the NAND bus width. This property works if the bus width is not specified explicitly. Signed-off-by: Alexander Shiyan <shc_work@mail.ru> --- .../devicetree/bindings/mtd/gpio-control-nand.txt | 2 ++ drivers/mtd/nand/gpio.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-)