Re: [PATCH 1/5] mtd: atmel_nand: add support for device tree

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Aug 01, 2014 at 03:23:20PM +0200, Raphaël Poggi wrote:
> Signed-off-by: Raphaël Poggi <poggi.raph@gmail.com>
> ---
>  drivers/mtd/nand/atmel_nand.c |  108 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 337e225..b7b0e3a 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -28,6 +28,10 @@
>  #include <init.h>
>  #include <gpio.h>
>  
> +#include <of.h>
> +#include <of_gpio.h>
> +#include <of_mtd.h>
> +
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
>  
> @@ -1038,6 +1042,89 @@ static void atmel_nand_hwctl(struct mtd_info *mtd, int mode)
>  #endif
>  }
>  
> +static int atmel_nand_of_init(struct atmel_nand_host *host, struct device_node *np)
> +{
> +	u32 val;
> +	u32 offset[2];
> +	int ecc_mode;
> +	struct atmel_nand_data *board = host->board;
> +	enum of_gpio_flags flags = 0;

Please add a:

	if (!IS_ENABLED(CONFIG_OFDEVICE))
		return -ENOSYS;

to this function. It will allow the compiler to throw it away when
device tree support is disabled.

> +
> +	if (of_property_read_u32(np, "atmel,nand-addr-offset", &val) == 0) {
> +	   if (val >= 32) {

Please fix the coding style. First indentation here is a tab, but the
second is 4 spaces. Indentation should be tabs generally.

> +		dev_err(host->dev, "invalid addr-offset %u\n", val);
> +		return -EINVAL;
> +	    }
> +	    board->ale = val;
> +	}
> +
> +	if (of_property_read_u32(np, "atmel,nand-cmd-offset", &val) == 0) {
> +	    if (val >= 32) {
> +		dev_err(host->dev, "invalid cmd-offset %u\n", val);
> +		return -EINVAL;
> +	    }
> +	    board->cle = val;
> +	}
> +
> +	ecc_mode = of_get_nand_ecc_mode(np);
> +
> +	board->ecc_mode = ecc_mode < 0 ? NAND_ECC_SOFT : ecc_mode;
> +
> +	board->on_flash_bbt = of_get_nand_on_flash_bbt(np);
> +
> +	if (of_get_nand_bus_width(np) == 16)
> +	    board->bus_width_16 = 1;
> +
> +	board->rdy_pin = of_get_gpio_flags(np, 0, &flags);
> +	board->enable_pin = of_get_gpio(np, 1);
> +	board->det_pin = of_get_gpio(np, 2);
> +
> +	board->has_pmecc = of_property_read_bool(np, "atmel,has-pmecc");
> +
> +	if (!(board->ecc_mode == NAND_ECC_HW) || !board->has_pmecc)
> +	    return 0;	/* Not using PMECC */
> +
> +	/* use PMECC, get correction capability, sector size and lookup
> +	* table offset.
> +	* If correction bits and sector size are not specified, then
> +	*   find
> +	* them from NAND ONFI parameters.
> +	*/
> +	if (of_property_read_u32(np, "atmel,pmecc-cap", &val) == 0) {
> +	    if ((val != 2) && (val != 4) && (val != 8) && (val != 12) && (val != 24)) {
> +		dev_err(host->dev, "Unsupported PMECC correction capability: %d"
> +				" should be 2, 4, 8, 12 or 24\n", val);
> +		return -EINVAL;
> +	    }
> +
> +	    board->pmecc_corr_cap = (u8)val;
> +	}
> +
> +	if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) == 0) {
> +	    if ((val != 512) && (val != 1024)) {
> +		dev_err(host->dev, "Unsupported PMECC sector size: %d"
> +				" should be 512 or 1024 bytes\n", val);
> +		return -EINVAL;
> +	    }
> +
> +	    board->pmecc_sector_size = (u16)val;
> +	}
> +
> +	if (of_property_read_u32_array(np, "atmel,pmecc-lookup-table-offset", offset, 2) != 0) {
> +	    dev_err(host->dev, "Cannot get PMECC lookup table offset\n");
> +	    return -EINVAL;
> +	}
> +
> +	if (!offset[0] && !offset[1]) {
> +	    dev_err(host->dev, "Invalid PMECC lookup table offset\n");
> +	    return -EINVAL;
> +	}
> +
> +	board->pmecc_lookup_table_offset = (board->pmecc_sector_size == 512) ? offset[0] : offset[1];
> +
> +	return 0;
> +}
> +
>  static int atmel_hw_nand_init_params(struct device_d *dev,
>  					 struct atmel_nand_host *host)
>  {
> @@ -1093,7 +1180,7 @@ static int atmel_hw_nand_init_params(struct device_d *dev,
>   */
>  static int __init atmel_nand_probe(struct device_d *dev)
>  {
> -	struct atmel_nand_data *pdata = dev->platform_data;
> +	struct atmel_nand_data *pdata;
>  	struct atmel_nand_host *host;
>  	struct mtd_info *mtd;
>  	struct nand_chip *nand_chip;
> @@ -1104,6 +1191,10 @@ static int __init atmel_nand_probe(struct device_d *dev)
>  	if (!host)
>  		return -ENOMEM;
>  
> +	pdata = kzalloc(sizeof(struct atmel_nand_data), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;

You only use this memory...

> +
>  	host->io_base = dev_request_mem_region(dev, 0);
>  
>  	mtd = &host->mtd;
> @@ -1111,6 +1202,15 @@ static int __init atmel_nand_probe(struct device_d *dev)
>  	host->board = pdata;
>  	host->dev = dev;
>  
> +	if (dev->device_node) {
> +	    res = atmel_nand_of_init(host, dev->device_node);
> +	    if (res)
> +		goto err_no_card;

...in this code path. Please add the memory allocation here then.

> +	}
> +	else {
> +	    pdata = dev->platform_data;
> +	}
> +

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox