| Message ID | 20210908113450.788452-4-tanureal@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve support for AMD SPI controllers | expand |
On Wed, Sep 08, 2021 at 12:34:44PM +0100, Lucas Tanure wrote: > Some controllers can't write to the bus after a read without > releasing the chip select, so add flag and a check in spi core Nothing you've added ever reads this flag and I'm not sure what anything would be able to constructively do with it so why add the flag? I don't understand what the use case is.
On 9/8/21 1:37 PM, Mark Brown wrote: > On Wed, Sep 08, 2021 at 12:34:44PM +0100, Lucas Tanure wrote: >> Some controllers can't write to the bus after a read without >> releasing the chip select, so add flag and a check in spi core > > Nothing you've added ever reads this flag and I'm not sure what anything > would be able to constructively do with it so why add the flag? I don't > understand what the use case is. > __spi_validate checks this flag and makes sure the message can be received by the controller. __spi_validate can't fix the message, so it only rejects the message.
On Thu, Sep 09, 2021 at 11:51:21AM +0100, Lucas tanure wrote: > On 9/8/21 1:37 PM, Mark Brown wrote: > > On Wed, Sep 08, 2021 at 12:34:44PM +0100, Lucas Tanure wrote: > > > Some controllers can't write to the bus after a read without > > > releasing the chip select, so add flag and a check in spi core > > Nothing you've added ever reads this flag and I'm not sure what anything > > would be able to constructively do with it so why add the flag? I don't > > understand what the use case is. > __spi_validate checks this flag and makes sure the message can be received > by the controller. > __spi_validate can't fix the message, so it only rejects the message. Given that this is hardware that can't possibly work how useful is that validation? It's a fairly unusual thing for devices to do in the first place, only applies if using the native chip select (which your patch doesn't check for) and I am not sure that this is a general enough pattern in controllers to have generic support for. I suspect that a lot of controllers with similar restrictions will be even more limited than this, for example only supporting one or two transfers with limits on the data, so it's not clear to me how useful this capability would be.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 65d14af9c015..1dbc8b6f1398 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3724,6 +3724,17 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return -EINVAL; } + if (ctlr->flags & SPI_CONTROLLER_NO_TX_RX_CS) { + bool read = false; + + list_for_each_entry(xfer, &message->transfers, transfer_list) { + if (read && xfer->tx_buf) + return -EINVAL; + if (xfer->rx_buf && !xfer->cs_change) + read = true; + } + } + message->status = -EINPROGRESS; return 0; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 8371bca13729..916b982dc2a1 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -514,6 +514,7 @@ struct spi_controller { #define SPI_CONTROLLER_MUST_TX BIT(4) /* requires tx */ #define SPI_MASTER_GPIO_SS BIT(5) /* GPIO CS must select slave */ +#define SPI_CONTROLLER_NO_TX_RX_CS BIT(6) /* can't write after a read in the same CS */ /* flag indicating if the allocation of this struct is devres-managed */ bool devm_allocated;
Some controllers can't write to the bus after a read without releasing the chip select, so add flag and a check in spi core Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- drivers/spi/spi.c | 11 +++++++++++ include/linux/spi/spi.h | 1 + 2 files changed, 12 insertions(+)