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

[Mugunthan V N]

On Thursday 21 April 2016 07:41 PM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 20 April 2016 at 23:55, Mugunthan V N <mugunthanvnm at 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 at 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 at 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