[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Vignesh R]

In case of DT boot, don't read default speed and mode for SPI from
CONFIG_*, instead read from DT node. This will make sure that boards
with multiple SPI/QSPI controllers can be probed at different
bus frequencies and SPI modes.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v3: Update commit message to mention SPI mode changes

v2: Initialize speed, mode to 0 instead of -1

 cmd/sf.c                 | 2 ++
 drivers/spi/spi-uclass.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index 42862d9d921a..286906c3a151 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 #ifdef CONFIG_DM_SPI_FLASH
 	struct udevice *new, *bus_dev;
 	int ret;
+	/* In DM mode defaults will be taken from DT */
+	speed = 0, mode = 0;
 #else
 	struct spi_flash *new;
 #endif
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index 5561f36762f9..5fb5630e2981 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 		       struct udevice **busp, struct spi_slave **devp)
 {
 	struct udevice *bus, *dev;
+	struct dm_spi_slave_platdata *plat;
 	bool created = false;
 	int ret;
 
@@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 	 * SPI flash chip - we will bind to the correct driver.
 	 */
 	if (ret == -ENODEV && drv_name) {
-		struct dm_spi_slave_platdata *plat;
-
 		debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n",
 		      __func__, dev_name, busnum, cs, drv_name);
 		ret = device_bind_driver(bus, drv_name, dev_name, &dev);
@@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 		slave->dev = dev;
 	}
 
+	plat = dev_get_parent_platdata(dev);
+	if (!speed) {
+		speed = plat->max_hz;
+		mode = plat->mode;
+	}
 	ret = spi_set_speed_mode(bus, speed, mode);
 	if (ret)
 		goto err;
-- 
2.8.1

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Simon Glass]

On 13 April 2016 at 04:10, Vignesh R <vigneshr@ti.com> wrote:
> In case of DT boot, don't read default speed and mode for SPI from
> CONFIG_*, instead read from DT node. This will make sure that boards
> with multiple SPI/QSPI controllers can be probed at different
> bus frequencies and SPI modes.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>
> v3: Update commit message to mention SPI mode changes
>
> v2: Initialize speed, mode to 0 instead of -1
>
>  cmd/sf.c                 | 2 ++
>  drivers/spi/spi-uclass.c | 8 ++++++--
>  2 files changed, 8 insertions(+), 2 deletions(-)


Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Mugunthan V N]

On Wednesday 13 April 2016 03:40 PM, Vignesh R wrote:
> In case of DT boot, don't read default speed and mode for SPI from
> CONFIG_*, instead read from DT node. This will make sure that boards
> with multiple SPI/QSPI controllers can be probed at different
> bus frequencies and SPI modes.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Jagan Teki]

On 13 April 2016 at 15:40, Vignesh R <vigneshr@ti.com> wrote:
> In case of DT boot, don't read default speed and mode for SPI from
> CONFIG_*, instead read from DT node. This will make sure that boards
> with multiple SPI/QSPI controllers can be probed at different
> bus frequencies and SPI modes.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>
> v3: Update commit message to mention SPI mode changes
>
> v2: Initialize speed, mode to 0 instead of -1
>
>  cmd/sf.c                 | 2 ++

I think env_sf.c also need to update, pls- do change that as well.

-- 
Jagan.

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Vignesh R]

On 05/09/2016 08:15 PM, Jagan Teki wrote:
> On 13 April 2016 at 15:40, Vignesh R <vigneshr@ti.com> wrote:
>> In case of DT boot, don't read default speed and mode for SPI from
>> CONFIG_*, instead read from DT node. This will make sure that boards
>> with multiple SPI/QSPI controllers can be probed at different
>> bus frequencies and SPI modes.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>
>> v3: Update commit message to mention SPI mode changes
>>
>> v2: Initialize speed, mode to 0 instead of -1
>>
>>  cmd/sf.c                 | 2 ++
> 
> I think env_sf.c also need to update, pls- do change that as well.

Ok, will post a v4

-- 
Regards
Vignesh

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Patrick DELAUNAY]

Hi Jagan,


> From: Patrick DELAUNAY
> Sent: mardi 19 février 2019 13:28
> Subject: RE: [PATCH v3] dm: spi: Read default speed and mode values from DT
> 
> Hi Jagan,
> 
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: jeudi 14 février 2019 18:05
> >
> > On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY
> > <patrick.delaunay@st.com>
> > wrote:
> > >
> > > Hi Jagan
> > >
> > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > Sent: samedi 9 février 2019 17:22
> > > > Subject: Re: [PATCH v3] dm: spi: Read default speed and mode
> > > > values from DT
> > > >
> > > > On Mon, Jan 28, 2019 at 2:37 PM Patrick Delaunay
> > > > <patrick.delaunay@st.com>
> > > > wrote:
> > > > >
> > > > > This patch update the behavior introduced by commit 96907c0fe50a ("dm:
> > > > > spi: Read default speed and mode values from DT")
> > > > >
> > > > > In case of DT boot, don't read default speed and mode for SPI
> > > > > from
> > > > > CONFIG_* but instead read from DT node. This will make sure that
> > > > > boards with multiple SPI/QSPI controllers can be probed at
> > > > > different bus frequencies and SPI modes.
> > > > >
> > > > > DT values will be always used when available (full DM support of
> > > > > SPI slave with available DT node) even if speed and mode are
> > > > > requested; for example in splash screen support (in
> > > > > splash_sf_read_raw) or in SPL boot (in spl_spi_load_image).
> > > > > The caller of spi_get_bus_and_cs() no more need to force speed=0.
> > > > >
> > > > > But the current behavior don't change if the SPI slave is not
> > > > > present (device with generic driver is created automatically) or
> > > > > if platdata is used (CONFIG_OF_PLATDATA).
> > > > >
> > > > >
> > > > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > > > > ---
> > > > >
> > > > > Changes in v3:
> > > > >     - complete rework of the patch-set to avoid regression
> > > > >
> > > > > Changes in v2:
> > > > >     - use variables to avoid duplicated code
> > > > >
> > > > >  README                   | 3 +++
> > > > >  cmd/sf.c                 | 3 +--
> > > > >  common/spl/spl_spi.c     | 2 ++
> > > > >  drivers/spi/spi-uclass.c | 4 +++-
> > > > >  include/spi.h            | 9 +++++----
> > > > >  5 files changed, 14 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/README b/README
> > > > > index 17d56b8..f7fe74f 100644
> > > > > --- a/README
> > > > > +++ b/README
> > > > > @@ -2184,6 +2184,9 @@ The following options need to be configured:
> > > > >                 CONFIG_SF_DEFAULT_MODE          (see include/spi.h)
> > > > >                 CONFIG_SF_DEFAULT_SPEED         in Hz
> > > > >
> > > > > +               In case of DT boot, SPI don't read default speed and mode
> > > > > +               from CONFIG_*, but from platdata values computed
> > > > > + from
> > available
> > > > > +               DT node
> > > >
> > > > This has to update in Kconfig help info.
> > >
> > > Ok but witch Kconfig ? whitch config ?
> > >
> > > drivers/mtd/spi/Kconfig
> > > config DM_SPI_FLASH
> > >
> > > PS: In master branch, these define are not in yet managed in
> > > Kconfig, but they
> > are still managed by defines:
> > >        scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE
> > >       And so documentation is done in README not in Kconfig
> > >
> > > some migration in Kconfig is pending (moveconfig) ?
> >
> > Yes, moving them and make changes on top would really nice to go. thanks!
> 
> In fact it was a question...
> But I have my answer, no migration are pending on your side.
> 
> So I try yesterday and this morning to start migration in Kconfig but it is more
> difficult than expected initially (make defconfig freeze my PC for some board after
> my modificaitons).
> 
> So I don't expect to make the change on the top of the moving serie at a short
> term, but I will continue to work on that.
> 
> It is possible to inverse the proposed order...
> this patch first and after the Kconfig migration ?

Finally I found and solve the issue, So I push the 2 series :

SPI migration in Kconfig = http://patchwork.ozlabs.org/project/uboot/list/?series=94485
V4 = http://patchwork.ozlabs.org/project/uboot/list/?series=94490

Regards

Patrick

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Patrick DELAUNAY]

Hi Jagan,

> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: jeudi 14 février 2019 18:05
> 
> On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY <patrick.delaunay@st.com>
> wrote:
> >
> > Hi Jagan
> >
> > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > Sent: samedi 9 février 2019 17:22
> > > Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values
> > > from DT
> > >
> > > On Mon, Jan 28, 2019 at 2:37 PM Patrick Delaunay
> > > <patrick.delaunay@st.com>
> > > wrote:
> > > >
> > > > This patch update the behavior introduced by commit 96907c0fe50a ("dm:
> > > > spi: Read default speed and mode values from DT")
> > > >
> > > > In case of DT boot, don't read default speed and mode for SPI from
> > > > CONFIG_* but instead read from DT node. This will make sure that
> > > > boards with multiple SPI/QSPI controllers can be probed at
> > > > different bus frequencies and SPI modes.
> > > >
> > > > DT values will be always used when available (full DM support of
> > > > SPI slave with available DT node) even if speed and mode are
> > > > requested; for example in splash screen support (in
> > > > splash_sf_read_raw) or in SPL boot (in spl_spi_load_image).
> > > > The caller of spi_get_bus_and_cs() no more need to force speed=0.
> > > >
> > > > But the current behavior don't change if the SPI slave is not
> > > > present (device with generic driver is created automatically) or
> > > > if platdata is used (CONFIG_OF_PLATDATA).
> > > >
> > > >
> > > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > > > ---
> > > >
> > > > Changes in v3:
> > > >     - complete rework of the patch-set to avoid regression
> > > >
> > > > Changes in v2:
> > > >     - use variables to avoid duplicated code
> > > >
> > > >  README                   | 3 +++
> > > >  cmd/sf.c                 | 3 +--
> > > >  common/spl/spl_spi.c     | 2 ++
> > > >  drivers/spi/spi-uclass.c | 4 +++-
> > > >  include/spi.h            | 9 +++++----
> > > >  5 files changed, 14 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/README b/README
> > > > index 17d56b8..f7fe74f 100644
> > > > --- a/README
> > > > +++ b/README
> > > > @@ -2184,6 +2184,9 @@ The following options need to be configured:
> > > >                 CONFIG_SF_DEFAULT_MODE          (see include/spi.h)
> > > >                 CONFIG_SF_DEFAULT_SPEED         in Hz
> > > >
> > > > +               In case of DT boot, SPI don't read default speed and mode
> > > > +               from CONFIG_*, but from platdata values computed from
> available
> > > > +               DT node
> > >
> > > This has to update in Kconfig help info.
> >
> > Ok but witch Kconfig ? whitch config ?
> >
> > drivers/mtd/spi/Kconfig
> > config DM_SPI_FLASH
> >
> > PS: In master branch, these define are not in yet managed in  Kconfig, but they
> are still managed by defines:
> >        scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE
> >       And so documentation is done in README not in Kconfig
> >
> > some migration in Kconfig is pending (moveconfig) ?
> 
> Yes, moving them and make changes on top would really nice to go. thanks!

In fact it was a question... 
But I have my answer, no migration are pending on your side.

So I try yesterday and this morning to start migration in Kconfig 
but it is more difficult than expected initially (make defconfig freeze my PC for some board after my modificaitons).

So I don't expect to make the change on the top of the moving serie at a short term, 
but I will continue to work on that.

It is possible to inverse the proposed order... 
this patch first and after the Kconfig migration ?

Regards

Patrick

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Jagan Teki]

On Tue, Feb 12, 2019 at 7:14 PM Patrick DELAUNAY
<patrick.delaunay@st.com> wrote:
>
> Hi Jagan
>
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: samedi 9 février 2019 17:22
> > Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
> >
> > On Mon, Jan 28, 2019 at 2:37 PM Patrick Delaunay <patrick.delaunay@st.com>
> > wrote:
> > >
> > > This patch update the behavior introduced by commit 96907c0fe50a ("dm:
> > > spi: Read default speed and mode values from DT")
> > >
> > > In case of DT boot, don't read default speed and mode for SPI from
> > > CONFIG_* but instead read from DT node. This will make sure that
> > > boards with multiple SPI/QSPI controllers can be probed at different
> > > bus frequencies and SPI modes.
> > >
> > > DT values will be always used when available (full DM support of SPI
> > > slave with available DT node) even if speed and mode are requested;
> > > for example in splash screen support (in splash_sf_read_raw) or in SPL
> > > boot (in spl_spi_load_image).
> > > The caller of spi_get_bus_and_cs() no more need to force speed=0.
> > >
> > > But the current behavior don't change if the SPI slave is not present
> > > (device with generic driver is created automatically) or if platdata
> > > is used (CONFIG_OF_PLATDATA).
> > >
> > >
> > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > > ---
> > >
> > > Changes in v3:
> > >     - complete rework of the patch-set to avoid regression
> > >
> > > Changes in v2:
> > >     - use variables to avoid duplicated code
> > >
> > >  README                   | 3 +++
> > >  cmd/sf.c                 | 3 +--
> > >  common/spl/spl_spi.c     | 2 ++
> > >  drivers/spi/spi-uclass.c | 4 +++-
> > >  include/spi.h            | 9 +++++----
> > >  5 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/README b/README
> > > index 17d56b8..f7fe74f 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -2184,6 +2184,9 @@ The following options need to be configured:
> > >                 CONFIG_SF_DEFAULT_MODE          (see include/spi.h)
> > >                 CONFIG_SF_DEFAULT_SPEED         in Hz
> > >
> > > +               In case of DT boot, SPI don't read default speed and mode
> > > +               from CONFIG_*, but from platdata values computed from available
> > > +               DT node
> >
> > This has to update in Kconfig help info.
>
> Ok but witch Kconfig ? whitch config ?
>
> drivers/mtd/spi/Kconfig
> config DM_SPI_FLASH
>
> PS: In master branch, these define are not in yet managed in  Kconfig, but they are still managed by defines:
>        scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE
>       And so documentation is done in README not in Kconfig
>
> some migration in Kconfig is pending (moveconfig) ?

Yes, moving them and make changes on top would really nice to go. thanks!

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Patrick DELAUNAY]

Hi Jagan

> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: samedi 9 février 2019 17:22
> Subject: Re: [PATCH v3] dm: spi: Read default speed and mode values from DT
> 
> On Mon, Jan 28, 2019 at 2:37 PM Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > This patch update the behavior introduced by commit 96907c0fe50a ("dm:
> > spi: Read default speed and mode values from DT")
> >
> > In case of DT boot, don't read default speed and mode for SPI from
> > CONFIG_* but instead read from DT node. This will make sure that
> > boards with multiple SPI/QSPI controllers can be probed at different
> > bus frequencies and SPI modes.
> >
> > DT values will be always used when available (full DM support of SPI
> > slave with available DT node) even if speed and mode are requested;
> > for example in splash screen support (in splash_sf_read_raw) or in SPL
> > boot (in spl_spi_load_image).
> > The caller of spi_get_bus_and_cs() no more need to force speed=0.
> >
> > But the current behavior don't change if the SPI slave is not present
> > (device with generic driver is created automatically) or if platdata
> > is used (CONFIG_OF_PLATDATA).
> >
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > Changes in v3:
> >     - complete rework of the patch-set to avoid regression
> >
> > Changes in v2:
> >     - use variables to avoid duplicated code
> >
> >  README                   | 3 +++
> >  cmd/sf.c                 | 3 +--
> >  common/spl/spl_spi.c     | 2 ++
> >  drivers/spi/spi-uclass.c | 4 +++-
> >  include/spi.h            | 9 +++++----
> >  5 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/README b/README
> > index 17d56b8..f7fe74f 100644
> > --- a/README
> > +++ b/README
> > @@ -2184,6 +2184,9 @@ The following options need to be configured:
> >                 CONFIG_SF_DEFAULT_MODE          (see include/spi.h)
> >                 CONFIG_SF_DEFAULT_SPEED         in Hz
> >
> > +               In case of DT boot, SPI don't read default speed and mode
> > +               from CONFIG_*, but from platdata values computed from available
> > +               DT node
> 
> This has to update in Kconfig help info.

Ok but witch Kconfig ? whitch config ?

drivers/mtd/spi/Kconfig
config DM_SPI_FLASH

PS: In master branch, these define are not in yet managed in  Kconfig, but they are still managed by defines:
       scripts/config_whitelist.txt:1713:CONFIG_SF_DEFAULT_MODE
      And so documentation is done in README not in Kconfig

some migration in Kconfig is pending (moveconfig) ?

Regards
Patrick

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Jagan Teki]

On Mon, Jan 28, 2019 at 2:37 PM Patrick Delaunay
<patrick.delaunay@st.com> wrote:
>
> This patch update the behavior introduced by
> commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
>
> In case of DT boot, don't read default speed and mode for SPI from
> CONFIG_* but instead read from DT node. This will make sure that boards
> with multiple SPI/QSPI controllers can be probed at different
> bus frequencies and SPI modes.
>
> DT values will be always used when available (full DM support of
> SPI slave with available DT node) even if speed and mode are requested;
> for example in splash screen support (in splash_sf_read_raw)
> or in SPL boot (in spl_spi_load_image).
> The caller of spi_get_bus_and_cs() no more need to force speed=0.
>
> But the current behavior don't change if the SPI slave is not
> present (device with generic driver is created automatically)
> or if platdata is used (CONFIG_OF_PLATDATA).
>
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v3:
>     - complete rework of the patch-set to avoid regression
>
> Changes in v2:
>     - use variables to avoid duplicated code
>
>  README                   | 3 +++
>  cmd/sf.c                 | 3 +--
>  common/spl/spl_spi.c     | 2 ++
>  drivers/spi/spi-uclass.c | 4 +++-
>  include/spi.h            | 9 +++++----
>  5 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/README b/README
> index 17d56b8..f7fe74f 100644
> --- a/README
> +++ b/README
> @@ -2184,6 +2184,9 @@ The following options need to be configured:
>                 CONFIG_SF_DEFAULT_MODE          (see include/spi.h)
>                 CONFIG_SF_DEFAULT_SPEED         in Hz
>
> +               In case of DT boot, SPI don't read default speed and mode
> +               from CONFIG_*, but from platdata values computed from available
> +               DT node

This has to update in Kconfig help info.

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Petr Vorel]

Hi Patrick,

> This patch update the behavior introduced by
> commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")
...

Reviewed-by: Petr Vorel <petr.vorel@gmail.com>

Nice idea.

Kind regards,
Petr

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Patrick Delaunay]

This patch update the behavior introduced by
commit 96907c0fe50a ("dm: spi: Read default speed and mode values from DT")

In case of DT boot, don't read default speed and mode for SPI from
CONFIG_* but instead read from DT node. This will make sure that boards
with multiple SPI/QSPI controllers can be probed at different
bus frequencies and SPI modes.

DT values will be always used when available (full DM support of
SPI slave with available DT node) even if speed and mode are requested;
for example in splash screen support (in splash_sf_read_raw)
or in SPL boot (in spl_spi_load_image).
The caller of spi_get_bus_and_cs() no more need to force speed=0.

But the current behavior don't change if the SPI slave is not
present (device with generic driver is created automatically)
or if platdata is used (CONFIG_OF_PLATDATA).


Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
    - complete rework of the patch-set to avoid regression

Changes in v2:
    - use variables to avoid duplicated code

 README                   | 3 +++
 cmd/sf.c                 | 3 +--
 common/spl/spl_spi.c     | 2 ++
 drivers/spi/spi-uclass.c | 4 +++-
 include/spi.h            | 9 +++++----
 5 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/README b/README
index 17d56b8..f7fe74f 100644
--- a/README
+++ b/README
@@ -2184,6 +2184,9 @@ The following options need to be configured:
 		CONFIG_SF_DEFAULT_MODE 		(see include/spi.h)
 		CONFIG_SF_DEFAULT_SPEED		in Hz
 
+		In case of DT boot, SPI don't read default speed and mode
+		from CONFIG_*, but from platdata values computed from available
+		DT node
 
 - TFTP Fixed UDP Port:
 		CONFIG_TFTP_PORT
diff --git a/cmd/sf.c b/cmd/sf.c
index 84bb057..f1811d2 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -81,14 +81,13 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 {
 	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
 	unsigned int cs = CONFIG_SF_DEFAULT_CS;
+	/* In DM mode, defaults speed and mode will be taken from DT */
 	unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
 	unsigned int mode = CONFIG_SF_DEFAULT_MODE;
 	char *endp;
 #ifdef CONFIG_DM_SPI_FLASH
 	struct udevice *new, *bus_dev;
 	int ret;
-	/* In DM mode defaults will be taken from DT */
-	speed = 0, mode = 0;
 #else
 	struct spi_flash *new;
 #endif
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
index 8cd4830..9b74473 100644
--- a/common/spl/spl_spi.c
+++ b/common/spl/spl_spi.c
@@ -77,6 +77,8 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 
 	/*
 	 * Load U-Boot image from SPI flash into RAM
+	 * In DM mode: defaults speed and mode will be
+	 * taken from DT when available
 	 */
 
 	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index 2bc289a..88cb2a1 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -328,7 +328,9 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 	}
 
 	plat = dev_get_parent_platdata(dev);
-	if (!speed) {
+
+	/* get speed and mode from platdata when available */
+	if (plat->max_hz) {
 		speed = plat->max_hz;
 		mode = plat->mode;
 	}
diff --git a/include/spi.h b/include/spi.h
index 92427e5..3785941 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -496,14 +496,15 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
  * device and slave device.
  *
  * If no such slave exists, and drv_name is not NULL, then a new slave device
- * is automatically bound on this chip select.
+ * is automatically bound on this chip select with requested speed and mode.
  *
- * Ths new slave device is probed ready for use with the given speed and mode.
+ * Ths new slave device is probed ready for use with the speed and mode
+ * from platdata when available or the requested values.
  *
  * @busnum:	SPI bus number
  * @cs:		Chip select to look for
- * @speed:	SPI speed to use for this slave
- * @mode:	SPI mode to use for this slave
+ * @speed:	SPI speed to use for this slave when not available in platdata
+ * @mode:	SPI mode to use for this slave when not available in platdata
  * @drv_name:	Name of driver to attach to this chip select
  * @dev_name:	Name of the new device thus created
  * @busp:	Returns bus device
-- 
2.7.4

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Qianyu Gong]

Hi Vignesh,

> -----Original Message-----
> From: Vignesh R [mailto:vigneshr at ti.com]
> Sent: Thursday, April 21, 2016 12:30 PM
> To: Qianyu Gong <qianyu.gong@nxp.com>; jteki at openedev.com;
> trini at konsulko.com
> Cc: u-boot at lists.denx.de; sr at denx.de; Mingkai Hu <mingkai.hu@nxp.com>
> Subject: Re: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode values
> from DT
> 
> Hi Qianyu,
> 
> [...]
> 
> >>>> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> >>> speed, int
> >>>
> >>>> mode,
> >>>
> >>>>                            slave->dev = dev;
> >>>
> >>>>             }
> >>>
> >>>>
> >>>
> >>>> +          plat = dev_get_parent_platdata(dev);
> >>>
> >>>> +          if (!speed) {
> >>>
> >>>> +                         speed = plat->max_hz;
> >>>
> >>>> +                         mode = plat->mode;
> >>>
> >>>> +          }
> >>>
> >>>>             ret = spi_set_speed_mode(bus, speed, mode);
> >>>
> >>>>             if (ret)
> >>>
> >>>>                            goto err;
> >>>
> >>>> --
> >>>
> >>>
> >>>
> >>> I just doubt if spi_set_speed_mode() has really made a difference to
> >>>
> >>> the actual transfer.
> >>>
> >>
> >> It does (see below)...
> >>
> >>> Seems that if the device is inactive, calling device_probe() would
> >>> also call
> >>>
> >>> spi_set_speed_mode() and do the data transfer. Even if it's active,
> >>> setting
> >>>
> >>> speed and mode for it again would not be necessary.
> >>
> >>
> >> Yes, spi_set_speed_mode() is called from
> >> spi_flash_probe_slave()->spi_claim_bus() as part of device_probe().
> >> spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed based on
> DT
> >> data to spi_set_speed_mode(). But that's not the issue.
> >>
> >> But in spi_get_bus_and_cs() (called from sf probe) there is a call to
> >> spi_set_speed_mode() after device_probe() for inactive devices. This call is to
> >
> > Yes. Actually I'm thinking that the second spi_set_speed_mode()(called from
> > spi_get_bus_and_cs()) could just be removed from the end of the function.
> >
> 
> If second call to spi_set_speed_mode() is removed then how would you
> override default speed/mode specified via DT with that of cmd line args
> passed to sf probe. (else we need to change the definition of sf probe
> to not accept speed/mode in case of DT)
> 

OK, I see. Thanks.

> >> _override_ the speed set via DT with those passed as cmd line args of sf probe.
> But,
> >> if no args are passed to sf probe, speed and mode default to
> >> CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in
> >> cmd/sf.c) instead of using DT inputs.
> >>
> >
> > Yes. But notice that the speed and mode will only be replaced by
> > CONFIG_SF_DEFAULT_SPEED/MODE at the end of the calling. Every time 'sf
> probe' is
> > called, the device will be removed(if active) and thus it'll always call
> device_probe()
> > to set the speed&mode from DT. Then the driver will use them in the actual
> transfer.
> 
> True, I see the first call and speed/mode is set to DT values
> accordingly. But, that's not the final spi_set_speed_mode() call to the
> SPI driver.
> 
> > After the transfer is finished, the speed and mode are replaced by
> > CONFIG_SF_DEFAULT_SPEED/MODE(or anything else) again but they wouldn't
> be used
> > for the transfer at all.
> >
> 
> Sorry... I don't understand. What do you mean by transfer? sf probe does
> not do any data transfer other than reading jedec id.
> The values set by the SPI driver during device_probe() will be
> overwritten by the last spi_set_speed_mode() call in
> spi_get_bus_and_cs() which happens to pass CONFIG_SF_DEFAULT_SPEED/MODE.
> 

Yes, I should say reading jedec id from flash.
   
> Here is the call sequence:
> 
> sf probe()
> ---> device_probe()
>   ---> spi_set_speed_mode(values from DT)
>     ---> driver's pi_set_speed_mode() /* set to DT values */

Here reading jedec id is finished before calling the second spi_set_speed_mode().

> ---> spi_set_speed_mode(overridden values)
>   ---> driver's spi_set_speed_mode() /* set to newer(not DT) values */
> 
> Now if sf read is called after sf probe. One would see
> CONFIG_SF_DEFAULT_SPEED/MODE values in driver settings and data
> transfers happen at DEFAULT_SPEED.
> 

OK. So my conclusion is that the second spi_set_speed_mode() does affect 'sf read/write/..'
(overridden values) but not affect 'sf probe'(always setting DT values).
I think that's kind of weird.. 

Regards,
Qianyu
> 
> > And there may be another related issue in this case that I have reported to Simon.
> > If you would like to test on your board, please remove the device_unbind() in
> > do_spi_flash_probe(). The current SPI driver model will discard users' spi slave in
> DT from
> > the second 'sf probe' and create a new one using
> CONFIG_SF_DEFAULT_SPEED/MODE.
> >
> 
> Yes, I am aware of the problem with second sf probe discarding values
> from DT. IIRC, Simon not just wanted to remove device_unbind() but also
> do some more refactoring of messages in uclass drivers. Maybe Simon is
> working on the fix.
> 
> --
> Regards
> Vignesh

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Vignesh R]

Hi Qianyu,

[...]

>>>> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int
>>> speed, int
>>>
>>>> mode,
>>>
>>>>                            slave->dev = dev;
>>>
>>>>             }
>>>
>>>>
>>>
>>>> +          plat = dev_get_parent_platdata(dev);
>>>
>>>> +          if (!speed) {
>>>
>>>> +                         speed = plat->max_hz;
>>>
>>>> +                         mode = plat->mode;
>>>
>>>> +          }
>>>
>>>>             ret = spi_set_speed_mode(bus, speed, mode);
>>>
>>>>             if (ret)
>>>
>>>>                            goto err;
>>>
>>>> --
>>>
>>>
>>>
>>> I just doubt if spi_set_speed_mode() has really made a difference to
>>>
>>> the actual transfer.
>>>
>>
>> It does (see below)...
>>
>>> Seems that if the device is inactive, calling device_probe() would
>>> also call
>>>
>>> spi_set_speed_mode() and do the data transfer. Even if it's active,
>>> setting
>>>
>>> speed and mode for it again would not be necessary.
>>
>>
>> Yes, spi_set_speed_mode() is called from
>> spi_flash_probe_slave()->spi_claim_bus() as part of device_probe().
>> spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed based on DT
>> data to spi_set_speed_mode(). But that's not the issue.
>>
>> But in spi_get_bus_and_cs() (called from sf probe) there is a call to
>> spi_set_speed_mode() after device_probe() for inactive devices. This call is to
> 
> Yes. Actually I'm thinking that the second spi_set_speed_mode()(called from
> spi_get_bus_and_cs()) could just be removed from the end of the function.
> 

If second call to spi_set_speed_mode() is removed then how would you
override default speed/mode specified via DT with that of cmd line args
passed to sf probe. (else we need to change the definition of sf probe
to not accept speed/mode in case of DT)

>> _override_ the speed set via DT with those passed as cmd line args of sf probe. But,
>> if no args are passed to sf probe, speed and mode default to
>> CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in
>> cmd/sf.c) instead of using DT inputs.
>>
> 
> Yes. But notice that the speed and mode will only be replaced by 
> CONFIG_SF_DEFAULT_SPEED/MODE at the end of the calling. Every time 'sf probe' is
> called, the device will be removed(if active) and thus it'll always call device_probe()
> to set the speed&mode from DT. Then the driver will use them in the actual transfer.

True, I see the first call and speed/mode is set to DT values
accordingly. But, that's not the final spi_set_speed_mode() call to the
SPI driver.

> After the transfer is finished, the speed and mode are replaced by
> CONFIG_SF_DEFAULT_SPEED/MODE(or anything else) again but they wouldn't be used
> for the transfer at all.
> 

Sorry... I don't understand. What do you mean by transfer? sf probe does
not do any data transfer other than reading jedec id.
The values set by the SPI driver during device_probe() will be
overwritten by the last spi_set_speed_mode() call in
spi_get_bus_and_cs() which happens to pass CONFIG_SF_DEFAULT_SPEED/MODE.

Here is the call sequence:

sf probe()
---> device_probe()
  ---> spi_set_speed_mode(values from DT)
    ---> driver's pi_set_speed_mode() /* set to DT values */
---> spi_set_speed_mode(overridden values)
  ---> driver's spi_set_speed_mode() /* set to newer(not DT) values */

Now if sf read is called after sf probe. One would see
CONFIG_SF_DEFAULT_SPEED/MODE values in driver settings and data
transfers happen at DEFAULT_SPEED.


> And there may be another related issue in this case that I have reported to Simon.
> If you would like to test on your board, please remove the device_unbind() in
> do_spi_flash_probe(). The current SPI driver model will discard users' spi slave in DT from
> the second 'sf probe' and create a new one using CONFIG_SF_DEFAULT_SPEED/MODE. 
>  

Yes, I am aware of the problem with second sf probe discarding values
from DT. IIRC, Simon not just wanted to remove device_unbind() but also
do some more refactoring of messages in uclass drivers. Maybe Simon is
working on the fix.

-- 
Regards
Vignesh

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Qianyu Gong]

Hi Vignesh,

> -----Original Message-----
> From: Vignesh R [mailto:vigneshr at ti.com]
> Sent: Wednesday, April 20, 2016 6:47 PM
> To: Qianyu Gong <qianyu.gong@nxp.com>; jteki at openedev.com;
> trini at konsulko.com
> Cc: u-boot at lists.denx.de; sr at denx.de; Mingkai Hu <mingkai.hu@nxp.com>
> Subject: Re: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode values
> from DT
> 
> On 04/20/2016 03:26 PM, Qianyu Gong wrote:
> > Hi Vignesh,
> >
> >> Date: Wed, 13 Apr 2016 15:40:53 +0530
> >
> >> From: Vignesh R <vigneshr at ti.com <mailto:vigneshr@ti.com>>
> >
> >> To: Jagan Teki <jteki at openedev.com <mailto:jteki@openedev.com>>, Tom
> > Rini <trini at konsulko.com <mailto:trini@konsulko.com>>
> >
> >> Cc: u-boot at lists.denx.de <mailto:u-boot@lists.denx.de>, Stefan Roese
> > <sr at denx.de <mailto:sr@denx.de>>
> >
> >> Subject: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode
> >
> >>             values    from DT
> >
> >> Message-ID: <1460542253-10580-1-git-send-email-vigneshr@ti.com
> > <mailto:1460542253-10580-1-git-send-email-vigneshr@ti.com>>
> >
> >> Content-Type: text/plain
> >
> >>
> >
> >> In case of DT boot, don't read default speed and mode for SPI from
> >
> >> CONFIG_*, instead read from DT node. This will make sure that boards
> >
> >> with multiple SPI/QSPI controllers can be probed at different
> >
> >> bus frequencies and SPI modes.
> >
> >>
> >
> >> Signed-off-by: Vignesh R <vigneshr at ti.com <mailto:vigneshr@ti.com>>
> >
> >> ---
> >
> >>
> >
> >> v3: Update commit message to mention SPI mode changes
> >
> >>
> >
> >> v2: Initialize speed, mode to 0 instead of -1
> >
> >>
> >
> >>  cmd/sf.c                 | 2 ++
> >
> >>  drivers/spi/spi-uclass.c | 8 ++++++--
> >
> >>  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> >>
> >
> >> diff --git a/cmd/sf.c b/cmd/sf.c
> >
> >> index 42862d9d921a..286906c3a151 100644
> >
> >> --- a/cmd/sf.c
> >
> >> +++ b/cmd/sf.c
> >
> >> @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char *
> >> const
> > argv[])
> >
> >>  #ifdef CONFIG_DM_SPI_FLASH
> >
> >>             struct udevice *new, *bus_dev;
> >
> >>             int ret;
> >
> >> +          /* In DM mode defaults will be taken from DT */
> >
> >> +          speed = 0, mode = 0;
> >
> >>  #else
> >
> >>             struct spi_flash *new;
> >
> >>  #endif
> >
> >> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> >
> >> index 5561f36762f9..5fb5630e2981 100644
> >
> >> --- a/drivers/spi/spi-uclass.c
> >
> >> +++ b/drivers/spi/spi-uclass.c
> >
> >> @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> > speed, int
> >
> >> mode,
> >
> >>                                   struct udevice **busp, struct
> > spi_slave **devp)
> >
> >>  {
> >
> >>             struct udevice *bus, *dev;
> >
> >> +          struct dm_spi_slave_platdata *plat;
> >
> >>             bool created = false;
> >
> >>             int ret;
> >
> >>
> >
> >> @@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> > speed, int
> >
> >> mode,
> >
> >>              * SPI flash chip - we will bind to the correct driver.
> >
> >>              */
> >
> >>             if (ret == -ENODEV && drv_name) {
> >
> >> -                         struct dm_spi_slave_platdata *plat;
> >
> >> -
> >
> >>                            debug("%s: Binding new device '%s',
> > busnum=%d, cs=%d,
> >
> >> driver=%s\n",
> >
> >>                                  __func__, dev_name, busnum, cs,
> > drv_name);
> >
> >>                            ret = device_bind_driver(bus, drv_name,
> > dev_name, &dev);
> >
> >> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> > speed, int
> >
> >> mode,
> >
> >>                            slave->dev = dev;
> >
> >>             }
> >
> >>
> >
> >> +          plat = dev_get_parent_platdata(dev);
> >
> >> +          if (!speed) {
> >
> >> +                         speed = plat->max_hz;
> >
> >> +                         mode = plat->mode;
> >
> >> +          }
> >
> >>             ret = spi_set_speed_mode(bus, speed, mode);
> >
> >>             if (ret)
> >
> >>                            goto err;
> >
> >> --
> >
> >
> >
> > I just doubt if spi_set_speed_mode() has really made a difference to
> >
> > the actual transfer.
> >
> 
> It does (see below)...
> 
> > Seems that if the device is inactive, calling device_probe() would
> > also call
> >
> > spi_set_speed_mode() and do the data transfer. Even if it's active,
> > setting
> >
> > speed and mode for it again would not be necessary.
> 
> 
> Yes, spi_set_speed_mode() is called from
> spi_flash_probe_slave()->spi_claim_bus() as part of device_probe().
> spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed based on DT
> data to spi_set_speed_mode(). But that's not the issue.
> 
> But in spi_get_bus_and_cs() (called from sf probe) there is a call to
> spi_set_speed_mode() after device_probe() for inactive devices. This call is to

Yes. Actually I'm thinking that the second spi_set_speed_mode()(called from
spi_get_bus_and_cs()) could just be removed from the end of the function.

> _override_ the speed set via DT with those passed as cmd line args of sf probe. But,
> if no args are passed to sf probe, speed and mode default to
> CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in
> cmd/sf.c) instead of using DT inputs.
> 

Yes. But notice that the speed and mode will only be replaced by 
CONFIG_SF_DEFAULT_SPEED/MODE at the end of the calling. Every time 'sf probe' is
called, the device will be removed(if active) and thus it'll always call device_probe()
to set the speed&mode from DT. Then the driver will use them in the actual transfer.
After the transfer is finished, the speed and mode are replaced by
CONFIG_SF_DEFAULT_SPEED/MODE(or anything else) again but they wouldn't be used
for the transfer at all.

And there may be another related issue in this case that I have reported to Simon.
If you would like to test on your board, please remove the device_unbind() in
do_spi_flash_probe(). The current SPI driver model will discard users' spi slave in DT from
the second 'sf probe' and create a new one using CONFIG_SF_DEFAULT_SPEED/MODE. 
 
Regards,
Qianyu

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Vignesh R]

On 04/20/2016 03:26 PM, Qianyu Gong wrote:
> Hi Vignesh,
> 
>  
> 
>> Date: Wed, 13 Apr 2016 15:40:53 +0530
> 
>> From: Vignesh R <vigneshr at ti.com <mailto:vigneshr@ti.com>>
> 
>> To: Jagan Teki <jteki at openedev.com <mailto:jteki@openedev.com>>, Tom
> Rini <trini at konsulko.com <mailto:trini@konsulko.com>>
> 
>> Cc: u-boot at lists.denx.de <mailto:u-boot@lists.denx.de>, Stefan Roese
> <sr at denx.de <mailto:sr@denx.de>>
> 
>> Subject: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode
> 
>>             values    from DT
> 
>> Message-ID: <1460542253-10580-1-git-send-email-vigneshr@ti.com
> <mailto:1460542253-10580-1-git-send-email-vigneshr@ti.com>>
> 
>> Content-Type: text/plain
> 
>>
> 
>> In case of DT boot, don't read default speed and mode for SPI from
> 
>> CONFIG_*, instead read from DT node. This will make sure that boards
> 
>> with multiple SPI/QSPI controllers can be probed at different
> 
>> bus frequencies and SPI modes.
> 
>>
> 
>> Signed-off-by: Vignesh R <vigneshr at ti.com <mailto:vigneshr@ti.com>>
> 
>> ---
> 
>>
> 
>> v3: Update commit message to mention SPI mode changes
> 
>>
> 
>> v2: Initialize speed, mode to 0 instead of -1
> 
>>
> 
>>  cmd/sf.c                 | 2 ++
> 
>>  drivers/spi/spi-uclass.c | 8 ++++++--
> 
>>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
>>
> 
>> diff --git a/cmd/sf.c b/cmd/sf.c
> 
>> index 42862d9d921a..286906c3a151 100644
> 
>> --- a/cmd/sf.c
> 
>> +++ b/cmd/sf.c
> 
>> @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const
> argv[])
> 
>>  #ifdef CONFIG_DM_SPI_FLASH
> 
>>             struct udevice *new, *bus_dev;
> 
>>             int ret;
> 
>> +          /* In DM mode defaults will be taken from DT */
> 
>> +          speed = 0, mode = 0;
> 
>>  #else
> 
>>             struct spi_flash *new;
> 
>>  #endif
> 
>> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> 
>> index 5561f36762f9..5fb5630e2981 100644
> 
>> --- a/drivers/spi/spi-uclass.c
> 
>> +++ b/drivers/spi/spi-uclass.c
> 
>> @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>                                   struct udevice **busp, struct
> spi_slave **devp)
> 
>>  {
> 
>>             struct udevice *bus, *dev;
> 
>> +          struct dm_spi_slave_platdata *plat;
> 
>>             bool created = false;
> 
>>             int ret;
> 
>>
> 
>> @@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>              * SPI flash chip - we will bind to the correct driver.
> 
>>              */
> 
>>             if (ret == -ENODEV && drv_name) {
> 
>> -                         struct dm_spi_slave_platdata *plat;
> 
>> -
> 
>>                            debug("%s: Binding new device '%s',
> busnum=%d, cs=%d,
> 
>> driver=%s\n",
> 
>>                                  __func__, dev_name, busnum, cs,
> drv_name);
> 
>>                            ret = device_bind_driver(bus, drv_name,
> dev_name, &dev);
> 
>> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>                            slave->dev = dev;
> 
>>             }
> 
>>
> 
>> +          plat = dev_get_parent_platdata(dev);
> 
>> +          if (!speed) {
> 
>> +                         speed = plat->max_hz;
> 
>> +                         mode = plat->mode;
> 
>> +          }
> 
>>             ret = spi_set_speed_mode(bus, speed, mode);
> 
>>             if (ret)
> 
>>                            goto err;
> 
>> --
> 
>  
> 
> I just doubt if spi_set_speed_mode() has really made a difference to
> 
> the actual transfer.
> 

It does (see below)...

> Seems that if the device is inactive, calling device_probe() would also
> call
> 
> spi_set_speed_mode() and do the data transfer. Even if it?s active, setting
> 
> speed and mode for it again would not be necessary.


Yes, spi_set_speed_mode() is called from
spi_flash_probe_slave()->spi_claim_bus() as part of device_probe().
spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed
based on DT data to spi_set_speed_mode(). But that's not the issue.

But in spi_get_bus_and_cs() (called from sf probe) there is a call to
spi_set_speed_mode() after device_probe() for inactive devices. This
call is to _override_ the speed set via DT with those passed as cmd line
args of sf probe. But, if no args are passed to sf probe, speed and mode
default to CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in
cmd/sf.c) instead of using DT inputs.

-- 
Regards
Vignesh

[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Qianyu Gong]

Hi Vignesh,



> Date: Wed, 13 Apr 2016 15:40:53 +0530

> From: Vignesh R <vigneshr at ti.com<mailto:vigneshr@ti.com>>

> To: Jagan Teki <jteki at openedev.com<mailto:jteki@openedev.com>>, Tom Rini <trini at konsulko.com<mailto:trini@konsulko.com>>

> Cc: u-boot at lists.denx.de<mailto:u-boot@lists.denx.de>, Stefan Roese <sr at denx.de<mailto:sr@denx.de>>

> Subject: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode

>             values    from DT

> Message-ID: <1460542253-10580-1-git-send-email-vigneshr at ti.com<mailto:1460542253-10580-1-git-send-email-vigneshr@ti.com>>

> Content-Type: text/plain

>

> In case of DT boot, don't read default speed and mode for SPI from

> CONFIG_*, instead read from DT node. This will make sure that boards

> with multiple SPI/QSPI controllers can be probed at different

> bus frequencies and SPI modes.

>

> Signed-off-by: Vignesh R <vigneshr at ti.com<mailto:vigneshr@ti.com>>

> ---

>

> v3: Update commit message to mention SPI mode changes

>

> v2: Initialize speed, mode to 0 instead of -1

>

>  cmd/sf.c                 | 2 ++

>  drivers/spi/spi-uclass.c | 8 ++++++--

>  2 files changed, 8 insertions(+), 2 deletions(-)

>

> diff --git a/cmd/sf.c b/cmd/sf.c

> index 42862d9d921a..286906c3a151 100644

> --- a/cmd/sf.c

> +++ b/cmd/sf.c

> @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const argv[])

>  #ifdef CONFIG_DM_SPI_FLASH

>             struct udevice *new, *bus_dev;

>             int ret;

> +          /* In DM mode defaults will be taken from DT */

> +          speed = 0, mode = 0;

>  #else

>             struct spi_flash *new;

>  #endif

> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c

> index 5561f36762f9..5fb5630e2981 100644

> --- a/drivers/spi/spi-uclass.c

> +++ b/drivers/spi/spi-uclass.c

> @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int

> mode,

>                                   struct udevice **busp, struct spi_slave **devp)

>  {

>             struct udevice *bus, *dev;

> +          struct dm_spi_slave_platdata *plat;

>             bool created = false;

>             int ret;

>

> @@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int

> mode,

>              * SPI flash chip - we will bind to the correct driver.

>              */

>             if (ret == -ENODEV && drv_name) {

> -                         struct dm_spi_slave_platdata *plat;

> -

>                            debug("%s: Binding new device '%s', busnum=%d, cs=%d,

> driver=%s\n",

>                                  __func__, dev_name, busnum, cs, drv_name);

>                            ret = device_bind_driver(bus, drv_name, dev_name, &dev);

> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int

> mode,

>                            slave->dev = dev;

>             }

>

> +          plat = dev_get_parent_platdata(dev);

> +          if (!speed) {

> +                         speed = plat->max_hz;

> +                         mode = plat->mode;

> +          }

>             ret = spi_set_speed_mode(bus, speed, mode);

>             if (ret)

>                            goto err;

> --

I just doubt if spi_set_speed_mode() has really made a difference to
the actual transfer.
Seems that if the device is inactive, calling device_probe() would also call
spi_set_speed_mode() and do the data transfer. Even if it's active, setting
speed and mode for it again would not be necessary.


Regards,
Qianyu