From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0FD52C433FE for ; Thu, 13 Oct 2022 19:42:47 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id EEC7884E23; Thu, 13 Oct 2022 21:42:44 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="nUcTPOZt"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0785784DAD; Thu, 13 Oct 2022 21:42:43 +0200 (CEST) Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 62E7D84E23 for ; Thu, 13 Oct 2022 21:42:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rogerq@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id C436CB81E91; Thu, 13 Oct 2022 19:42:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0245DC433D7; Thu, 13 Oct 2022 19:42:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1665690157; bh=bpjooKIBLxTrnLOEYqe+ckbAxKBA2JisAN+vmzL9lmw=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=nUcTPOZt6vyiIWqSIsS2lVcx03vtIuKtO35jcrg7dnGEPaO8rHc6tzZiLUfMSEk6U ae7rop80USJ6S05+9ZnDvijkLTJuXiiRFUSt+feHBZl0yW1PPCMHveBZ8TYd+7bAyh 7ALrXY8/qC0haryMqFfuKtq74gEjnD1avHRtooz63rVAginOY8GDivejKIF9ckzMW6 pn4VS1B1pnr3K0ZDNf5Bk2Hx4ApP+19hX48cK6FRjTeSjh+18QJh0MQ2SbMkPzLGHu SeeEfL7qtXzgKaBDvyqrm++e/y+w2ss4WU0vWO3IvIdD51470tFPD/7uwnSeAtkvKW HVNLtoEpFyqIQ== Message-ID: Date: Thu, 13 Oct 2022 22:42:32 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [u-boot][PATCH 10/14] mtd: rawnand: omap_gpmc: support u-boot driver model Content-Language: en-US To: Adam Ford , Tom Rini Cc: dario.binacchi@amarulasolutions.com, michael@amarulasolutions.com, trini@konsulko.com, u-boot@lists.denx.de References: <20221011115012.6181-1-rogerq@kernel.org> <20221011115012.6181-11-rogerq@kernel.org> <2b4e24dc-6eb6-81d5-fc14-ecd6923e4308@kernel.org> From: Roger Quadros In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 13/10/2022 14:17, Adam Ford wrote: > On Wed, Oct 12, 2022 at 6:42 AM Adam Ford wrote: >> >> On Wed, Oct 12, 2022 at 1:22 AM Roger Quadros wrote: >>> >>> Hi Adam, >>> >>> On 11/10/2022 18:01, Adam Ford wrote: >>>> On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros 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 >>>>> --- >>>>> 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 >>>>> #include >>>>> #include >>>>> +#include >>>>> #include >>>>> >>>>> #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