Re: [PATCH 1/4] spi: dw: differentiate between unsupported and invalid requests

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Wed, May 22, 2024 at 04:52:52PM +0200, Miquel Raynal wrote:
> The driver does not support dirmap write operations, return -EOPTNOTSUPP
> in this case.
> 
> Most controllers have a maximum linear mapping area. Requests beyond
> this limit can be considered invalid, rather than unsupported.
> 
> From a caller (and reviewer) point of view, distinguising between the
> two may be helpful because somehow one can be "fixed" while the other
> will always be refused no matter how hard we try.
> 
> As part of a wider work to bring spi-nand continuous reads, it was
> useful to easily catch the upper limit direct mapping boundaries for
> each controller, with the idea of enlarging this area from a page to an
> eraseblock, without risking too many regressions.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/spi/spi-dw-bt1.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-bt1.c b/drivers/spi/spi-dw-bt1.c
> index 5391bcac305c..4577e8096cd9 100644
> --- a/drivers/spi/spi-dw-bt1.c
> +++ b/drivers/spi/spi-dw-bt1.c
> @@ -55,13 +55,15 @@ static int dw_spi_bt1_dirmap_create(struct spi_mem_dirmap_desc *desc)
>  	    !dwsbt1->dws.mem_ops.supports_op(desc->mem, &desc->info.op_tmpl))
>  		return -EOPNOTSUPP;
> 
> +	if (desc->info.op_tmpl.data.dir != SPI_MEM_DATA_IN)
> +		return -EOPNOTSUPP;
> +
>  	/*
>  	 * Make sure the requested region doesn't go out of the physically
> -	 * mapped flash memory bounds and the operation is read-only.
> +	 * mapped flash memory bounds.
>  	 */
> -	if (desc->info.offset + desc->info.length > dwsbt1->map_len ||
> -	    desc->info.op_tmpl.data.dir != SPI_MEM_DATA_IN)
> -		return -EOPNOTSUPP;
> +	if (desc->info.offset + desc->info.length > dwsbt1->map_len)
> +		return -EINVAL;

Seems reasonable. Indeed the out of bounds situation is better
described by the EINVAL error. So the change looks good to me:

Reviewed-by: Serge Semin <fancer.lancer@xxxxxxxxx>

But note the error number won't be propagated that far and will be
overwritten in spi_mem_dirmap_create() with either zero or
-EOPNOTSUPP.

-Serge(y)

>  
>  	return 0;
>  }
> -- 
> 2.40.1
>