Re: [u-boot][PATCH 00/14] rawnand: omap_gpmc: driver model support

[Roger Quadros <rogerq@kernel.org>]

On 17/12/2022 15:51, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Minimal diff
> 
> diff --git a/configs/chiliboard_defconfig b/configs/chiliboard_defconfig
> index 458c4558fd..c7f8fd2e25 100644
> --- a/configs/chiliboard_defconfig
> +++ b/configs/chiliboard_defconfig
> @@ -47,12 +47,14 @@ CONFIG_CMD_MTDPARTS=y
>  CONFIG_MTDIDS_DEFAULT="nand0=8000000.nand"
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=8000000.nand:128k(NAND.SPL),128k(NAND.SPL.backup1),128k(NAND.SPL.backup2),128k(NAND.SPL.backup3),256k(NAND.u-boot-spl-os),1m(NAND.u-boot),128k(NAND.u-boot-env),128k(NAND.u-boot-env.backup1),8m(NAND.kernel),-(NAND.file-system)"
>  CONFIG_OF_CONTROL=y
> +CONFIG_SPL_OF_CONTROL=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_MMC=y
>  CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_NET_RETRY_COUNT=10
>  CONFIG_BOOTP_SEND_HOSTNAME=y
> +CONFIG_SPL_DM=y
>  CONFIG_BOOTCOUNT_LIMIT=y
>  CONFIG_SYS_BOOTCOUNT_BE=y
>  CONFIG_SYS_I2C_LEGACY=y

This worked perfectly.
But this platform my not yet be utilizing DM for NAND/ELM driver yet
so is this change acceptable?

cheers,
-roger

> 
> On Sat, Dec 17, 2022 at 2:46 PM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
>>
>> Hi
>>
>> take my config
>>
>> Michael
>>
>> On Sat, Dec 17, 2022 at 2:43 PM Roger Quadros <rogerq@kernel.org> wrote:
>>>
>>> On 17/12/2022 15:38, Michael Nazzareno Trimarchi wrote:
>>>> Hi
>>>>
>>>> On Sat, Dec 17, 2022 at 2:00 PM Roger Quadros <rogerq@kernel.org> wrote:
>>>>>
>>>>> Hi Michael & Dario,
>>>>>
>>>>> On 12/12/2022 11:39, Michael Nazzareno Trimarchi wrote:
>>>>>> Hi Roger
>>>>>>
>>>>>> Most of the building problem can be tested with this configuration
>>>>>>
>>>>>> make ARCH=arm chiliboard_defconfig
>>>>>
>>>>> I resolved the original issue for all boards but now face a new issue
>>>>> only with the chiliboard.
>>>>>
>>>>> arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/omap_elm.o: in function `dev_read_resource':
>>>>> /work/u-boot/include/dm/read.h:1139: undefined reference to `ofnode_read_resource'
>>>>>   AR      common/built-in.o
>>>>> make[2]: *** [/work/u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
>>>>> make[1]: *** [/work/u-boot/Makefile:2071: spl/u-boot-spl] Error 2
>>>>> make[1]: *** Waiting for unfinished jobs....
>>>>> make[1]: Leaving directory '/tmp'
>>>>> make: *** [Makefile:177: sub-make] Error 2
>>>>>
>>>>> The following config options are set
>>>>> CONFIG_DM_DEV_READ_INLINE=y
>>>>
>>>> What happen is that is not set?
>>>
>>> I removed "default y" for it in drivers/core/Kconfig
>>>
>>> Now I get
>>>
>>> arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/omap_elm.o: in function `elm_probe':
>>> /work/u-boot/drivers/mtd/nand/raw/omap_elm.c:206: undefined reference to `dev_read_resource'
>>> make[2]: *** [/work/u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
>>> make[1]: *** [/work/u-boot/Makefile:2071: spl/u-boot-spl] Error 2
>>> make[1]: *** Waiting for unfinished jobs....
>>>
>>> cheers,
>>> -roger
>>>
>>>>
>>>> Michael
>>>>
>>>>> CONFIG_SUPPORT_OF_CONTROL=y
>>>>>
>>>>> My understanding is that in case of spl build (Makefile.spl), the
>>>>> drivers/core/ofnode.o does not seem to be included
>>>>> thus causing the linking error.
>>>>>
>>>>> Any suggestions how to fix this?
>>>>>
>>>>> I've pushed the patches here
>>>>> https://github.com/rogerq/u-boot/commits/for-v2023.01/am64-nand-base-1.4
>>>>>
>>>>> cheers,
>>>>> -roger
>>>>>
>>>>>>
>>>>>> Michael
>>>>>>
>>>>>> On Mon, Dec 12, 2022 at 10:27 AM Dario Binacchi
>>>>>> <dario.binacchi@amarulasolutions.com> wrote:
>>>>>>>
>>>>>>> Hi Roger,
>>>>>>>
>>>>>>> On Mon, Dec 12, 2022 at 10:12 AM Roger Quadros <rogerq@kernel.org> wrote:
>>>>>>>>
>>>>>>>> Hi Dario,
>>>>>>>>
>>>>>>>> On 11/12/2022 15:56, Dario Binacchi wrote:
>>>>>>>>> Hi Roger,
>>>>>>>>>
>>>>>>>>> On Fri, Nov 25, 2022 at 1:38 PM Roger Quadros <rogerq@kernel.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Michael,
>>>>>>>>>>
>>>>>>>>>> On 08/11/2022 11:26, Michael Nazzareno Trimarchi wrote:
>>>>>>>>>>> Hi Roger
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Nov 4, 2022 at 2:27 PM Roger Quadros <rogerq@kernel.org> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> On 11/10/2022 14:49, Roger Quadros wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> This series adds driver model support for rawnand: omap_gpmc
>>>>>>>>>>>>> and omap_elm drivers.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This will enable the driver to be used on K2/K3 platforms as well.
>>>>>>>>>>>>
>>>>>>>>>>>> Any comments on patches 5 and later? Thanks
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> We will try to close this week.
>>>>>>>>>>
>>>>>>>>>> Could you please give your comments on the last few patches. Thanks!
>>>>>>>>>>
>>>>>>>>>> cheers,
>>>>>>>>>> -roger
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Michael
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> cheers,
>>>>>>>>>>>> -roger
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> cheers,
>>>>>>>>>>>>> -roger
>>>>>>>>>>>>>
>>>>>>>>>>>>> Roger Quadros (14):
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Deprecate asm/arch/mem.h
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Enable build for K2/K3 platforms
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Fix build warning on 64-bit platforms
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Optimize NAND reads
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction
>>>>>>>>>>>>>   mtd: rawnand: nand_base: Allow base driver to be used in SPL without
>>>>>>>>>>>>>     nand_bbt
>>>>>>>>>>>>>   mtd: rawnand: nand_spl_loaders: Fix cast type build warning
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Reduce .bss usage
>>>>>>>>>>>>>   dt-bindings: mtd: Add ti,gpmc-nand DT binding documentation
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: support u-boot driver model
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Add SPL NAND support
>>>>>>>>>>>>>   mtd: rawnand: omap_gpmc: Enable SYS_NAND_PAGE_COUNT for OMAP_GPMC
>>>>>>>>>>>>>   dt-bindings: mtd: Add ti,elm DT binding documentation
>>>>>>>>>>>>>   mtd: rawnand: omap_elm: u-boot driver model support
>>>>>>>>>>>>>
>>>>>>>>>>>>>  doc/device-tree-bindings/mtd/ti,elm.yaml      |  72 +++
>>>>>>>>>>>>>  .../mtd/ti,gpmc-nand.yaml                     | 129 +++++
>>>>>>>>>>>>>  drivers/mtd/nand/raw/Kconfig                  |  11 +-
>>>>>>>>>>>>>  drivers/mtd/nand/raw/Makefile                 |   2 +-
>>>>>>>>>>>>>  drivers/mtd/nand/raw/nand_base.c              |  18 +-
>>>>>>>>>>>>>  drivers/mtd/nand/raw/nand_spl_loaders.c       |   2 +-
>>>>>>>>>>>>>  drivers/mtd/nand/raw/omap_elm.c               |  33 +-
>>>>>>>>>>>>>  .../mtd => drivers/mtd/nand/raw}/omap_elm.h   |   6 +
>>>>>>>>>>>>>  drivers/mtd/nand/raw/omap_gpmc.c              | 500 +++++++++++++-----
>>>>>>>>>>>>>  9 files changed, 637 insertions(+), 136 deletions(-)
>>>>>>>>>>>>>  create mode 100644 doc/device-tree-bindings/mtd/ti,elm.yaml
>>>>>>>>>>>>>  create mode 100644 doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
>>>>>>>>>>>>>  rename {include/linux/mtd => drivers/mtd/nand/raw}/omap_elm.h (97%)
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I tried to merge your whole series but after the second fix and the
>>>>>>>>> third time the CI/CD pipeline failed
>>>>>>>>
>>>>>>>> Do you have the link to the failure?
>>>>>>>
>>>>>>> These are the CI/CD pipelines links:
>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/540827
>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/540876
>>>>>>> but I think you don't have permission to access them.
>>>>>>>
>>>>>>> Anyway:
>>>>>>>
>>>>>>> for https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/540827:
>>>>>>> +====================================================
>>>>>>> 345 arm: + am335x_guardian
>>>>>>> 346+drivers/mtd/nand/raw/omap_gpmc.c:1208:26: error: 'nand_chip'
>>>>>>> defined but not used [-Werror=unused-variable]
>>>>>>> 347+ 1208 | static struct nand_chip *nand_chip; /* First NAND chip for
>>>>>>> SPL use only */
>>>>>>> 348+ | ^~~~~~~~~
>>>>>>> 349+cc1: all warnings being treated as errors
>>>>>>> 350+make[5]: *** [scripts/Makefile.build:258:
>>>>>>> drivers/mtd/nand/raw/omap_gpmc.o] Error 1
>>>>>>> 351+make[4]: *** [scripts/Makefile.build:398: drivers/mtd/nand/raw] Error 2
>>>>>>> 352+make[3]: *** [scripts/Makefile.build:398: drivers/mtd/nand] Error 2
>>>>>>> 353+make[2]: *** [scripts/Makefile.build:398: drivers/mtd] Error 2
>>>>>>> 354+make[1]: *** [Makefile:1871: drivers] Error 2
>>>>>>> 355+make: *** [Makefile:177: sub-make] Error 2
>>>>>>>
>>>>>>> for https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/540876:
>>>>>>> +====================================================
>>>>>>> 498 arm: + chiliboard
>>>>>>> 499+arm-linux-gnueabi-ld.bfd: drivers/mtd/nand/raw/nand.o: in function
>>>>>>> `nand_init_chip':
>>>>>>> 500+drivers/mtd/nand/raw/nand.c:92: undefined reference to `board_nand_init'
>>>>>>> 501+make[1]: *** [Makefile:1778: u-boot] Error 1
>>>>>>> 502+make: *** [Makefile:177: sub-make] Error 2
>>>>>>> 503 arm: w+ am335x_shc_netboot
>>>>>>> 504+===================== WARNING ======================
>>>>>>> 505+This board does not use CONFIG_DM_I2C (Driver Model
>>>>>>> 506+for I2C drivers). Please update the board to use
>>>>>>> 507+CONFIG_DM_I2C before the v2022.04 release. Failure to
>>>>>>> 508+update by the deadline may result in board removal.
>>>>>>> 509+See doc/develop/driver-model/migration.rst for more info.
>>>>>>> 510+====================================================
>>>>>>> 511 arm: + cm_t43
>>>>>>> 512+arm-linux-gnueabi-ld.bfd: drivers/mtd/nand/raw/nand.o: in function
>>>>>>> `nand_init_chip':
>>>>>>> 513+drivers/mtd/nand/raw/nand.c:92: undefined reference to `board_nand_init'
>>>>>>> 514+make[1]: *** [Makefile:1778: u-boot] Error 1
>>>>>>> 515+make: *** [Makefile:177: sub-make] Error 2
>>>>>>> 516 arm: w+ am335x_shc_sdboot
>>>>>>>
>>>>>>> In both cases failed the "build all 32bit ARM platforms" test.
>>>>>>>
>>>>>>> I think you have to run the command:
>>>>>>> ./tools/buildman/buildman -o /tmp -PEWM arm -x aarch64
>>>>>>> if you have to run the tests locally.
>>>>>>>
>>>>>>> Thanks and regards,
>>>>>>> Dario
>>>>>>>
>>>>>>>>
>>>>>>>>> I thought it's better you fix the problems. So, I only accepted some
>>>>>>>>> of the first few patches in the series:
>>>>>>>>> 01/14 mtd: rawnand: omap_gpmc: Deprecate asm/arch/mem.h
>>>>>>>>> 02/14 mtd: rawnand: omap_gpmc: Enable build for K2/K3 platforms
>>>>>>>>> 03/14 mtd: rawnand: omap_gpmc: Fix build warning on 64-bit platforms
>>>>>>>>> 04/14 mtd: rawnand: omap_gpmc: Optimize NAND reads
>>>>>>>>> 07/14 mtd: rawnand: nand_spl_loaders: Fix cast type build warning
>>>>>>>>> 08/14 mtd: rawnand: omap_gpmc: Reduce .bss usage
>>>>>>>>>
>>>>>>>>> For the others, please fix them to run the tests successfully.
>>>>>>>>
>>>>>>>> No problem. I will try to fix and run them through the CI testing myself
>>>>>>>> before re-posting.
>>>>>>>>
>>>>>>>> cheers,
>>>>>>>> -roger
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Dario Binacchi
>>>>>>>
>>>>>>> Embedded Linux Developer
>>>>>>>
>>>>>>> dario.binacchi@amarulasolutions.com
>>>>>>>
>>>>>>> __________________________________
>>>>>>>
>>>>>>>
>>>>>>> Amarula Solutions SRL
>>>>>>>
>>>>>>> Via Le Canevare 30, 31100 Treviso, Veneto, IT
>>>>>>>
>>>>>>> T. +39 042 243 5310
>>>>>>> info@amarulasolutions.com
>>>>>>>
>>>>>>> www.amarulasolutions.com
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>> --
>> Michael Nazzareno Trimarchi
>> Co-Founder & Chief Executive Officer
>> M. +39 347 913 2170
>> michael@amarulasolutions.com
>> __________________________________
>>
>> Amarula Solutions BV
>> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
>> T. +31 (0)85 111 9172
>> info@amarulasolutions.com
>> www.amarulasolutions.com
> 
> 
>