| Message ID | 20240522145255.995778-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 19a9aa9302276d49a4c4890f656359808d3a0151 |
| Headers | show |
| Series | spi: differentiate between unsupported and invalid requests | expand |
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@bootlin.com> > --- > 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@gmail.com> 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 >
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; return 0; }
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@bootlin.com> --- drivers/spi/spi-dw-bt1.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)