[U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
The current code would always use the speed and mode set by CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE. But if using SPI driver model it should get the values from DT.
Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com --- drivers/net/fm/fm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 00cdfd4..6308d22 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -371,8 +371,18 @@ int fm_init_common(int index, struct ccsr_fman *reg) void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); int ret = 0;
+#ifdef CONFIG_DM_SPI_FLASH + struct udevice *new; + + /* Will get the speed and mode from Device Tree */ + ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, + 0, 0, &new); + + ucode_flash = dev_get_uclass_priv(new); +#else ucode_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); +#endif if (!ucode_flash) printf("SF: probe for ucode failed\n"); else {
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(-)
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/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
________________________________ 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/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
On 07/20/2016 03:51 AM, Gong Qianyu wrote:
The current code would always use the speed and mode set by CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE. But if using SPI driver model it should get the values from DT.
Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com
drivers/net/fm/fm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 00cdfd4..6308d22 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -371,8 +371,18 @@ int fm_init_common(int index, struct ccsr_fman *reg) void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); int ret = 0;
+#ifdef CONFIG_DM_SPI_FLASH
- struct udevice *new;
- /* Will get the speed and mode from Device Tree */
- ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
0, 0, &new);- ucode_flash = dev_get_uclass_priv(new);
+#else ucode_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); +#endif if (!ucode_flash) printf("SF: probe for ucode failed\n"); else {
Why not just use spi_flash_probe() with speed and mode passed as 0?
York
-----Original Message----- From: york sun Sent: Thursday, July 28, 2016 1:35 AM 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 1/2] net: fm: fix spi flash probe for using driver model
On 07/20/2016 03:51 AM, Gong Qianyu wrote:
The current code would always use the speed and mode set by CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE. But if using SPI driver model it should get the values from DT.
Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com
drivers/net/fm/fm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 00cdfd4..6308d22 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -371,8 +371,18 @@ int fm_init_common(int index, struct ccsr_fman *reg) void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); int ret = 0;
+#ifdef CONFIG_DM_SPI_FLASH
- struct udevice *new;
- /* Will get the speed and mode from Device Tree */
- ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
0, 0, &new);- ucode_flash = dev_get_uclass_priv(new); #else ucode_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);+#endif if (!ucode_flash) printf("SF: probe for ucode failed\n"); else {
Why not just use spi_flash_probe() with speed and mode passed as 0?
York
As Simon said spi_flash_probe() "is an old-style function and would be removed when all SPI flash drivers use dm", so I think for dm spi_flash_probe_bus_cs() should be used.
Regards, Qianyu
On 28 July 2016 at 08:06, Qianyu Gong qianyu.gong@nxp.com wrote:
-----Original Message----- From: york sun Sent: Thursday, July 28, 2016 1:35 AM 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 1/2] net: fm: fix spi flash probe for using driver model
On 07/20/2016 03:51 AM, Gong Qianyu wrote:
The current code would always use the speed and mode set by CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE. But if using SPI driver model it should get the values from DT.
Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com
drivers/net/fm/fm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 00cdfd4..6308d22 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -371,8 +371,18 @@ int fm_init_common(int index, struct ccsr_fman *reg) void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); int ret = 0;
+#ifdef CONFIG_DM_SPI_FLASH
- struct udevice *new;
- /* Will get the speed and mode from Device Tree */
Below one look good phrase for me. /* speed and mode will be read from DT */
- ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
0, 0, &new);- ucode_flash = dev_get_uclass_priv(new); #else ucode_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);+#endif if (!ucode_flash) printf("SF: probe for ucode failed\n"); else {
Why not just use spi_flash_probe() with speed and mode passed as 0?
York
As Simon said spi_flash_probe() "is an old-style function and would be removed when all SPI flash drivers use dm", so I think for dm spi_flash_probe_bus_cs() should be used.
Correct!
Reviewed-by: Jagan Teki jteki@openedev.com
Hi Jagan,
Thanks. I'll fix it in the next version.
Regards, Qianyu
-----Original Message----- From: Jagan Teki [mailto:jagannadh.teki@gmail.com] Sent: Thursday, July 28, 2016 9:36 PM To: Qianyu Gong qianyu.gong@nxp.com Cc: york sun york.sun@nxp.com; u-boot@lists.denx.de; Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; Zhiqiang Hou zhiqiang.hou@nxp.com; Wenbin Song wenbin.song@nxp.com; Mingkai Hu mingkai.hu@nxp.com Subject: Re: [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
On 28 July 2016 at 08:06, Qianyu Gong qianyu.gong@nxp.com wrote:
-----Original Message----- From: york sun Sent: Thursday, July 28, 2016 1:35 AM 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 1/2] net: fm: fix spi flash probe for using driver model
On 07/20/2016 03:51 AM, Gong Qianyu wrote:
The current code would always use the speed and mode set by CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE. But if using SPI driver model it should get the values from DT.
Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com
drivers/net/fm/fm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 00cdfd4..6308d22 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -371,8 +371,18 @@ int fm_init_common(int index, struct ccsr_fman
*reg)
void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); int ret = 0;+#ifdef CONFIG_DM_SPI_FLASH
- struct udevice *new;
- /* Will get the speed and mode from Device Tree */
Below one look good phrase for me. /* speed and mode will be read from DT */
- ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
0, 0, &new);- ucode_flash = dev_get_uclass_priv(new); #else ucode_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);+#endif if (!ucode_flash) printf("SF: probe for ucode failed\n"); else {
Why not just use spi_flash_probe() with speed and mode passed as 0?
York
As Simon said spi_flash_probe() "is an old-style function and would be removed when all SPI flash drivers use dm", so I think for dm spi_flash_probe_bus_cs() should be used.
Correct!
Reviewed-by: Jagan Teki jteki@openedev.com
-- Jagan.
Hi York,
-----Original Message----- From: york sun Sent: Wednesday, August 03, 2016 12:08 AM To: Qianyu Gong qianyu.gong@nxp.com; 'Jagan Teki' jagannadh.teki@gmail.com Cc: u-boot@lists.denx.de; Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; Zhiqiang Hou zhiqiang.hou@nxp.com; Wenbin Song wenbin.song@nxp.com; Mingkai Hu mingkai.hu@nxp.com Subject: Re: [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
On 07/29/2016 04:00 AM, Qianyu Gong wrote:
Hi Jagan,
Thanks. I'll fix it in the next version.
Qianyu,
Is there anything you need to fix?
York
Yes. I just sent the v2 patch. And I also dropped the [PATCH 2/2]. I think I should first move the related SPI macros to defconfig before removing those env defines.
Regards, Qianyu
participants (4)
-
Gong Qianyu -
Jagan Teki -
Qianyu Gong
-
york sun