[u-boot][PATCH 10/14] mtd: rawnand: omap_gpmc: support u-boot driver model

[Roger Quadros]

Hi Adam,

On 11/10/2022 18:01, Adam Ford wrote:
> On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros <rogerq at kernel.org> wrote:
>>
>> Adds driver model support.
>>
>> We need to be able to self initialize the NAND controller/chip
>> at probe and so enable CONFIG_SYS_NAND_SELF_INIT.
>>
>> Doing so requires nand_register() API which is provided by nand.c
>> and needs to be enabled during SPL build via CONFIG_SPL_NAND_INIT.
>> But nand.c also provides nand_init() so we need to get rid of nand_init()
>> in omap_gpmc driver if CONFIG_SPL_NAND_INIT is set.
>>
>> Signed-off-by: Roger Quadros <rogerq at kernel.org>
>> ---
>>  drivers/mtd/nand/raw/Kconfig     |  1 +
>>  drivers/mtd/nand/raw/omap_gpmc.c | 55 +++++++++++++++++++++++++++++++-
>>  2 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>> index bc5cabdfc2..1d23144ce4 100644
>> --- a/drivers/mtd/nand/raw/Kconfig
>> +++ b/drivers/mtd/nand/raw/Kconfig
>> @@ -190,6 +190,7 @@ config NAND_LPC32XX_SLC
>>  config NAND_OMAP_GPMC
>>         bool "Support OMAP GPMC NAND controller"
>>         depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || ARCH_K3
>> +       select SYS_NAND_SELF_INIT if ARCH_K3
> 
> I have a question about this down below.
> 
>>         help
>>           Enables omap_gpmc.c driver for OMAPx and AMxxxx platforms.
>>           GPMC controller is used for parallel NAND flash devices, and can
>> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
>> index e772a914c8..7192ca9e5a 100644
>> --- a/drivers/mtd/nand/raw/omap_gpmc.c
>> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
>> @@ -7,6 +7,7 @@
>>  #include <common.h>
>>  #include <log.h>
>>  #include <asm/io.h>
>> +#include <dm/uclass.h>
>>  #include <linux/errno.h>
>>
>>  #ifdef CONFIG_ARCH_OMAP2PLUS
>> @@ -1121,7 +1122,7 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t hardware, uint32_t eccstrength)
>>   *   nand_scan about special functionality. See the defines for further
>>   *   explanation
>>   */
>> -int board_nand_init(struct nand_chip *nand)
>> +int gpmc_nand_init(struct nand_chip *nand)
>>  {
>>         int32_t gpmc_config = 0;
>>         int cs = cs_next++;
>> @@ -1201,3 +1202,55 @@ int board_nand_init(struct nand_chip *nand)
>>
>>         return 0;
>>  }
>> +
>> +static struct nand_chip *nand_chip;    /* First NAND chip for SPL use only */
>> +
>> +#if CONFIG_IS_ENABLED(SYS_NAND_SELF_INIT)
>> +
>> +static int gpmc_nand_probe(struct udevice *dev)
>> +{
>> +       struct nand_chip *nand = dev_get_priv(dev);
>> +       struct mtd_info *mtd = nand_to_mtd(nand);
>> +       int ret;
>> +
>> +       gpmc_nand_init(nand);
>> +
>> +       ret = nand_scan(mtd, CONFIG_SYS_NAND_MAX_CHIPS);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = nand_register(0, mtd);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (!nand_chip)
>> +               nand_chip = nand;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct udevice_id gpmc_nand_ids[] = {
>> +       { .compatible = "ti,am64-nand" },
>> +       { .compatible = "ti,omap2-nand" },
> 
> The gpmc_nand_ids reference to omap2, but it's encapsulated inside the
> SYS_NAND_SELF_INIT ifdef which appears to only be set if K3.  Should
> this code be expected to work on OMAP2?  I don't think K3 is set for
> OMAP2+.  If so, should the SYS_NAND_SELF_INIT be selected if OMAP2 is
> selected?

We want to eventually get this working using driver model and SYS_NAND_SELF_INIT
for OMAP2 as well but just that I didn't work on it yet or test it.

One challenge is that OMAP2 boards tend to either select nand_spl_simple.c
or am335x_spl_bch.c for NAND support at SPL.

We will need to figure out if it is possible to use CONFIG_SPL_NAND_INIT
and this driver instead.
One issue might be that everything doesn't fit in resources available at SPL?

> 
> I have a DM3730 that I can test with this.  Do you have a repo I can

That would be great. Thanks!

> point to to test?  If not, I'll pull the series from patchwork, but I
> need to know what branch to use as a starting point.

You can use this Repo as reference.
https://github.com/rogerq/u-boot/commits/for-v2023.01/am64-nand-base-1.0-test

It has a few patches on top consisting of device tree and u-boot configuration
for AM64 platform. You can ignore the last 2 patches as they are only for a
workaround on early AM64 boards.

If you hit any hurdles, we can discuss how to resolve.

> 
> thanks,
> 
> adam
> 
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(gpmc_nand) = {
>> +       .name           = "gpmc-nand",
>> +       .id             = UCLASS_MTD,
>> +       .of_match       = gpmc_nand_ids,
>> +       .probe          = gpmc_nand_probe,
>> +       .priv_auto      = sizeof(struct nand_chip),
>> +};
>> +
>> +void board_nand_init(void)
>> +{
>> +       struct udevice *dev;
>> +       int ret;
>> +
>> +       ret = uclass_get_device_by_driver(UCLASS_MTD,
>> +                                         DM_DRIVER_GET(gpmc_nand), &dev);
>> +       if (ret && ret != -ENODEV)
>> +               pr_err("%s: Failed to get GPMC device: %d\n", __func__, ret);
>> +}
>> +#endif /* CONFIG_SYS_NAND_SELF_INIT */
>> --
>> 2.17.1
>>

cheers,
-roger