All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 1/2] spl_spi: Read default speed and mode values from DT
Date: Tue, 27 Nov 2018 09:11:22 +0100	[thread overview]
Message-ID: <CAAh8qswHteqKOH8QEOBx-AD9999deyUzP9P6_smDmWsx0O8D4w@mail.gmail.com> (raw)
In-Reply-To: <28c7e7892f2a4b09b9ec5f3a4039716e@SFHDAG6NODE3.st.com>

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?

Regards,
Simon

  reply	other threads:[~2018-11-27  8:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 17:33 [U-Boot] [PATCH v2 0/2] Read default speed and mode values from DT Patrick Delaunay
2018-11-19 17:33 ` [U-Boot] [PATCH v2 1/2] spl_spi: " Patrick Delaunay
2018-11-19 19:08   ` Simon Goldschmidt
2018-11-27  8:06     ` Patrick DELAUNAY
2018-11-27  8:11       ` Simon Goldschmidt [this message]
2018-11-27 12:33         ` Patrick DELAUNAY
2018-11-27 12:54           ` Simon Goldschmidt
2018-12-10 11:01             ` Patrick DELAUNAY
2018-12-10 22:48   ` Adam Ford
2018-12-13 16:04     ` Patrick DELAUNAY
2018-11-19 17:33 ` [U-Boot] [PATCH v2 2/2] splash: sf: " Patrick Delaunay
2018-12-10 15:50 ` [U-Boot] [PATCH v2 0/2] " Adam Ford
2018-12-13 16:17   ` Patrick DELAUNAY

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAh8qswHteqKOH8QEOBx-AD9999deyUzP9P6_smDmWsx0O8D4w@mail.gmail.com \
    --to=simon.k.r.goldschmidt@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.