[EXT] Re: [PATCH v9 11/14] Layerscape: Enable Job ring driver model.

[Gaurav Jain]

Hi Michael

> -----Original Message-----
> From: Michael Walle <michael at walle.cc>
> Sent: Tuesday, January 11, 2022 5:41 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: u-boot at lists.denx.de; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>; Simon Glass
> <sjg at chromium.org>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>; Silvano Di
> Ninno <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-
> uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>;
> Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: [EXT] Re: [PATCH v9 11/14] Layerscape: Enable Job ring driver model.
> 
> Caution: EXT Email
> 
> Hi Gaurav,
> 
> Am 2022-01-11 12:28, schrieb Gaurav Jain:
> > @@ -1329,6 +1331,7 @@ config TARGET_LS2080ARDB
> >       select FSL_DDR_BIST
> >       select FSL_DDR_INTERACTIVE if !SPL
> >       select GPIO_EXTRA_HEADER
> > +     select FSL_CAAM
> 
> See below.
> 
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > index 2ded3e4efc..29aa9a3f10 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0+
> >  /*
> > - * Copyright 2017-2020 NXP
> > + * Copyright 2017-2021 NXP
> >   * Copyright 2014-2015 Freescale Semiconductor, Inc.
> >   */
> >
> > @@ -49,6 +49,7 @@
> >  #endif
> >  #endif
> >  #include <linux/mii.h>
> > +#include <dm.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -1650,6 +1651,13 @@ __weak int serdes_misc_init(void)
> >
> >  int arch_misc_init(void)
> >  {
> > +     struct udevice *dev;
> > +     int ret;
> > +
> > +     ret = uclass_get_device_by_driver(UCLASS_MISC,
> > DM_DRIVER_GET(caam_jr), &dev);
> > +     if (ret)
> > +             printf("Failed to initialize %s: %d\n", dev->name, ret);
> 
> This will fail when the CAAM driver isn't enabled and will then print this error
> message. Should be guarded with IS_ENABLED(FSL_CAAM).
I agree. Will add this.
If you have more comments, please share.
I will check and incorporate the changes in single version.

> 
> 
> > diff --git a/configs/ls2088ardb_tfa_defconfig
> > b/configs/ls2088ardb_tfa_defconfig
> > index a28b45b129..5230d7ead8 100644
> > --- a/configs/ls2088ardb_tfa_defconfig
> > +++ b/configs/ls2088ardb_tfa_defconfig
> > @@ -53,7 +53,6 @@ CONFIG_ENV_ADDR=0x580500000
> > CONFIG_NET_RANDOM_ETHADDR=y  CONFIG_DM=y  CONFIG_SATA_CEVA=y
> > -CONFIG_FSL_CAAM=y
> >  CONFIG_DDR_CLK_FREQ=133333333
> >  CONFIG_DDR_ECC=y
> >  CONFIG_ECC_INIT_VIA_DDRCONTROLLER=y
> ..
> 
> I still don't understand why you make the FSL_CAAM mandatory and remove the
> option here. Same for all other boards.
Instead of enabling FSL_CAAM in every defconfig for a particular board, I moved FSL_CAAM to specific TARGET boards.
this will enable FSL_CAAM to every defconfig for a target board.

Regards
Gaurav Jain
> 
> -michael