* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
@ 2016-07-20 10:39 Gong Qianyu
2016-07-20 10:39 ` [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if " Gong Qianyu
2016-07-27 17:34 ` [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for " york sun
0 siblings, 2 replies; 15+ messages in thread
From: Gong Qianyu @ 2016-07-20 10:39 UTC (permalink / raw)
To: u-boot
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 {
--
2.1.0.27.g96db324
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-20 10:39 [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model Gong Qianyu
@ 2016-07-20 10:39 ` Gong Qianyu
2016-07-25 22:15 ` york sun
2016-07-27 17:34 ` [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for " york sun
1 sibling, 1 reply; 15+ messages in thread
From: Gong Qianyu @ 2016-07-20 10:39 UTC (permalink / raw)
To: u-boot
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 */
--
2.1.0.27.g96db324
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-20 10:39 ` [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if " Gong Qianyu
@ 2016-07-25 22:15 ` york sun
2016-07-26 4:05 ` Qianyu Gong
0 siblings, 1 reply; 15+ messages in thread
From: york sun @ 2016-07-25 22:15 UTC (permalink / raw)
To: u-boot
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 */
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-25 22:15 ` york sun
@ 2016-07-26 4:05 ` Qianyu Gong
2016-07-26 4:26 ` york sun
0 siblings, 1 reply; 15+ messages in thread
From: Qianyu Gong @ 2016-07-26 4:05 UTC (permalink / raw)
To: u-boot
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 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@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 */
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-26 4:05 ` Qianyu Gong
@ 2016-07-26 4:26 ` york sun
2016-07-27 10:00 ` Qianyu Gong
0 siblings, 1 reply; 15+ messages in thread
From: york sun @ 2016-07-26 4:26 UTC (permalink / raw)
To: u-boot
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 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@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 */
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-26 4:26 ` york sun
@ 2016-07-27 10:00 ` Qianyu Gong
2016-07-27 14:55 ` york sun
0 siblings, 1 reply; 15+ messages in thread
From: Qianyu Gong @ 2016-07-27 10:00 UTC (permalink / raw)
To: u-boot
Hi York,
> -----Original Message-----
> From: york sun
> Sent: Tuesday, July 26, 2016 12:26 PM
> To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot at 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 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@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 */
> >>
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-27 10:00 ` Qianyu Gong
@ 2016-07-27 14:55 ` york sun
2016-07-28 2:57 ` Qianyu Gong
0 siblings, 1 reply; 15+ messages in thread
From: york sun @ 2016-07-27 14:55 UTC (permalink / raw)
To: u-boot
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 at 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 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@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 */
>>>>
>>>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-07-20 10:39 [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model Gong Qianyu
2016-07-20 10:39 ` [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if " Gong Qianyu
@ 2016-07-27 17:34 ` york sun
2016-07-28 2:36 ` Qianyu Gong
1 sibling, 1 reply; 15+ messages in thread
From: york sun @ 2016-07-27 17:34 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-07-27 17:34 ` [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for " york sun
@ 2016-07-28 2:36 ` Qianyu Gong
2016-07-28 13:35 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Qianyu Gong @ 2016-07-28 2:36 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: york sun
> Sent: Thursday, July 28, 2016 1:35 AM
> To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot at 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-27 14:55 ` york sun
@ 2016-07-28 2:57 ` Qianyu Gong
2016-07-28 15:50 ` york sun
0 siblings, 1 reply; 15+ messages in thread
From: Qianyu Gong @ 2016-07-28 2:57 UTC (permalink / raw)
To: u-boot
Hi York,
> -----Original Message-----
> From: york sun
> Sent: Wednesday, July 27, 2016 10:55 PM
> To: Qianyu Gong <qianyu.gong@nxp.com>; u-boot at 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 at 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 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@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 */
> >>>>
> >>>
> >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-07-28 2:36 ` Qianyu Gong
@ 2016-07-28 13:35 ` Jagan Teki
2016-07-29 11:00 ` Qianyu Gong
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2016-07-28 13:35 UTC (permalink / raw)
To: u-boot
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 at 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if using driver model
2016-07-28 2:57 ` Qianyu Gong
@ 2016-07-28 15:50 ` york sun
0 siblings, 0 replies; 15+ messages in thread
From: york sun @ 2016-07-28 15:50 UTC (permalink / raw)
To: u-boot
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 at 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 at 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-07-28 13:35 ` Jagan Teki
@ 2016-07-29 11:00 ` Qianyu Gong
2016-08-02 16:08 ` york sun
0 siblings, 1 reply; 15+ messages in thread
From: Qianyu Gong @ 2016-07-29 11:00 UTC (permalink / raw)
To: u-boot
Hi Jagan,
Thanks. I'll fix it in the next version.
Regards,
Qianyu
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at 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 at 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 at 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-07-29 11:00 ` Qianyu Gong
@ 2016-08-02 16:08 ` york sun
2016-08-03 3:29 ` Qianyu Gong
0 siblings, 1 reply; 15+ messages in thread
From: york sun @ 2016-08-02 16:08 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model
2016-08-02 16:08 ` york sun
@ 2016-08-03 3:29 ` Qianyu Gong
0 siblings, 0 replies; 15+ messages in thread
From: Qianyu Gong @ 2016-08-03 3:29 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-08-03 3:29 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-20 10:39 [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for using driver model Gong Qianyu
2016-07-20 10:39 ` [U-Boot] [PATCH 2/2] config.h: clean unused CONFIG_ENV_SPI_* if " Gong Qianyu
2016-07-25 22:15 ` york sun
2016-07-26 4:05 ` Qianyu Gong
2016-07-26 4:26 ` york sun
2016-07-27 10:00 ` Qianyu Gong
2016-07-27 14:55 ` york sun
2016-07-28 2:57 ` Qianyu Gong
2016-07-28 15:50 ` york sun
2016-07-27 17:34 ` [U-Boot] [PATCH 1/2] net: fm: fix spi flash probe for " york sun
2016-07-28 2:36 ` Qianyu Gong
2016-07-28 13:35 ` Jagan Teki
2016-07-29 11:00 ` Qianyu Gong
2016-08-02 16:08 ` york sun
2016-08-03 3:29 ` Qianyu Gong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.