| 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 | expand |
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 >
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
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;