On Wed, Oct 12, 2022 at 1:22 AM Roger Quadros <rog...@kernel.org> wrote:
>
> Hi Adam,
>
> On 11/10/2022 18:01, Adam Ford wrote:
> > On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros <rog...@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 <rog...@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?
On my board the GPMC runs more than just NAND. I am hoping to get the
GPMC driver working in U-Boot first then in SPL (assuming it fits). I
have LTO enabled on my DM3730, so I am hoping it might help make some
room. I am hoping that once the NAND is working that the GPMC driver
can be used in U-Boot to handle the configuration of the bus for
handling Ethernet, so some of the quirks and manual board file
configuration can be removed.
>
> >
> > 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
>
Thanks!
> 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.
I'll just pull in the branch and build for my DM3730 then start
enabling and disabling stuff. I haven't checked how big SPL is, but
SPL keeps growing instead of shrinking. OF_PLATDATA might be an
option if it doesn't fit in SPL, but I was hoping to avoid that.
adam
>
> >
> > 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
