* [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory
@ 2020-01-08 9:08 Kuldeep Singh
2020-01-08 9:21 ` Schrempf Frieder
0 siblings, 1 reply; 5+ messages in thread
From: Kuldeep Singh @ 2020-01-08 9:08 UTC (permalink / raw)
To: u-boot
Current PFE firmware access spi-nor memory directly. New spi-mem
framework does not support direct memory access. So, let's use
spi_flash_read API to access memory instead of directly using it.
Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
---
v3:
-Replace ret with 0 if flash probe fails
v2:
-Add return error code
-Some changes in displayed log
drivers/net/pfe_eth/pfe_firmware.c | 45 +++++++++++++++++++++++++++++++++++++-
include/configs/ls1012a_common.h | 5 ++++-
2 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
index e4563f1..6145f4e 100644
--- a/drivers/net/pfe_eth/pfe_firmware.c
+++ b/drivers/net/pfe_eth/pfe_firmware.c
@@ -12,13 +12,14 @@
#include <net/pfe_eth/pfe_eth.h>
#include <net/pfe_eth/pfe_firmware.h>
+#include <spi_flash.h>
#ifdef CONFIG_CHAIN_OF_TRUST
#include <fsl_validate.h>
#endif
#define PFE_FIRMWARE_FIT_CNF_NAME "config at 1"
-static const void *pfe_fit_addr = (void *)CONFIG_SYS_LS_PFE_FW_ADDR;
+static const void *pfe_fit_addr;
/*
* PFE elf firmware loader.
@@ -159,6 +160,44 @@ static int pfe_fit_check(void)
return ret;
}
+int pfe_spi_flash_init(void)
+{
+ struct spi_flash *pfe_flash;
+ int ret = 0;
+ void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
+
+#ifdef CONFIG_DM_SPI_FLASH
+ struct udevice *new;
+
+ /* 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);
+
+ pfe_flash = dev_get_uclass_priv(new);
+#else
+ pfe_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
+ CONFIG_ENV_SPI_CS,
+ CONFIG_ENV_SPI_MAX_HZ,
+ CONFIG_ENV_SPI_MODE);
+#endif
+ if (!pfe_flash) {
+ printf("SF: probe for pfe failed\n");
+ return 0;
+ }
+
+ ret = spi_flash_read(pfe_flash,
+ CONFIG_SYS_LS_PFE_FW_ADDR,
+ CONFIG_SYS_QE_FMAN_FW_LENGTH,
+ addr);
+ if (ret)
+ printf("SF: read for pfe failed\n");
+
+ pfe_fit_addr = addr;
+ spi_flash_free(pfe_flash);
+
+ return ret;
+}
+
/*
* PFE firmware initialization.
* Loads different firmware files from FIT image.
@@ -183,6 +222,10 @@ int pfe_firmware_init(void)
int ret = 0;
int fw_count;
+ ret = pfe_spi_flash_init();
+ if (ret)
+ goto err;
+
ret = pfe_fit_check();
if (ret)
goto err;
diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h
index 2579e2f..cbc5bff 100644
--- a/include/configs/ls1012a_common.h
+++ b/include/configs/ls1012a_common.h
@@ -36,9 +36,12 @@
/* Size of malloc() pool */
#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 * 1024)
+/* PFE */
+#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
+#define CONFIG_SYS_QE_FMAN_FW_LENGTH 0x10000
+
/*SPI device */
#if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_TFABOOT)
-#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
#define CONFIG_SPI_FLASH_SPANSION
#define CONFIG_FSL_SPI_INTERFACE
#define CONFIG_SF_DATAFLASH
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory
2020-01-08 9:08 [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory Kuldeep Singh
@ 2020-01-08 9:21 ` Schrempf Frieder
2020-01-10 10:58 ` [EXT] " Kuldeep Singh
0 siblings, 1 reply; 5+ messages in thread
From: Schrempf Frieder @ 2020-01-08 9:21 UTC (permalink / raw)
To: u-boot
On 08.01.20 10:08, Kuldeep Singh wrote:
> Current PFE firmware access spi-nor memory directly. New spi-mem
> framework does not support direct memory access. So, let's use
> spi_flash_read API to access memory instead of directly using it.
>
> Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
> ---
> v3:
> -Replace ret with 0 if flash probe fails
> v2:
> -Add return error code
> -Some changes in displayed log
>
> drivers/net/pfe_eth/pfe_firmware.c | 45 +++++++++++++++++++++++++++++++++++++-
> include/configs/ls1012a_common.h | 5 ++++-
> 2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
> index e4563f1..6145f4e 100644
> --- a/drivers/net/pfe_eth/pfe_firmware.c
> +++ b/drivers/net/pfe_eth/pfe_firmware.c
> @@ -12,13 +12,14 @@
>
> #include <net/pfe_eth/pfe_eth.h>
> #include <net/pfe_eth/pfe_firmware.h>
> +#include <spi_flash.h>
> #ifdef CONFIG_CHAIN_OF_TRUST
> #include <fsl_validate.h>
> #endif
>
> #define PFE_FIRMWARE_FIT_CNF_NAME "config at 1"
>
> -static const void *pfe_fit_addr = (void *)CONFIG_SYS_LS_PFE_FW_ADDR;
> +static const void *pfe_fit_addr;
>
> /*
> * PFE elf firmware loader.
> @@ -159,6 +160,44 @@ static int pfe_fit_check(void)
> return ret;
> }
>
> +int pfe_spi_flash_init(void)
> +{
> + struct spi_flash *pfe_flash;
> + int ret = 0;
> + void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
> +
> +#ifdef CONFIG_DM_SPI_FLASH
> + struct udevice *new;
> +
> + /* 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);
> +
> + pfe_flash = dev_get_uclass_priv(new);
> +#else
> + pfe_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
> + CONFIG_ENV_SPI_CS,
> + CONFIG_ENV_SPI_MAX_HZ,
> + CONFIG_ENV_SPI_MODE);
> +#endif
> + if (!pfe_flash) {
> + printf("SF: probe for pfe failed\n");
> + return 0;
No again. It seems like you're not getting what I'm trying to tell you,
so it would be better to just ask back instead of posting new versions.
pfe_spi_flash_init() should return an error code in case the probe of
the SPI flash failed. An error code is a negative value like for example
-ENODEV to report that the device is not available to the calling function.
> + }
> +
> + ret = spi_flash_read(pfe_flash,
> + CONFIG_SYS_LS_PFE_FW_ADDR,
> + CONFIG_SYS_QE_FMAN_FW_LENGTH,
> + addr);
> + if (ret)
> + printf("SF: read for pfe failed\n");
> +
> + pfe_fit_addr = addr;
> + spi_flash_free(pfe_flash);
> +
> + return ret;
> +}
> +
> /*
> * PFE firmware initialization.
> * Loads different firmware files from FIT image.
> @@ -183,6 +222,10 @@ int pfe_firmware_init(void)
> int ret = 0;
> int fw_count;
>
> + ret = pfe_spi_flash_init();
> + if (ret)
> + goto err;
> +
> ret = pfe_fit_check();
> if (ret)
> goto err;
> diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h
> index 2579e2f..cbc5bff 100644
> --- a/include/configs/ls1012a_common.h
> +++ b/include/configs/ls1012a_common.h
> @@ -36,9 +36,12 @@
> /* Size of malloc() pool */
> #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 * 1024)
>
> +/* PFE */
> +#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> +#define CONFIG_SYS_QE_FMAN_FW_LENGTH 0x10000
> +
> /*SPI device */
> #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_TFABOOT)
> -#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> #define CONFIG_SPI_FLASH_SPANSION
> #define CONFIG_FSL_SPI_INTERFACE
> #define CONFIG_SF_DATAFLASH
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory
2020-01-08 9:21 ` Schrempf Frieder
@ 2020-01-10 10:58 ` Kuldeep Singh
2020-01-13 8:37 ` Schrempf Frieder
0 siblings, 1 reply; 5+ messages in thread
From: Kuldeep Singh @ 2020-01-10 10:58 UTC (permalink / raw)
To: u-boot
Hi Frieder,
> -----Original Message-----
> From: Schrempf Frieder <frieder.schrempf@kontron.de>
> Sent: Wednesday, January 8, 2020 2:52 PM
> To: Kuldeep Singh <kuldeep.singh@nxp.com>; u-boot at lists.denx.de
> Cc: Joe Hershberger <joe.hershberger@ni.com>; Thomas Hebb
> <tommyhebb@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>
> Subject: [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access
> flash memory
>
> Caution: EXT Email
>
> On 08.01.20 10:08, Kuldeep Singh wrote:
> > Current PFE firmware access spi-nor memory directly. New spi-mem
> > framework does not support direct memory access. So, let's use
> > spi_flash_read API to access memory instead of directly using it.
> >
> > Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
> > ---
> > v3:
> > -Replace ret with 0 if flash probe fails
> > v2:
> > -Add return error code
> > -Some changes in displayed log
> >
> > drivers/net/pfe_eth/pfe_firmware.c | 45
> +++++++++++++++++++++++++++++++++++++-
> > include/configs/ls1012a_common.h | 5 ++++-
> > 2 files changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/pfe_eth/pfe_firmware.c
> > b/drivers/net/pfe_eth/pfe_firmware.c
> > index e4563f1..6145f4e 100644
> > --- a/drivers/net/pfe_eth/pfe_firmware.c
> > +++ b/drivers/net/pfe_eth/pfe_firmware.c
> > @@ -12,13 +12,14 @@
> >
> > #include <net/pfe_eth/pfe_eth.h>
> > #include <net/pfe_eth/pfe_firmware.h>
> > +#include <spi_flash.h>
> > #ifdef CONFIG_CHAIN_OF_TRUST
> > #include <fsl_validate.h>
> > #endif
> >
> > #define PFE_FIRMWARE_FIT_CNF_NAME "config at 1"
> >
> > -static const void *pfe_fit_addr = (void *)CONFIG_SYS_LS_PFE_FW_ADDR;
> > +static const void *pfe_fit_addr;
> >
> > /*
> > * PFE elf firmware loader.
> > @@ -159,6 +160,44 @@ static int pfe_fit_check(void)
> > return ret;
> > }
> >
> > +int pfe_spi_flash_init(void)
> > +{
> > + struct spi_flash *pfe_flash;
> > + int ret = 0;
> > + void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
> > +
> > +#ifdef CONFIG_DM_SPI_FLASH
> > + struct udevice *new;
> > +
> > + /* 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);
> > +
> > + pfe_flash = dev_get_uclass_priv(new); #else
> > + pfe_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
> > + CONFIG_ENV_SPI_CS,
> > + CONFIG_ENV_SPI_MAX_HZ,
> > + CONFIG_ENV_SPI_MODE); #endif
> > + if (!pfe_flash) {
> > + printf("SF: probe for pfe failed\n");
> > + return 0;
>
> No again. It seems like you're not getting what I'm trying to tell you, so it
> would be better to just ask back instead of posting new versions.
>
> pfe_spi_flash_init() should return an error code in case the probe of the SPI
> flash failed. An error code is a negative value like for example -ENODEV to
> report that the device is not available to the calling function.
Thanks Frieder for providing the info.
I will return -ENODEV here instead of 0.
>
> > + }
> > +
> > + ret = spi_flash_read(pfe_flash,
> > + CONFIG_SYS_LS_PFE_FW_ADDR,
> > + CONFIG_SYS_QE_FMAN_FW_LENGTH,
> > + addr);
> > + if (ret)
> > + printf("SF: read for pfe failed\n");
I think -EIO should also be returned here as pfe functionality will fail if data is not read.
Please confirm the same if error code is correct/required. I will update this in next version.
Thanks
Kuldeep
> > +
> > + pfe_fit_addr = addr;
> > + spi_flash_free(pfe_flash);
> > +
> > + return ret;
> > +}
> > +
> > /*
> > * PFE firmware initialization.
> > * Loads different firmware files from FIT image.
> > @@ -183,6 +222,10 @@ int pfe_firmware_init(void)
> > int ret = 0;
> > int fw_count;
> >
> > + ret = pfe_spi_flash_init();
> > + if (ret)
> > + goto err;
> > +
> > ret = pfe_fit_check();
> > if (ret)
> > goto err;
> > diff --git a/include/configs/ls1012a_common.h
> > b/include/configs/ls1012a_common.h
> > index 2579e2f..cbc5bff 100644
> > --- a/include/configs/ls1012a_common.h
> > +++ b/include/configs/ls1012a_common.h
> > @@ -36,9 +36,12 @@
> > /* Size of malloc() pool */
> > #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 *
> 1024)
> >
> > +/* PFE */
> > +#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> > +#define CONFIG_SYS_QE_FMAN_FW_LENGTH 0x10000
> > +
> > /*SPI device */
> > #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_TFABOOT)
> > -#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> > #define CONFIG_SPI_FLASH_SPANSION
> > #define CONFIG_FSL_SPI_INTERFACE
> > #define CONFIG_SF_DATAFLASH
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory
2020-01-10 10:58 ` [EXT] " Kuldeep Singh
@ 2020-01-13 8:37 ` Schrempf Frieder
2020-01-13 8:41 ` Kuldeep Singh
0 siblings, 1 reply; 5+ messages in thread
From: Schrempf Frieder @ 2020-01-13 8:37 UTC (permalink / raw)
To: u-boot
On 10.01.20 11:58, Kuldeep Singh wrote:
> Hi Frieder,
>
>> -----Original Message-----
>> From: Schrempf Frieder <frieder.schrempf@kontron.de>
>> Sent: Wednesday, January 8, 2020 2:52 PM
>> To: Kuldeep Singh <kuldeep.singh@nxp.com>; u-boot at lists.denx.de
>> Cc: Joe Hershberger <joe.hershberger@ni.com>; Thomas Hebb
>> <tommyhebb@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>
>> Subject: [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access
>> flash memory
>>
>> Caution: EXT Email
>>
>> On 08.01.20 10:08, Kuldeep Singh wrote:
>>> Current PFE firmware access spi-nor memory directly. New spi-mem
>>> framework does not support direct memory access. So, let's use
>>> spi_flash_read API to access memory instead of directly using it.
>>>
>>> Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
>>> ---
>>> v3:
>>> -Replace ret with 0 if flash probe fails
>>> v2:
>>> -Add return error code
>>> -Some changes in displayed log
>>>
>>> drivers/net/pfe_eth/pfe_firmware.c | 45
>> +++++++++++++++++++++++++++++++++++++-
>>> include/configs/ls1012a_common.h | 5 ++++-
>>> 2 files changed, 48 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/pfe_eth/pfe_firmware.c
>>> b/drivers/net/pfe_eth/pfe_firmware.c
>>> index e4563f1..6145f4e 100644
>>> --- a/drivers/net/pfe_eth/pfe_firmware.c
>>> +++ b/drivers/net/pfe_eth/pfe_firmware.c
>>> @@ -12,13 +12,14 @@
>>>
>>> #include <net/pfe_eth/pfe_eth.h>
>>> #include <net/pfe_eth/pfe_firmware.h>
>>> +#include <spi_flash.h>
>>> #ifdef CONFIG_CHAIN_OF_TRUST
>>> #include <fsl_validate.h>
>>> #endif
>>>
>>> #define PFE_FIRMWARE_FIT_CNF_NAME "config at 1"
>>>
>>> -static const void *pfe_fit_addr = (void *)CONFIG_SYS_LS_PFE_FW_ADDR;
>>> +static const void *pfe_fit_addr;
>>>
>>> /*
>>> * PFE elf firmware loader.
>>> @@ -159,6 +160,44 @@ static int pfe_fit_check(void)
>>> return ret;
>>> }
>>>
>>> +int pfe_spi_flash_init(void)
>>> +{
>>> + struct spi_flash *pfe_flash;
>>> + int ret = 0;
>>> + void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
>>> +
>>> +#ifdef CONFIG_DM_SPI_FLASH
>>> + struct udevice *new;
>>> +
>>> + /* 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);
>>> +
>>> + pfe_flash = dev_get_uclass_priv(new); #else
>>> + pfe_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
>>> + CONFIG_ENV_SPI_CS,
>>> + CONFIG_ENV_SPI_MAX_HZ,
>>> + CONFIG_ENV_SPI_MODE); #endif
>>> + if (!pfe_flash) {
>>> + printf("SF: probe for pfe failed\n");
>>> + return 0;
>>
>> No again. It seems like you're not getting what I'm trying to tell you, so it
>> would be better to just ask back instead of posting new versions.
>>
>> pfe_spi_flash_init() should return an error code in case the probe of the SPI
>> flash failed. An error code is a negative value like for example -ENODEV to
>> report that the device is not available to the calling function.
>
> Thanks Frieder for providing the info.
> I will return -ENODEV here instead of 0.
>
>>
>>> + }
>>> +
>>> + ret = spi_flash_read(pfe_flash,
>>> + CONFIG_SYS_LS_PFE_FW_ADDR,
>>> + CONFIG_SYS_QE_FMAN_FW_LENGTH,
>>> + addr);
>>> + if (ret)
>>> + printf("SF: read for pfe failed\n");
>
> I think -EIO should also be returned here as pfe functionality will fail if data is not read.
> Please confirm the same if error code is correct/required. I will update this in next version.
No, I don't think it's a good idea to return an error code here. You
already get an error code back from spi_flash_read() if it fails and you
pass this error to the caller at the end of the function, which should
be sufficient.
Also returning here would require to add a call to spi_flash_free() first.
>
> Thanks
> Kuldeep
>
>>> +
>>> + pfe_fit_addr = addr;
>>> + spi_flash_free(pfe_flash);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /*
>>> * PFE firmware initialization.
>>> * Loads different firmware files from FIT image.
>>> @@ -183,6 +222,10 @@ int pfe_firmware_init(void)
>>> int ret = 0;
>>> int fw_count;
>>>
>>> + ret = pfe_spi_flash_init();
>>> + if (ret)
>>> + goto err;
>>> +
>>> ret = pfe_fit_check();
>>> if (ret)
>>> goto err;
>>> diff --git a/include/configs/ls1012a_common.h
>>> b/include/configs/ls1012a_common.h
>>> index 2579e2f..cbc5bff 100644
>>> --- a/include/configs/ls1012a_common.h
>>> +++ b/include/configs/ls1012a_common.h
>>> @@ -36,9 +36,12 @@
>>> /* Size of malloc() pool */
>>> #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 *
>> 1024)
>>>
>>> +/* PFE */
>>> +#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
>>> +#define CONFIG_SYS_QE_FMAN_FW_LENGTH 0x10000
>>> +
>>> /*SPI device */
>>> #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_TFABOOT)
>>> -#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
>>> #define CONFIG_SPI_FLASH_SPANSION
>>> #define CONFIG_FSL_SPI_INTERFACE
>>> #define CONFIG_SF_DATAFLASH
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory
2020-01-13 8:37 ` Schrempf Frieder
@ 2020-01-13 8:41 ` Kuldeep Singh
0 siblings, 0 replies; 5+ messages in thread
From: Kuldeep Singh @ 2020-01-13 8:41 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Schrempf Frieder <frieder.schrempf@kontron.de>
> Sent: Monday, January 13, 2020 2:07 PM
> To: Kuldeep Singh <kuldeep.singh@nxp.com>; u-boot at lists.denx.de
> Cc: Joe Hershberger <joe.hershberger@ni.com>; Thomas Hebb
> <tommyhebb@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>
> Subject: Re: [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to access
> flash memory
>
> Caution: EXT Email
>
> On 10.01.20 11:58, Kuldeep Singh wrote:
> > Hi Frieder,
> >
> >> -----Original Message-----
> >> From: Schrempf Frieder <frieder.schrempf@kontron.de>
> >> Sent: Wednesday, January 8, 2020 2:52 PM
> >> To: Kuldeep Singh <kuldeep.singh@nxp.com>; u-boot at lists.denx.de
> >> Cc: Joe Hershberger <joe.hershberger@ni.com>; Thomas Hebb
> >> <tommyhebb@gmail.com>; Patrick Delaunay <patrick.delaunay@st.com>
> >> Subject: [EXT] Re: [Patch v3] net: pfe_eth: Use spi_flash_read API to
> >> access flash memory
> >>
> >> Caution: EXT Email
> >>
> >> On 08.01.20 10:08, Kuldeep Singh wrote:
> >>> Current PFE firmware access spi-nor memory directly. New spi-mem
> >>> framework does not support direct memory access. So, let's use
> >>> spi_flash_read API to access memory instead of directly using it.
> >>>
> >>> Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
> >>> ---
> >>> v3:
> >>> -Replace ret with 0 if flash probe fails
> >>> v2:
> >>> -Add return error code
> >>> -Some changes in displayed log
> >>>
> >>> drivers/net/pfe_eth/pfe_firmware.c | 45
> >> +++++++++++++++++++++++++++++++++++++-
> >>> include/configs/ls1012a_common.h | 5 ++++-
> >>> 2 files changed, 48 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/pfe_eth/pfe_firmware.c
> >>> b/drivers/net/pfe_eth/pfe_firmware.c
> >>> index e4563f1..6145f4e 100644
> >>> --- a/drivers/net/pfe_eth/pfe_firmware.c
> >>> +++ b/drivers/net/pfe_eth/pfe_firmware.c
> >>> @@ -12,13 +12,14 @@
> >>>
> >>> #include <net/pfe_eth/pfe_eth.h>
> >>> #include <net/pfe_eth/pfe_firmware.h>
> >>> +#include <spi_flash.h>
> >>> #ifdef CONFIG_CHAIN_OF_TRUST
> >>> #include <fsl_validate.h>
> >>> #endif
> >>>
> >>> #define PFE_FIRMWARE_FIT_CNF_NAME "config at 1"
> >>>
> >>> -static const void *pfe_fit_addr = (void
> >>> *)CONFIG_SYS_LS_PFE_FW_ADDR;
> >>> +static const void *pfe_fit_addr;
> >>>
> >>> /*
> >>> * PFE elf firmware loader.
> >>> @@ -159,6 +160,44 @@ static int pfe_fit_check(void)
> >>> return ret;
> >>> }
> >>>
> >>> +int pfe_spi_flash_init(void)
> >>> +{
> >>> + struct spi_flash *pfe_flash;
> >>> + int ret = 0;
> >>> + void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
> >>> +
> >>> +#ifdef CONFIG_DM_SPI_FLASH
> >>> + struct udevice *new;
> >>> +
> >>> + /* 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);
> >>> +
> >>> + pfe_flash = dev_get_uclass_priv(new); #else
> >>> + pfe_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
> >>> + CONFIG_ENV_SPI_CS,
> >>> + CONFIG_ENV_SPI_MAX_HZ,
> >>> + CONFIG_ENV_SPI_MODE); #endif
> >>> + if (!pfe_flash) {
> >>> + printf("SF: probe for pfe failed\n");
> >>> + return 0;
> >>
> >> No again. It seems like you're not getting what I'm trying to tell
> >> you, so it would be better to just ask back instead of posting new versions.
> >>
> >> pfe_spi_flash_init() should return an error code in case the probe of
> >> the SPI flash failed. An error code is a negative value like for
> >> example -ENODEV to report that the device is not available to the calling
> function.
> >
> > Thanks Frieder for providing the info.
> > I will return -ENODEV here instead of 0.
> >
> >>
> >>> + }
> >>> +
> >>> + ret = spi_flash_read(pfe_flash,
> >>> + CONFIG_SYS_LS_PFE_FW_ADDR,
> >>> + CONFIG_SYS_QE_FMAN_FW_LENGTH,
> >>> + addr);
> >>> + if (ret)
> >>> + printf("SF: read for pfe failed\n");
> >
> > I think -EIO should also be returned here as pfe functionality will fail if data
> is not read.
> > Please confirm the same if error code is correct/required. I will update this
> in next version.
>
> No, I don't think it's a good idea to return an error code here. You already get
> an error code back from spi_flash_read() if it fails and you pass this error to
> the caller at the end of the function, which should be sufficient.
Okay. I will not return error code here.
Thanks
Kuldeep
>
> Also returning here would require to add a call to spi_flash_free() first.
>
> >
> > Thanks
> > Kuldeep
> >
> >>> +
> >>> + pfe_fit_addr = addr;
> >>> + spi_flash_free(pfe_flash);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> /*
> >>> * PFE firmware initialization.
> >>> * Loads different firmware files from FIT image.
> >>> @@ -183,6 +222,10 @@ int pfe_firmware_init(void)
> >>> int ret = 0;
> >>> int fw_count;
> >>>
> >>> + ret = pfe_spi_flash_init();
> >>> + if (ret)
> >>> + goto err;
> >>> +
> >>> ret = pfe_fit_check();
> >>> if (ret)
> >>> goto err;
> >>> diff --git a/include/configs/ls1012a_common.h
> >>> b/include/configs/ls1012a_common.h
> >>> index 2579e2f..cbc5bff 100644
> >>> --- a/include/configs/ls1012a_common.h
> >>> +++ b/include/configs/ls1012a_common.h
> >>> @@ -36,9 +36,12 @@
> >>> /* Size of malloc() pool */
> >>> #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 *
> >> 1024)
> >>>
> >>> +/* PFE */
> >>> +#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> >>> +#define CONFIG_SYS_QE_FMAN_FW_LENGTH 0x10000
> >>> +
> >>> /*SPI device */
> >>> #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_TFABOOT)
> >>> -#define CONFIG_SYS_FMAN_FW_ADDR 0x400d0000
> >>> #define CONFIG_SPI_FLASH_SPANSION
> >>> #define CONFIG_FSL_SPI_INTERFACE
> >>> #define CONFIG_SF_DATAFLASH
> >>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-13 8:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 9:08 [Patch v3] net: pfe_eth: Use spi_flash_read API to access flash memory Kuldeep Singh
2020-01-08 9:21 ` Schrempf Frieder
2020-01-10 10:58 ` [EXT] " Kuldeep Singh
2020-01-13 8:37 ` Schrempf Frieder
2020-01-13 8:41 ` Kuldeep Singh
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.