[v4,1/4] mtd: nand: gpio: Add DT property to automatically determine bus width

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(-)

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.