Hi Fabrizio,
On Tue, Sep 26, 2023 at 11:08 PM Fabrizio Castro
<fabrizio.castro.jz@xxxxxxxxxxx> wrote:
> The CSI IP found inside the Renesas RZ/V2M SoC supports
> both SPI Master and SPI Slave roles.
>
> When working in slave mode, the CSI IP has the option
> of using its Slave Select (SS) pin to enable TX and RX
> operations. Since the SPI slave cannot control the clock,
> when working as slave it's best not to stop operations
> during a transfer, as by doing so the IP will not send or
> receive data, regardless of clock and active level on pin SS.
> A side effect from not stopping operations is that the RX
> FIFO needs to be flushed, word by word, when RX data needs
> to be discarded.
>
> Finally, when in slave mode timings are tighter, as missing a
> deadline translates to errors being thrown, resulting in
> aborting the transfer. In order to speed things up, we can
> avoid waiting for the TX FIFO to be empty, we can just wait
> for the RX FIFO to contain at least the number of words that
> we expect.
>
> Add slave support to the currently existing CSI driver.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
Thanks for your patch!
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -861,8 +861,10 @@ config SPI_RSPI
> config SPI_RZV2M_CSI
> tristate "Renesas RZ/V2M CSI controller"
> depends on ARCH_RENESAS || COMPILE_TEST
> + depends on SPI_SLAVE
Isn't that a bit too strict?
The driver can/should be used/usable in host mode when SPI_SLAVE
is not enabled.
> help
> - SPI driver for Renesas RZ/V2M Clocked Serial Interface (CSI)
> + SPI driver for Renesas RZ/V2M Clocked Serial Interface (CSI).
> + CSI supports master and slave roles.
>
> config SPI_QCOM_QSPI
> tristate "QTI QSPI controller"
> --- a/drivers/spi/spi-rzv2m-csi.c
> +++ b/drivers/spi/spi-rzv2m-csi.c
> @@ -99,6 +112,9 @@ struct rzv2m_csi_priv {
> wait_queue_head_t wait;
> u32 errors;
> u32 status;
> + int mode;
Do you need this flag?
You can use spi_controller_is_target() instead.
> + int slave_select;
> + bool slave_aborted;
> };
>
> static void rzv2m_csi_reg_write_bit(const struct rzv2m_csi_priv *csi,
> @@ -279,32 +303,23 @@ static int rzv2m_csi_wait_for_interrupt(struct rzv2m_csi_priv *csi,
>
> rzv2m_csi_enable_irqs(csi, enable_bits);
>
> - ret = wait_event_timeout(csi->wait,
> - ((csi->status & wait_mask) == wait_mask) ||
> - csi->errors, HZ);
> + if (csi->mode == RZV2M_CSI_SPI_SLAVE) {
spi_controller_is_target()
> + ret = wait_event_interruptible(csi->wait,
> + ((csi->status & wait_mask) == wait_mask) ||
> + csi->errors || csi->slave_aborted);
target_aborted (everywhere)
> + if (ret || csi->slave_aborted)
> + ret = -EINTR;
> + } else {
> + ret = wait_event_timeout(csi->wait,
> + ((csi->status & wait_mask) == wait_mask) ||
> + csi->errors, HZ) == 0 ? -ETIMEDOUT : 0;
> + }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
