| Message ID | 20180426161820.2852-3-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 26.04.2018 18:18, Boris Brezillon wrote: > Some SPI/QuadSPI controllers only expose a high-level SPI memory > interface, thus preventing any regular SPI transfers from being done. > > In that case, SPI controller drivers can leave all ->transfer_xxx() > hooks empty and only implement the spi_mem_ops interface. > > Adjust the core to allow such situations: > - extend spi_controller_check_ops() to accept situations where all > ->transfer_xxx() pointers are NULL only if ->mem_ops != NULL > - make sure we do not initialize the SPI message queue if > ctlr->transfer_one and ctlr->transfer_one_message are missing > - return -ENOTSUPP if someone tries to do a regular SPI transfer on > a controller that does not support it > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> Reviewed-by: Frieder Schrempf <frieder.schrempf@exceet.de> Tested-by: Frieder Schrempf <frieder.schrempf@exceet.de> Regards, Frieder > --- > Changes in v4: > - none > > Changes in v3: > - none > > Changes in v2: > - patch added in v2 > --- > drivers/spi/spi.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 9ab65fb2738e..c85b0cf7b4a9 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -28,6 +28,7 @@ > #include <linux/slab.h> > #include <linux/mod_devicetable.h> > #include <linux/spi/spi.h> > +#include <linux/spi/spi-mem.h> > #include <linux/of_gpio.h> > #include <linux/pm_runtime.h> > #include <linux/pm_domain.h> > @@ -2071,12 +2072,19 @@ static int of_spi_register_master(struct spi_controller *ctlr) > static int spi_controller_check_ops(struct spi_controller *ctlr) > { > /* > - * The controller must at least implement one of the ->transfer() > - * hooks. > + * The controller may implement only the high-level SPI-memory like > + * operations if it does not support regular SPI transfers, and this is > + * valid use case. > + * If ->mem_ops is NULL, we request that at least one of the > + * ->transfer_xxx() method be implemented. > */ > - if (!ctlr->transfer && !ctlr->transfer_one && > - !ctlr->transfer_one_message) > + if (ctlr->mem_ops) { > + if (!ctlr->mem_ops->exec_op) > + return -EINVAL; > + } else if (!ctlr->transfer && !ctlr->transfer_one && > + !ctlr->transfer_one_message) { > return -EINVAL; > + } > > return 0; > } > @@ -2187,10 +2195,14 @@ int spi_register_controller(struct spi_controller *ctlr) > spi_controller_is_slave(ctlr) ? "slave" : "master", > dev_name(&ctlr->dev)); > > - /* If we're using a queued driver, start the queue */ > - if (ctlr->transfer) > + /* > + * If we're using a queued driver, start the queue. Note that we don't > + * need the queueing logic if the driver is only supporting high-level > + * memory operations. > + */ > + if (ctlr->transfer) { > dev_info(dev, "controller is unqueued, this is deprecated\n"); > - else { > + } else if (ctlr->transfer_one || ctlr->transfer_one_message) { > status = spi_controller_initialize_queue(ctlr); > if (status) { > device_del(&ctlr->dev); > @@ -2920,6 +2932,13 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) > { > struct spi_controller *ctlr = spi->controller; > > + /* > + * Some controllers do not support doing regular SPI transfers. Return > + * ENOTSUPP when this is the case. > + */ > + if (!ctlr->transfer) > + return -ENOTSUPP; > + > message->spi = spi; > > SPI_STATISTICS_INCREMENT_FIELD(&ctlr->statistics, spi_async); > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 9ab65fb2738e..c85b0cf7b4a9 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -28,6 +28,7 @@ #include <linux/slab.h> #include <linux/mod_devicetable.h> #include <linux/spi/spi.h> +#include <linux/spi/spi-mem.h> #include <linux/of_gpio.h> #include <linux/pm_runtime.h> #include <linux/pm_domain.h> @@ -2071,12 +2072,19 @@ static int of_spi_register_master(struct spi_controller *ctlr) static int spi_controller_check_ops(struct spi_controller *ctlr) { /* - * The controller must at least implement one of the ->transfer() - * hooks. + * The controller may implement only the high-level SPI-memory like + * operations if it does not support regular SPI transfers, and this is + * valid use case. + * If ->mem_ops is NULL, we request that at least one of the + * ->transfer_xxx() method be implemented. */ - if (!ctlr->transfer && !ctlr->transfer_one && - !ctlr->transfer_one_message) + if (ctlr->mem_ops) { + if (!ctlr->mem_ops->exec_op) + return -EINVAL; + } else if (!ctlr->transfer && !ctlr->transfer_one && + !ctlr->transfer_one_message) { return -EINVAL; + } return 0; } @@ -2187,10 +2195,14 @@ int spi_register_controller(struct spi_controller *ctlr) spi_controller_is_slave(ctlr) ? "slave" : "master", dev_name(&ctlr->dev)); - /* If we're using a queued driver, start the queue */ - if (ctlr->transfer) + /* + * If we're using a queued driver, start the queue. Note that we don't + * need the queueing logic if the driver is only supporting high-level + * memory operations. + */ + if (ctlr->transfer) { dev_info(dev, "controller is unqueued, this is deprecated\n"); - else { + } else if (ctlr->transfer_one || ctlr->transfer_one_message) { status = spi_controller_initialize_queue(ctlr); if (status) { device_del(&ctlr->dev); @@ -2920,6 +2932,13 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) { struct spi_controller *ctlr = spi->controller; + /* + * Some controllers do not support doing regular SPI transfers. Return + * ENOTSUPP when this is the case. + */ + if (!ctlr->transfer) + return -ENOTSUPP; + message->spi = spi; SPI_STATISTICS_INCREMENT_FIELD(&ctlr->statistics, spi_async);
Some SPI/QuadSPI controllers only expose a high-level SPI memory interface, thus preventing any regular SPI transfers from being done. In that case, SPI controller drivers can leave all ->transfer_xxx() hooks empty and only implement the spi_mem_ops interface. Adjust the core to allow such situations: - extend spi_controller_check_ops() to accept situations where all ->transfer_xxx() pointers are NULL only if ->mem_ops != NULL - make sure we do not initialize the SPI message queue if ctlr->transfer_one and ctlr->transfer_one_message are missing - return -ENOTSUPP if someone tries to do a regular SPI transfer on a controller that does not support it Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- Changes in v4: - none Changes in v3: - none Changes in v2: - patch added in v2 --- drivers/spi/spi.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-)