| Message ID | 1469011169-39131-2-git-send-email-Qianyu.Gong@nxp.com |
|---|---|
| State | Deferred |
| Delegated to: | York Sun |
| Headers | show |
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@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 */ >
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. Regards, Qianyu
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 > > > > Regards, > > Qianyu > > ------------------------------------------------------------------------ > *From:* york sun > *Sent:* Tuesday, July 26, 2016 6:15:14 AM > *To:* Qianyu Gong; u-boot@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@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 */ >> >
Hi York, > -----Original Message----- > From: york sun > Sent: Tuesday, July 26, 2016 12:26 PM > To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot@lists.denx.de; Prabhakar > Kushwaha <prabhakar.kushwaha@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com> > Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou <zhiqiang.hou@nxp.com>; > Wenbin Song <wenbin.song@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. Regards, Qianyu > > > > > > > > Regards, > > > > Qianyu > > > > ---------------------------------------------------------------------- > > -- > > *From:* york sun > > *Sent:* Tuesday, July 26, 2016 6:15:14 AM > > *To:* Qianyu Gong; u-boot@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@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 */ > >> > >
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@nxp.com>; u-boot@lists.denx.de; Prabhakar >> Kushwaha <prabhakar.kushwaha@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com> >> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou <zhiqiang.hou@nxp.com>; >> Wenbin Song <wenbin.song@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 > > Regards, > Qianyu > >>> >>> >>> >>> Regards, >>> >>> Qianyu >>> >>> ---------------------------------------------------------------------- >>> -- >>> *From:* york sun >>> *Sent:* Tuesday, July 26, 2016 6:15:14 AM >>> *To:* Qianyu Gong; u-boot@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@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 */ >>>> >>> > >
Hi York, > -----Original Message----- > From: york sun > Sent: Wednesday, July 27, 2016 10:55 PM > To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot@lists.denx.de; Simon Glass > <sjg@chromium.org> > Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou <zhiqiang.hou@nxp.com>; > Wenbin Song <wenbin.song@nxp.com>; Yao Yuan <yao.yuan@nxp.com>; Mingkai > Hu <mingkai.hu@nxp.com>; Prabhakar Kushwaha > <prabhakar.kushwaha@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@nxp.com>; u-boot@lists.denx.de; > >> Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Mingkai Hu > >> <mingkai.hu@nxp.com> > >> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou > >> <zhiqiang.hou@nxp.com>; Wenbin Song <wenbin.song@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@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@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 */ > >>>> > >>> > > > >
On 07/27/2016 07:57 PM, Qianyu Gong wrote: > Hi York, > >> -----Original Message----- >> From: york sun >> Sent: Wednesday, July 27, 2016 10:55 PM >> To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot@lists.denx.de; Simon Glass >> <sjg@chromium.org> >> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou <zhiqiang.hou@nxp.com>; >> Wenbin Song <wenbin.song@nxp.com>; Yao Yuan <yao.yuan@nxp.com>; Mingkai >> Hu <mingkai.hu@nxp.com>; Prabhakar Kushwaha >> <prabhakar.kushwaha@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@nxp.com>; u-boot@lists.denx.de; >>>> Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Mingkai Hu >>>> <mingkai.hu@nxp.com> >>>> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Zhiqiang Hou >>>> <zhiqiang.hou@nxp.com>; Wenbin Song <wenbin.song@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? > If SPI is not always enabled, you don't need to select DM_SPI_FLASH. Just be aware, you can end up with a config in which legacy SPI enabled, but DM_SPI_FLASH not enabled, if you run menuconfig. York
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 */
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. Signed-off-by: Gong Qianyu <Qianyu.Gong@nxp.com> --- include/configs/ls1012a_common.h | 2 -- include/configs/ls1043a_common.h | 2 -- 2 files changed, 4 deletions(-)