[U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model

[Qianyu Gong]

Hi York,

> -----Original Message-----
> From: york sun
> Sent: Wednesday, July 27, 2016 10:55 PM
> To: Qianyu Gong <qianyu.gong at nxp.com>; u-boot at lists.denx.de; Simon Glass
> <sjg at chromium.org>
> Cc: Shaohui Xie <shaohui.xie at nxp.com>; Zhiqiang Hou <zhiqiang.hou at nxp.com>;
> Wenbin Song <wenbin.song at nxp.com>; Yao Yuan <yao.yuan at nxp.com>; Mingkai
> Hu <mingkai.hu at nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>
> Subject: Re: [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver
> model
> 
> On 07/27/2016 03:00 AM, Qianyu Gong wrote:
> >
> > Hi York,
> >
> >> -----Original Message-----
> >> From: york sun
> >> Sent: Tuesday, July 26, 2016 12:26 PM
> >> To: Qianyu Gong <qianyu.gong at nxp.com>; u-boot at lists.denx.de;
> >> Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> >> <mingkai.hu at nxp.com>
> >> Cc: Shaohui Xie <shaohui.xie at nxp.com>; Zhiqiang Hou
> >> <zhiqiang.hou at nxp.com>; Wenbin Song <wenbin.song at nxp.com>
> >> Subject: Re: [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if
> >> using driver model
> >>
> >> On 07/25/2016 09:05 PM, Qianyu Gong wrote:
> >>> Hi York,
> >>>
> >>>
> >>> As the drivel model is a trend anyway, I just doubt if it is
> >>> necessary to support non-DM for the new platforms.
> >>>
> >>> In fact, we have discarded configurations for non-DM SPI such as SPI
> >>> mode related macros
> >>>
> >>> when doing LS1043A upstream. So the current configuration of LS1043A
> >>> doesn't support non-DM SPI.
> >>>
> >>>
> >>> LS1012A supports both ways but the code doesn't differentiate the
> >>> respective macros.
> >>>
> >>> The CONFIG_ENV_SPI_* are set for FMAN ucode at the beginning but I
> >>> just find that LS1012A doesn't have FMAN. So it's dead code if using
> >>> DM or just duplicated code that is the same with defines in
> >>> common/env_sf.c if using non-DM.
> >>
> >> Qianyu,
> >>
> >> If DM_SPI_FLASH should always be set, please select it from Kconfig.
> >>
> >> York
> >>
> >>
> >
> > For LS1043A, DM_SPI_FLASH is still defined in
> include/configs/ls1043a_common.h.
> > So I think it won't be affected by menuconfig. But it should have been moved to
> defconfig.
> >
> > As DM_SPI_FLASH doesn't depend on any platforms as per
> > "drivers/mtd/spi/Kconfig", I can just focus on solving the issue
> > caused by deselecting DM_SPI_FLASH. I also discussed with Yuan Yao.
> >
> > So how about I adding anything in Fman Kconfig like this?
> > "
> > config SYS_QE_FW_IN_SPIFLASH
> >         depends on (FSL_LAYERSCAPE && DM_SPI_FLASH) || PPC "
> > But as for the existing code, it may need more efforts.
> >
> 
> I think you can add "select" for the platforms which always use DM_SPI_FLASH, for
> example TARGET_LS1043AQDS.
> 
> Simon,
> 
> Please comment if this is a good practice.
> 
> York
> 

If one doesn't select DSPI/QSPI on LS1043A boards, there would be no need to 
select DM_SPI_FLASH. So to some extent it's not always used, correct? 

Regards,
Qianyu

> >
> > Regards,
> > Qianyu
> >
> >>>
> >>>
> >>>
> >>> Regards,
> >>>
> >>> Qianyu
> >>>
> >>> --------------------------------------------------------------------
> >>> --
> >>> --
> >>> *From:* york sun
> >>> *Sent:* Tuesday, July 26, 2016 6:15:14 AM
> >>> *To:* Qianyu Gong; u-boot at lists.denx.de; Prabhakar Kushwaha; Mingkai
> >>> Hu
> >>> *Cc:* Shaohui Xie; Zhiqiang Hou; Wenbin Song
> >>> *Subject:* Re: [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_*
> >>> if using driver model
> >>>
> >>> On 07/20/2016 03:51 AM, Gong Qianyu wrote:
> >>>> When using SPI driver model, it will get the values from DT. So
> >>>> there is no need to set CONFIG_ENV_SPI_MAX_HZ and
> >>>> CONFIG_ENV_SPI_MODE any more.
> >>>>
> >>>
> >>> You indicate these macros are not needed _if_ using driver model.
> >>> You presume the driver model is always used. You have
> >>> CONFIG_DM_SPI_FLASH in defconfig, but you don't have it selected in
> >>> Kconfig for those platforms. This can leave a possible configuration
> >>> if one runs "make menuconfig" and deselect DM_SPI_FLASH.
> >>>
> >>> York
> >>>
> >>>
> >>>> Signed-off-by: Gong Qianyu <Qianyu.Gong at nxp.com>
> >>>> ---
> >>>>  include/configs/ls1012a_common.h | 2 --
> >>>> include/configs/ls1043a_common.h | 2 --
> >>>>  2 files changed, 4 deletions(-)
> >>>>
> >>>> diff --git a/include/configs/ls1012a_common.h
> >>>> b/include/configs/ls1012a_common.h
> >>>> index fba2fac..1602f09 100644
> >>>> --- a/include/configs/ls1012a_common.h
> >>>> +++ b/include/configs/ls1012a_common.h
> >>>> @@ -52,8 +52,6 @@
> >>>>  #define CONFIG_SYS_FMAN_FW_ADDR              0x400d0000
> >>>>  #define CONFIG_ENV_SPI_BUS           0
> >>>>  #define CONFIG_ENV_SPI_CS            0
> >>>> -#define CONFIG_ENV_SPI_MAX_HZ                1000000
> >>>> -#define CONFIG_ENV_SPI_MODE          0x03
> >>>>  #define CONFIG_SPI_FLASH_SPANSION
> >>>>  #define CONFIG_FSL_SPI_INTERFACE
> >>>>  #define CONFIG_SF_DATAFLASH
> >>>> diff --git a/include/configs/ls1043a_common.h
> >>>> b/include/configs/ls1043a_common.h
> >>>> index b0d4a8d..028f7d9 100644
> >>>> --- a/include/configs/ls1043a_common.h
> >>>> +++ b/include/configs/ls1043a_common.h
> >>>> @@ -222,8 +222,6 @@
> >>>>  #define CONFIG_SYS_FMAN_FW_ADDR              0x400d0000
> >>>>  #define CONFIG_ENV_SPI_BUS           0
> >>>>  #define CONFIG_ENV_SPI_CS            0
> >>>> -#define CONFIG_ENV_SPI_MAX_HZ                1000000
> >>>> -#define CONFIG_ENV_SPI_MODE          0x03
> >>>>  #else
> >>>>  #define CONFIG_SYS_QE_FMAN_FW_IN_NOR
> >>>>  /* FMan fireware Pre-load address */
> >>>>
> >>>
> >
> >