On 13/10/2022 14:17, Adam Ford wrote: > On Wed, Oct 12, 2022 at 6:42 AM Adam Ford <aford...@gmail.com> wrote: >> >> 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! > > I haven't spend a lot of time, but here is what I have so far: > > U-Boot 2022.10-rc4-gf499f45d18-dirty (Oct 13 2022 - 05:33:33 -0500) > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > DRAM: 256 MiB > Error binding driver 'gpmc-nand': -96 > Some drivers failed to bind > Error binding driver 'ti-gpmc': -96 > Some drivers failed to bind > Error binding driver 'simple_bus': -96 > Some drivers failed to bind > initcall sequence 8ffde84c failed at call 8011705d (err=-96) > ### ERROR ### Please RESET the board ### > > There was a small conflict with arch/arm/mach-omap2/mem-common.c > where I needed to remove a reference to gpmc_cfg to let it compile. > > +#ifndef CONFIG_TI_GPMC > const struct gpmc *gpmc_cfg = (struct gpmc *)GPMC_BASE; > +#endif
OK. But eventually we don't want to include mem-common.c when TI_GPMC driver is enabled. > > After that, I had to enable CONFIG_CLK and CONFIG_CLK_TI_GATE, but I > am not sure it's sufficient. I haven't had enough time to check to > see if the OMAP3 clock drivers are fully supported in U-Boot or not. Tom could answer this question. > I am guessing they'll need to be functional enough to fetch the fck so > the GPMC controller knows how fast it's running to properly calculate > the timings. Even if fck is not available we could assume a default value for now so driver can proceed. > > Do you have any suggestions on what I should do to diagnose the -96 > errors on all the GPMC sub-nodes? -96 is EPFNOSUPPORT and it seems to be coming from uclass_add() Did you enable CONFIG_DM_MEMORY and CONFIG_DM_MTD? If that doesn't work then try to add some prints in device_bind_common() to see where it fails. Please see this patch to see what all I had to enable to get NAND working on AM64 https://github.com/rogerq/u-boot/commit/f831875b6551588f7c83314d5770e2b73b18bc37 The following 2 patches might also be helpful to get NAND to work https://github.com/rogerq/u-boot/commit/2c64a620e2a65eef3bbe3aa1e74488daea9e019d https://github.com/rogerq/u-boot/commit/50d7ed49bc4737cccecc5ce2e3f2c3b17d540a5f > > adam >>> >>>> 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 cheers, -roger