[U-Boot,1/2] spl_spi: Read default speed and mode values from DT

Message ID 1541696240-11680-2-git-send-email-patrick.delaunay@st.com
State Changes Requested
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series
  • Read default speed and mode values from DT
Related show

Commit Message

Patrick Delaunay Nov. 8, 2018, 4:57 p.m.
In case of DT boot, don't read default speed and mode for SPI from
CONFIG_*, instead read from DT node.

Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 common/spl/spl_spi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Simon Goldschmidt Nov. 9, 2018, 6:44 a.m. | #1
On Thu, Nov 8, 2018 at 5:58 PM Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> In case of DT boot, don't read default speed and mode for SPI from
> CONFIG_*, instead read from DT node.
>
> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  common/spl/spl_spi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
> index 8cd4830..3cefc9a 100644
> --- a/common/spl/spl_spi.c
> +++ b/common/spl/spl_spi.c
> @@ -78,11 +78,18 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
>         /*
>          * Load U-Boot image from SPI flash into RAM
>          */
> -
> +#ifdef CONFIG_DM_SPI_FLASH
> +       /* In DM mode defaults will be taken from DT */
> +       flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> +                               CONFIG_SF_DEFAULT_CS,
> +                               0,
> +                               0);

Code duplication is never good. Wouldn't it be nicer to only have an
#if for the two differing parameters (e.g. via local variables)
instead of duplicating the function call?

Simon

> +#else
>         flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
>                                 CONFIG_SF_DEFAULT_CS,
>                                 CONFIG_SF_DEFAULT_SPEED,
>                                 CONFIG_SF_DEFAULT_MODE);
> +#endif
>         if (!flash) {
>                 puts("SPI probe failed.\n");
>                 return -ENODEV;
> --
> 2.7.4
>
Patrick Delaunay Nov. 12, 2018, 12:32 p.m. | #2
Hi Simon,

> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Sent: vendredi 9 novembre 2018 07:45
> Subject: Re: [PATCH 1/2] spl_spi: Read default speed and mode values from DT
> Importance: High
> 
> On Thu, Nov 8, 2018 at 5:58 PM Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > In case of DT boot, don't read default speed and mode for SPI from
> > CONFIG_*, instead read from DT node.
> >
> > Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  common/spl/spl_spi.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index
> > 8cd4830..3cefc9a 100644
> > --- a/common/spl/spl_spi.c
> > +++ b/common/spl/spl_spi.c
> > @@ -78,11 +78,18 @@ static int spl_spi_load_image(struct spl_image_info
> *spl_image,
> >         /*
> >          * Load U-Boot image from SPI flash into RAM
> >          */
> > -
> > +#ifdef CONFIG_DM_SPI_FLASH
> > +       /* In DM mode defaults will be taken from DT */
> > +       flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> > +                               CONFIG_SF_DEFAULT_CS,
> > +                               0,
> > +                               0);
> 
> Code duplication is never good. Wouldn't it be nicer to only have an #if for the
> two differing parameters (e.g. via local variables) instead of duplicating the
> function call?

Ok. I take the point and I will update the patchset with local variable in v2.
I just wait few day to be sure it is the only remark.

> Simon
> 
> > +#else
> >         flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> >                                 CONFIG_SF_DEFAULT_CS,
> >                                 CONFIG_SF_DEFAULT_SPEED,
> >                                 CONFIG_SF_DEFAULT_MODE);
> > +#endif
> >         if (!flash) {
> >                 puts("SPI probe failed.\n");
> >                 return -ENODEV;
> > --
> > 2.7.4
> >

Regards
Patrick

Patch

diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
index 8cd4830..3cefc9a 100644
--- a/common/spl/spl_spi.c
+++ b/common/spl/spl_spi.c
@@ -78,11 +78,18 @@  static int spl_spi_load_image(struct spl_image_info *spl_image,
 	/*
 	 * Load U-Boot image from SPI flash into RAM
 	 */
-
+#ifdef CONFIG_DM_SPI_FLASH
+	/* In DM mode defaults will be taken from DT */
+	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
+				CONFIG_SF_DEFAULT_CS,
+				0,
+				0);
+#else
 	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
 				CONFIG_SF_DEFAULT_CS,
 				CONFIG_SF_DEFAULT_SPEED,
 				CONFIG_SF_DEFAULT_MODE);
+#endif
 	if (!flash) {
 		puts("SPI probe failed.\n");
 		return -ENODEV;