Re: [PATCH v2 2/2] dfu: sf: rely on DT for spi speed and mode

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Neil,

Thank you for the patch.

On mar., oct. 01, 2024 at 18:06, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> Align with cmd_sf, and try to rely on DT for spi speed and mode,
> and still fallback on spi_flash_probe() if it fails.
>
> With the current scheme, spi_flash_probe() will be called
> with CONFIG_SF_DEFAULT_SPEED and CONFIG_SF_DEFAULT_MODE
> with are set to 0 by default on DT platforms using DM_SPI_FLASH.
>
> Like cmd_sf, keep the option to specify the speed and mode
> from the dfu_alt_mode string, but rely on DT properties
> if not specified.
>
> Using CONFIG_SF_DEFAULT_SPEED and CONFIG_SF_DEFAULT_MODE
> makes the SPIFC controller on Amlogic Meson G12B & SM1
> hardware fail and is unable to recover until a system reboot.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  drivers/dfu/dfu_sf.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
> index 7c1c0f9e2dc..8f7296adec6 100644
> --- a/drivers/dfu/dfu_sf.c
> +++ b/drivers/dfu/dfu_sf.c
> @@ -123,6 +123,7 @@ static struct spi_flash *parse_dev(char *devstr)
>  	unsigned int mode = CONFIG_SF_DEFAULT_MODE;
>  	char *s, *endp;
>  	struct spi_flash *dev;
> +	bool use_dt = true;
>  
>  	s = strsep(&devstr, ":");
>  	if (!s || !*s || (bus = simple_strtoul(s, &endp, 0), *endp)) {
> @@ -143,6 +144,8 @@ static struct spi_flash *parse_dev(char *devstr)
>  			printf("Invalid SPI speed %s\n", s);
>  			return NULL;
>  		}
> +		if (IS_ENABLED(CONFIG_DM_SPI_FLASH))
> +			use_dt = false;
>  	}
>  
>  	s = strsep(&devstr, ":");
> @@ -152,9 +155,20 @@ static struct spi_flash *parse_dev(char *devstr)
>  			printf("Invalid SPI mode %s\n", s);
>  			return NULL;
>  		}
> +		if (IS_ENABLED(CONFIG_DM_SPI_FLASH))
> +			use_dt = false;
>  	}
>  
> -	dev = spi_flash_probe(bus, cs, speed, mode);
> +	if (IS_ENABLED(CONFIG_DM_SPI_FLASH) && use_dt) {
> +		struct udevice *new;
> +
> +		if (!spi_flash_probe_bus_cs(bus, cs, &new))
> +			dev = dev_get_uclass_priv(new);
> +		else
> +			dev = NULL;
> +	} else {
> +		dev = spi_flash_probe(bus, cs, speed, mode);
> +	}
>  	if (!dev) {
>  		printf("Failed to create SPI flash at %u:%u:%u:%u\n",
>  		       bus, cs, speed, mode);
>
> -- 
> 2.34.1