Hi Fabrizio, Thanks for the patch. > -----Original Message----- > From: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> > Sent: 24 June 2025 20:23 > Das <biju.das.jz@xxxxxxxxxxxxxx>; Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > Subject: [PATCH 3/6] spi: Add driver for the RZ/V2H(P) RSPI IP > > The Renesas RZ/V2H(P) RSPI IP supports 4-wire and 3-wire serial communications in both host role and > target role. > It can use a DMA, but the I/O can also be driven by the processor. > > RX-only, TX-only, and RX-TX operations are available in DMA mode, while in processor I/O mode it only > RX-TX operations are supported. > > Add a driver to support 4-wire serial communications as host role in processor I/O mode. > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> > --- > > I have noticed a problem when unbinding the driver that is solved by: > https://lore.kernel.org/all/20250616135357.3929441-1-claudiu.beznea.uj@xxxxxxxxxxxxxx/ > > Once the above series gets accepted I'll send a patch to add runtime pm support, and I'll also switch > to using devm_spi_register_controller. > > drivers/spi/Kconfig | 8 + > drivers/spi/Makefile | 1 + > drivers/spi/spi-rzv2h-rspi.c | 469 +++++++++++++++++++++++++++++++++++ > 3 files changed, 478 insertions(+) > create mode 100644 drivers/spi/spi-rzv2h-rspi.c > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index f2d2295a5501..fcc6987945fa 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -923,6 +923,14 @@ config SPI_RSPI > help > SPI driver for Renesas RSPI and QSPI blocks. > > +config SPI_RZV2H_RSPI > + tristate "Renesas RZ/V2H RSPI controller" > + depends on ARCH_RENESAS || COMPILE_TEST > + help > + RSPI driver for the Renesas RZ/V2H Serial Peripheral Interface (RSPI). > + RSPI supports both SPI host and SPI target roles. This option only > + enables the SPI host role. > + > config SPI_RZV2M_CSI > tristate "Renesas RZ/V2M CSI controller" > depends on ARCH_RENESAS || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile > index 4ea89f6fc531..c19d02653b8a 100644 > --- a/drivers/spi/Makefile > +++ b/drivers/spi/Makefile > @@ -126,6 +126,7 @@ obj-$(CONFIG_MACH_REALTEK_RTL) += spi-realtek-rtl.o > obj-$(CONFIG_SPI_REALTEK_SNAND) += spi-realtek-rtl-snand.o > obj-$(CONFIG_SPI_RPCIF) += spi-rpc-if.o > obj-$(CONFIG_SPI_RSPI) += spi-rspi.o > +obj-$(CONFIG_SPI_RZV2H_RSPI) += spi-rzv2h-rspi.o > obj-$(CONFIG_SPI_RZV2M_CSI) += spi-rzv2m-csi.o > obj-$(CONFIG_SPI_S3C64XX) += spi-s3c64xx.o > obj-$(CONFIG_SPI_SC18IS602) += spi-sc18is602.o > diff --git a/drivers/spi/spi-rzv2h-rspi.c b/drivers/spi/spi-rzv2h-rspi.c new file mode 100644 index > 000000000000..9541f2c2ab2b > --- /dev/null > +++ b/drivers/spi/spi-rzv2h-rspi.c > @@ -0,0 +1,469 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Renesas RZ/V2H Renesas Serial Peripheral Interface (RSPI) > + * > + * Copyright (C) 2025 Renesas Electronics Corporation */ > + > +#include <linux/bitops.h> > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/limits.h> > +#include <linux/log2.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/property.h> > +#include <linux/reset.h> > +#include <linux/spi/spi.h> > + > +/* Registers */ > +#define RSPI_SPDR 0x00 > +#define RSPI_SPCR 0x08 > +#define RSPI_SSLP 0x10 > +#define RSPI_SPBR 0x11 > +#define RSPI_SPSCR 0x13 > +#define RSPI_SPCMD 0x14 > +#define RSPI_SPDCR2 0x44 > +#define RSPI_SPSR 0x52 > +#define RSPI_SPSRC 0x6a > +#define RSPI_SPFCR 0x6c > + > +/* Register SPCR */ > +#define RSPI_SPCR_MSTR BIT(30) > +#define RSPI_SPCR_SPRIE BIT(17) > +#define RSPI_SPCR_SCKASE BIT(12) > +#define RSPI_SPCR_SPE BIT(0) > + > +/* Register SPBR */ > +#define RSPI_SPBR_SPR_MIN 0 > +#define RSPI_SPBR_SPR_MAX 255 > + > +/* Register SPCMD */ > +#define RSPI_SPCMD_SSLA GENMASK(25, 24) > +#define RSPI_SPCMD_SPB GENMASK(20, 16) > +#define RSPI_SPCMD_LSBF BIT(12) > +#define RSPI_SPCMD_SSLKP BIT(7) > +#define RSPI_SPCMD_BRDV GENMASK(3, 2) > +#define RSPI_SPCMD_CPOL BIT(1) > +#define RSPI_SPCMD_CPHA BIT(0) > + > +#define RSPI_SPCMD_BRDV_MIN 0 > +#define RSPI_SPCMD_BRDV_MAX 3 > + > +/* Register SPDCR2 */ > +#define RSPI_SPDCR2_TTRG GENMASK(11, 8) > +#define RSPI_SPDCR2_RTRG GENMASK(3, 0) > +#define RSPI_FIFO_SIZE 16 > + > +/* Register SPSR */ > +#define RSPI_SPSR_SPRF BIT(15) > + > +/* Register RSPI_SPSRC */ > +#define RSPI_SPSRC_CLEAR 0xfd80 > + > +#define RSPI_RESET_NUM 2 > + > +enum rspi_clocks { > + RSPI_CLK_PCLK, > + RSPI_CLK_PCLK_SFR, > + RSPI_CLK_TCLK, > + RSPI_CLK_NUM > +}; Do we need this enum? > + > +struct rzv2h_rspi_priv { > + struct reset_control_bulk_data resets[RSPI_RESET_NUM]; > + struct clk_bulk_data clks[RSPI_CLK_NUM]; It is used only in probe/remove. By using devm_clk_bulk_get_all_enabled() this can be local?? > + struct spi_controller *controller; > + void __iomem *base; > + wait_queue_head_t wait; > + unsigned int bytes_per_word; > + u32 freq; > + u16 status; > +}; > + > +#define RZV2H_RSPI_TX(func, type) \ > +static inline void rzv2h_rspi_tx_##type(struct rzv2h_rspi_priv *rspi, \ > + const void *txbuf, \ > + unsigned int index) { \ > + type buf = 0; \ > + \ > + if (txbuf) \ > + buf = ((type *)txbuf)[index]; \ > + \ > + func(buf, rspi->base + RSPI_SPDR); \ > +} > + > +#define RZV2H_RSPI_RX(func, type) \ > +static inline void rzv2h_rspi_rx_##type(struct rzv2h_rspi_priv *rspi, \ > + void *rxbuf, \ > + unsigned int index) { \ > + type buf = func(rspi->base + RSPI_SPDR); \ > + \ > + if (rxbuf) \ > + ((type *)rxbuf)[index] = buf; \ > +} > + > +RZV2H_RSPI_TX(writel, u32) > +RZV2H_RSPI_TX(writew, u16) > +RZV2H_RSPI_TX(writeb, u8) > +RZV2H_RSPI_RX(readl, u32) > +RZV2H_RSPI_RX(readw, u16) > +RZV2H_RSPI_RX(readl, u8) > + > +static void rzv2h_rspi_reg_rmw(const struct rzv2h_rspi_priv *rspi, > + int reg_offs, u32 bit_mask, u32 value) { > + u32 tmp; > + > + value <<= __ffs(bit_mask); > + tmp = (readl(rspi->base + reg_offs) & ~bit_mask) | value; > + writel(tmp, rspi->base + reg_offs); > +} > + > +static inline void rzv2h_rspi_spe_disable(const struct rzv2h_rspi_priv > +*rspi) { > + rzv2h_rspi_reg_rmw(rspi, RSPI_SPCR, RSPI_SPCR_SPE, 0); } > + > +static inline void rzv2h_rspi_spe_enable(const struct rzv2h_rspi_priv > +*rspi) { > + rzv2h_rspi_reg_rmw(rspi, RSPI_SPCR, RSPI_SPCR_SPE, 1); } > + > +static inline void rzv2h_rspi_clear_fifos(const struct rzv2h_rspi_priv > +*rspi) { > + writeb(1, rspi->base + RSPI_SPFCR); > +} > + > +static inline void rzv2h_rspi_clear_all_irqs(struct rzv2h_rspi_priv > +*rspi) { > + writew(RSPI_SPSRC_CLEAR, rspi->base + RSPI_SPSRC); > + rspi->status = 0; > +} > + > +static irqreturn_t rzv2h_rx_irq_handler(int irq, void *data) { > + struct rzv2h_rspi_priv *rspi = data; > + > + rspi->status = readw(rspi->base + RSPI_SPSR); > + wake_up(&rspi->wait); > + > + return IRQ_HANDLED; > +} > + > +static inline int rzv2h_rspi_wait_for_interrupt(struct rzv2h_rspi_priv *rspi, > + u32 wait_mask) > +{ > + return wait_event_timeout(rspi->wait, (rspi->status & wait_mask), > + HZ) == 0 ? -ETIMEDOUT : 0; > +} > + > +static void rzv2h_rspi_send(struct rzv2h_rspi_priv *rspi, const void *txbuf, > + unsigned int index) > +{ > + switch (rspi->bytes_per_word) { > + case 4: > + rzv2h_rspi_tx_u32(rspi, txbuf, index); > + break; > + case 2: > + rzv2h_rspi_tx_u16(rspi, txbuf, index); > + break; > + default: > + rzv2h_rspi_tx_u8(rspi, txbuf, index); > + } > +} > + > +static int rzv2h_rspi_receive(struct rzv2h_rspi_priv *rspi, void *rxbuf, > + unsigned int index) > +{ > + int ret; > + > + ret = rzv2h_rspi_wait_for_interrupt(rspi, RSPI_SPSR_SPRF); > + if (ret) > + return ret; > + > + switch (rspi->bytes_per_word) { > + case 4: > + rzv2h_rspi_rx_u32(rspi, rxbuf, index); > + break; > + case 2: > + rzv2h_rspi_rx_u16(rspi, rxbuf, index); > + break; > + default: > + rzv2h_rspi_rx_u8(rspi, rxbuf, index); > + } > + > + return 0; > +} > + > +static int rzv2h_rspi_transfer_one(struct spi_controller *controller, > + struct spi_device *spi, > + struct spi_transfer *transfer) > +{ > + struct rzv2h_rspi_priv *rspi = spi_controller_get_devdata(controller); > + unsigned int words_to_transfer, i; > + int ret = 0; > + > + transfer->effective_speed_hz = rspi->freq; > + words_to_transfer = transfer->len / rspi->bytes_per_word; > + > + for (i = 0; i < words_to_transfer; i++) { > + rzv2h_rspi_clear_all_irqs(rspi); > + > + rzv2h_rspi_send(rspi, transfer->tx_buf, i); > + > + ret = rzv2h_rspi_receive(rspi, transfer->rx_buf, i); > + if (ret) > + break; > + } > + > + rzv2h_rspi_clear_all_irqs(rspi); > + > + if (ret) > + transfer->error = SPI_TRANS_FAIL_IO; > + > + spi_finalize_current_transfer(controller); > + > + return ret; > +} > + > +static inline u32 rzv2h_rspi_calc_bitrate(unsigned long tclk_rate, u8 spr, > + u8 brdv) > +{ > + return DIV_ROUND_UP(tclk_rate, (2 * (spr + 1) * (1 << brdv))); } > + > +static u32 rzv2h_rspi_setup_clock(struct rzv2h_rspi_priv *rspi, u32 hz) > +{ > + unsigned long tclk_rate; > + int spr; > + u8 brdv; > + > + /* > + * From the manual: > + * Bit rate = f(RSPI_n_TCLK)/(2*(n+1)*2^(N)) > + * > + * Where: > + * * RSPI_n_TCLK is fixed to 200MHz on V2H > + * * n = SPR - is RSPI_SPBR.SPR (from 0 to 255) > + * * N = BRDV - is RSPI_SPCMD.BRDV (from 0 to 3) > + */ > + tclk_rate = clk_get_rate(rspi->clks[RSPI_CLK_TCLK].clk); > + for (brdv = RSPI_SPCMD_BRDV_MIN; brdv <= RSPI_SPCMD_BRDV_MAX; brdv++) { > + spr = DIV_ROUND_UP(tclk_rate, hz * (1 << (brdv + 1))); > + spr--; > + if (spr >= RSPI_SPBR_SPR_MIN && spr <= RSPI_SPBR_SPR_MAX) > + goto clock_found; > + } > + > + return 0; > + > +clock_found: > + rzv2h_rspi_reg_rmw(rspi, RSPI_SPCMD, RSPI_SPCMD_BRDV, brdv); > + writeb(spr, rspi->base + RSPI_SPBR); > + > + return rzv2h_rspi_calc_bitrate(tclk_rate, spr, brdv); } > + > +static int rzv2h_rspi_prepare_message(struct spi_controller *ctlr, > + struct spi_message *message) > +{ > + struct rzv2h_rspi_priv *rspi = spi_controller_get_devdata(ctlr); > + const struct spi_device *spi = message->spi; > + struct spi_transfer *xfer; > + u32 speed_hz = U32_MAX; > + u8 bits_per_word; > + u32 conf32; > + u16 conf16; > + > + /* Make sure SPCR.SPE is 0 before amending the configuration */ > + rzv2h_rspi_spe_disable(rspi); > + > + /* Configure the device to work in "host" mode */ > + conf32 = RSPI_SPCR_MSTR; > + > + /* Auto-stop function */ > + conf32 |= RSPI_SPCR_SCKASE; > + > + /* SPI receive buffer full interrupt enable */ > + conf32 |= RSPI_SPCR_SPRIE; > + > + writel(conf32, rspi->base + RSPI_SPCR); > + > + /* Use SPCMD0 only */ > + writeb(0x0, rspi->base + RSPI_SPSCR); > + > + /* Setup mode */ > + conf32 = FIELD_PREP(RSPI_SPCMD_CPOL, !!(spi->mode & SPI_CPOL)); > + conf32 |= FIELD_PREP(RSPI_SPCMD_CPHA, !!(spi->mode & SPI_CPHA)); > + conf32 |= FIELD_PREP(RSPI_SPCMD_LSBF, !!(spi->mode & SPI_LSB_FIRST)); > + conf32 |= FIELD_PREP(RSPI_SPCMD_SSLKP, 1); > + conf32 |= FIELD_PREP(RSPI_SPCMD_SSLA, spi_get_chipselect(spi, 0)); > + writel(conf32, rspi->base + RSPI_SPCMD); > + if (spi->mode & SPI_CS_HIGH) > + writeb(BIT(spi_get_chipselect(spi, 0)), rspi->base + RSPI_SSLP); > + else > + writeb(0, rspi->base + RSPI_SSLP); > + > + /* Setup FIFO thresholds */ > + conf16 = FIELD_PREP(RSPI_SPDCR2_TTRG, RSPI_FIFO_SIZE - 1); > + conf16 |= FIELD_PREP(RSPI_SPDCR2_RTRG, 0); > + writew(conf16, rspi->base + RSPI_SPDCR2); > + > + rzv2h_rspi_clear_fifos(rspi); > + > + list_for_each_entry(xfer, &message->transfers, transfer_list) { > + if (!xfer->speed_hz) > + continue; > + > + speed_hz = min(xfer->speed_hz, speed_hz); > + bits_per_word = xfer->bits_per_word; > + } > + > + if (speed_hz == U32_MAX) > + return -EINVAL; > + > + rspi->bytes_per_word = roundup_pow_of_two(BITS_TO_BYTES(bits_per_word)); > + rzv2h_rspi_reg_rmw(rspi, RSPI_SPCMD, RSPI_SPCMD_SPB, bits_per_word - > +1); > + > + rspi->freq = rzv2h_rspi_setup_clock(rspi, speed_hz); > + if (!rspi->freq) > + return -EINVAL; > + > + rzv2h_rspi_spe_enable(rspi); > + > + return 0; > +} > + > +static int rzv2h_rspi_unprepare_message(struct spi_controller *ctlr, > + struct spi_message *message) > +{ > + struct rzv2h_rspi_priv *rspi = spi_controller_get_devdata(ctlr); > + > + rzv2h_rspi_spe_disable(rspi); > + rzv2h_rspi_clear_fifos(rspi); > + > + return 0; > +} > + > +static int rzv2h_rspi_probe(struct platform_device *pdev) { > + struct spi_controller *controller; > + struct device *dev = &pdev->dev; > + struct rzv2h_rspi_priv *rspi; > + unsigned long tclk_rate; > + int irq_rx, ret; > + > + controller = devm_spi_alloc_host(dev, sizeof(*rspi)); > + if (!controller) > + return -ENOMEM; > + > + rspi = spi_controller_get_devdata(controller); > + platform_set_drvdata(pdev, rspi); > + > + rspi->controller = controller; > + > + rspi->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(rspi->base)) > + return PTR_ERR(rspi->base); > + > + rspi->clks[RSPI_CLK_PCLK].id = "pclk"; > + rspi->clks[RSPI_CLK_PCLK_SFR].id = "pclk_sfr"; > + rspi->clks[RSPI_CLK_TCLK].id = "tclk"; > + ret = devm_clk_bulk_get(dev, RSPI_CLK_NUM, rspi->clks); > + if (ret) > + return dev_err_probe(dev, ret, "cannot get clocks\n"); > + > + rspi->resets[0].id = "presetn"; > + rspi->resets[1].id = "tresetn"; > + ret = devm_reset_control_bulk_get_exclusive(dev, RSPI_RESET_NUM, > + rspi->resets); > + if (ret) > + return dev_err_probe(dev, ret, "cannot get resets\n"); > + > + irq_rx = platform_get_irq_byname(pdev, "rx"); > + if (irq_rx < 0) > + return dev_err_probe(dev, irq_rx, "cannot get IRQ 'rx'\n"); > + > + ret = devm_request_irq(dev, irq_rx, rzv2h_rx_irq_handler, 0, > + dev_name(dev), rspi); > + if (ret) > + return dev_err_probe(dev, ret, "cannot request `rx` IRQ\n"); > + > + ret = clk_bulk_prepare_enable(RSPI_CLK_NUM, rspi->clks); > + if (ret) > + return dev_err_probe(dev, ret, "failed to enable clocks\n"); Can't we use devm_clk_bulk_get_all_enabled() instead that will fill struct clk_bulk_data?? > + > + ret = reset_control_bulk_deassert(RSPI_RESET_NUM, rspi->resets); > + if (ret) { > + dev_err(dev, "failed to deassert resets\n"); > + goto quit_clocks; > + } > + > + init_waitqueue_head(&rspi->wait); > + > + tclk_rate = clk_get_rate(rspi->clks[RSPI_CLK_TCLK].clk); Is it not possible to get this clk from struct clk_bulk_data by using the id "tclk"?? Cheers, Biju
