Hi Patrice,
On Wed, 12 Jan 2022 at 03:59, Patrice Chotard
<patrice.chot...@foss.st.com> wrote:
>
> Add spi_flash_probe_bus_cs() and spi_get_bus_and_cs() new "use_dt"
> param which allows to select SPI speed and mode from DT or from
> default value passed in parameters.
>
> Since commit e2e95e5e2542 ("spi: Update speed/mode on change")
> when calling "sf probe" or "env save" on SPI flash,
> spi_set_speed_mode() is called twice.
>
> spi_get_bus_and_cs()
> |--> spi_claim_bus()
> | |--> spi_set_speed_mode(speed and mode from DT)
> ...
> |--> spi_set_speed_mode(default speed and mode value)
>
> The first spi_set_speed_mode() call is done with speed and mode
> values from DT, whereas the second call is done with speed
> and mode set to default value (speed is set to CONFIG_SF_DEFAULT_SPEED)
>
> This is an issue because SPI flash performance are impacted by
> using default speed which can be lower than the one defined in DT.
>
> One solution is to set CONFIG_SF_DEFAULT_SPEED to the speed defined
> in DT, but we loose flexibility offered by DT.
>
> Another issue can be encountered with 2 SPI flashes using 2 different
> speeds. In this specific case usage of CONFIG_SF_DEFAULT_SPEED is not
> flexible compared to get the 2 different speeds from DT.
>
> By adding new parameter "use_dt" to spi_flash_probe_bus_cs() and to
> spi_get_bus_and_cs(), this allows to force usage of either speed and
> mode from DT (use_dt = true) or to use speed and mode parameter value.
>
> Signed-off-by: Patrice Chotard <patrice.chot...@foss.st.com>
> Cc: Marek Behun <marek.be...@nic.cz>
> Cc: Jagan Teki <ja...@amarulasolutions.com>
> Cc: Vignesh R <vigne...@ti.com>
> Cc: Joe Hershberger <joe.hershber...@ni.com>
> Cc: Ramon Fried <rfried....@gmail.com>
> Cc: Lukasz Majewski <lu...@denx.de>
> Cc: Marek Vasut <ma...@denx.de>
> Cc: Wolfgang Denk <w...@denx.de>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Stefan Roese <s...@denx.de>
> Cc: "Pali Rohár" <p...@kernel.org>
> Cc: Konstantin Porotchkin <kos...@marvell.com>
> Cc: Igal Liberman <ig...@marvell.com>
> Cc: Bin Meng <bmeng...@gmail.com>
> Cc: Pratyush Yadav <p.ya...@ti.com>
> Cc: Sean Anderson <sean...@gmail.com>
> Cc: Anji J <anji.jagarlm...@nxp.com>
> Cc: Biwen Li <biwen...@nxp.com>
> Cc: Priyanka Jain <priyanka.j...@nxp.com>
> Cc: Chaitanya Sakinam <chaitanya.saki...@nxp.com>
> ---
>
> board/CZ.NIC/turris_mox/turris_mox.c | 2 +-
> cmd/sf.c | 5 ++++-
> cmd/spi.c | 2 +-
> drivers/mtd/spi/sf-uclass.c | 8 ++++----
> drivers/net/fm/fm.c | 5 +++--
> drivers/net/pfe_eth/pfe_firmware.c | 2 +-
> drivers/net/sni_netsec.c | 3 ++-
> drivers/spi/spi-uclass.c | 8 ++++----
> drivers/usb/gadget/max3420_udc.c | 2 +-
> env/sf.c | 2 +-
> include/spi.h | 9 +++++----
> include/spi_flash.h | 2 +-
> test/dm/spi.c | 15 ++++++++-------
> 13 files changed, 36 insertions(+), 29 deletions(-)
I think this is a good idea, but should use a separate function name
instead of adding an argument, since it doesn't make sense to pass
parameters that are ignored.
Also we should probably have devicetree as the default (the base function name).
Regards,
Simon
