Hi Sascha,
Thanks for your review.
I will send a v2 with the modification. I will also reorder patches in
the patchset.
Raphaël
2014-08-04 19:43 GMT+02:00 Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>:
> On Fri, Aug 01, 2014 at 03:23:20PM +0200, Raphaël Poggi wrote:
>> Signed-off-by: Raphaël Poggi <poggi.raph@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
