Hi Simon,
From: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Sent: mardi 27 novembre 2018 09:11
On Tue, Nov 27, 2018 at 9:06 AM Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Simon,
-----Original Message----- From: Simon Goldschmidt simon.k.r.goldschmidt@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@st.com Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
I was commenting about code duplication, which is better now, so:
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@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).
2/ sent a v3 of this patchset (it can make more time to solve the potential issue)
Regards, Simon
Reagrds Patrick