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

[Simon Goldschmidt]

On Tue, Nov 27, 2018 at 1:33 PM Patrick DELAUNAY
<patrick.delaunay at st.com> wrote:
>
> Hi Simon,
>
> > From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > Sent: mardi 27 novembre 2018 09:11
>
> >
> > On Tue, Nov 27, 2018 at 9:06 AM Patrick DELAUNAY
> > <patrick.delaunay at st.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > > -----Original Message-----
> > > > From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > > Sent: lundi 19 novembre 2018 20:09
> > > >
> > > > On 19.11.2018 18:33, Patrick Delaunay 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 at st.com>
> > > > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> > > >
> > > > I was commenting about code duplication, which is better now, so:
> > > >
> > > > Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > >
> > > > I do think it would be nice to invert the logic. That way, we could
> > > > get completely rid of those two CONFIG_SF_DEFAULT settings for
> > > > DM_SPI_FLASH boards (and eventually for all boards - when's the deadline
> > for that?).
> > > > But there are other places that still do it like you do here, so
> > > > it's probably better to change them all at once...
> > >
> > > Agree with you.
> > >
> > > In fact I hesitate with directly change the header file
> > >
> > > #ifdef CONFIG_DM_SPI_FLASH
> > >         /* In DM mode defaults will be taken from DT */
> > >         #define CONFIG_SF_DEFAULT_SPEED 0
> > >         #define CONFIG_SF_DEFAULT_MODE 0 #endif
> >
> > This looks correct to me.
> >
> > > But I am afraid of the potential impacts (define is used in many
> > > boards), but I think it sould be more cleaner way to force the expected
> > behavior.
> >
> > Can't we just push this and fix whatever it breaks?
>
> Yes I can push it, what is the correct patch strategy.
>
> 1/ accept this patch (for short term solution) and propose a update after (I can do it next week).

I'm totally OK with 1, yes. That's why I've already sent my
reviewed-by. But I would really appreciate a later update, this got me
confused already... ;-)

Regards,
Simon

>
> 2/ sent a v3 of this patchset (it can make more time to solve the potential issue)
>
> >
> > Regards,
> > Simon
>
> Reagrds
> Patrick