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

Mugunthan V N
2016-04-22T05:06:04Z

On Thursday 21 April 2016 07:41 PM, Simon Glass wrote:
> 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.
>

Okay then I will remove this check as it doesn't add value.

Regards
Mugunthan V N