| Message ID | 20221011115012.6181-11-rogerq@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | rawnand: omap_gpmc: driver model support | expand |
On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros <rogerq@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@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? I have a DM3730 that I can test with this. Do you have a repo I can 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. 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 >
Hi Adam, On 11/10/2022 18:01, Adam Ford wrote: > On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros <rogerq@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@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
On Wed, Oct 12, 2022 at 1:22 AM Roger Quadros <rogerq@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 <rogerq@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@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
Hi Adam, On Wed, Oct 12, 2022 at 06:42:22AM -0500, Adam Ford wrote: [snip] > 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. Any estimate on that? I moved IGEP to use DM for ethernet, but for reasons you mentioned quirk in DT is still there. Reposting that patch would currently add one more quirk into codebase, so I'd better avoid that. Thank you, L.
On Wed, Oct 12, 2022 at 6:57 AM Ladislav Michl <oss-lists@triops.cz> wrote: > > Hi Adam, > > On Wed, Oct 12, 2022 at 06:42:22AM -0500, Adam Ford wrote: > [snip] > > 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. > > Any estimate on that? I moved IGEP to use DM for ethernet, but for reasons > you mentioned quirk in DT is still there. Reposting that patch would > currently add one more quirk into codebase, so I'd better avoid that. I won't know until I have a chance to sit down and try out the code framework that's proposed. I'll be doing it as a side project, so I can't guarantee speed, but since Roger is trying to get NAND working, my work will start there with the hope of evolving it to do more as I get time. adam > > Thank you, > L.
On Wed, Oct 12, 2022 at 6:42 AM Adam Ford <aford173@gmail.com> wrote: > > On Wed, Oct 12, 2022 at 1:22 AM Roger Quadros <rogerq@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 <rogerq@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@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 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. 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. Do you have any suggestions on what I should do to diagnose the -96 errors on all the GPMC sub-nodes? 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
On 13/10/2022 14:17, Adam Ford wrote: > On Wed, Oct 12, 2022 at 6:42 AM Adam Ford <aford173@gmail.com> wrote: >> >> On Wed, Oct 12, 2022 at 1:22 AM Roger Quadros <rogerq@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 <rogerq@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@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
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 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" }, + { } +}; + +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 */
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@kernel.org> --- drivers/mtd/nand/raw/Kconfig | 1 + drivers/mtd/nand/raw/omap_gpmc.c | 55 +++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-)