On Tue, Nov 27, 2018 at 1:33 PM Patrick DELAUNAY <patrick.delau...@st.com> wrote: > > Hi Simon, > > > From: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> > > Sent: mardi 27 novembre 2018 09:11 > > > > > On Tue, Nov 27, 2018 at 9:06 AM Patrick DELAUNAY > > <patrick.delau...@st.com> wrote: > > > > > > Hi Simon, > > > > > > > -----Original Message----- > > > > From: Simon Goldschmidt <simon.k.r.goldschm...@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.kere...@st.com> > > > > > Signed-off-by: Patrick Delaunay <patrick.delau...@st.com> > > > > > > > > I was commenting about code duplication, which is better now, so: > > > > > > > > Reviewed-by: Simon Goldschmidt <simon.k.r.goldschm...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot