[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

Simon Glass
2016-04-21T14:11:55Z

Hi Mugunthan,

On 20 April 2016 at 23:55, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote:
>> Hi Mugunthan,
>>
>> On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>> Implement a NAND uclass so that the NAND devices can be
>>> accessed via the DM framework.
>>>
>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>> ---
>>> drivers/mtd/nand/Kconfig | 10 +++++++
>>> drivers/mtd/nand/Makefile | 2 ++
>>> drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>> drivers/mtd/nand/nand.c | 6 +++-
>>> include/dm/uclass-id.h | 1 +
>>> include/nand.h | 5 ++++
>>> 6 files changed, 85 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/mtd/nand/nand-uclass.c
>>>
>>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>>> index 7787505..53b57b8 100644
>>> --- a/drivers/mtd/nand/Kconfig
>>> +++ b/drivers/mtd/nand/Kconfig
>>> @@ -8,6 +8,16 @@ menuconfig NAND
>>>
>>> if NAND
>>>
>>> +config DM_NAND
>>> + bool "Enable driver model for NAND"
>>> + depends on NAND && DM
>>> + help
>>> + Enable driver model for NAND. The NAND interface is then
>>> + implemented by the NAND uclass. Multiple NAND devices can
>>> + be attached and used. The 'nand' command works as normal.
>>> +
>>> + If the NAND drivers doesn't support DM, say N.
>>> +
>>> config SYS_NAND_SELF_INIT
>>> bool
>>> help
>>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>>> index 6fb3718..7745705 100644
>>> --- a/drivers/mtd/nand/Makefile
>>> +++ b/drivers/mtd/nand/Makefile
>>> @@ -39,6 +39,8 @@ endif # not spl
>>>
>>> ifdef NORMAL_DRIVERS
>>>
>>> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
>>> +
>>> obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>>>
>>> obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
>>> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
>>> new file mode 100644
>>> index 0000000..d68d357
>>> --- /dev/null
>>> +++ b/drivers/mtd/nand/nand-uclass.c
>>> @@ -0,0 +1,62 @@
>>> +/*
>>> + * NAND uclass driver for NAND bus.
>>> + *
>>> + * (C) Copyright 2012-2016
>>> + * Texas Instruments Incorporated, <www.ti.com>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <errno.h>
>>> +#include <nand.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +#ifdef CONFIG_DM_NAND
>>> +
>>> +nand_info_t *get_nand_dev_by_index(int idx)
>>> +{
>>> + nand_info_t *nand;
>>> + struct udevice *dev;
>>> + int ret;
>>> +
>>> + ret = uclass_get_device(UCLASS_NAND, idx, &dev);
>>> + if (ret) {
>>> + error("NAND device (%d) not found\n", idx);
>>
>> This should return -ENODEV. Also please avoid printf() in drivers. The
>> caller can report the error.
>>
>
> Return type is pointer so returned NULL and NULL denotes no dev.
> Will change this error to debug in v2.

OK, I see that this is the existing API.

>
>>> + return NULL;
>>> + }
>>> +
>>> + nand = (nand_info_t *)dev_get_uclass_priv(dev);
>>> + if (!nand) {
>>> + error("Nand device not ready\n");
>>> + return NULL;
>>> + }
>>
>> But if the device has been probed ((with uclass_get_device()) then
>> this cannot be NULL.
>
> This check is just a failsafe. ideally it should not fail here.

I cannot get to probe the device without this data, since this would
be a bug in DM. So the check has no value.

[snip]

Regards,
Simon