| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdVsYohH5FVv6r4ha0AaHRoHjNF1ErjW1FNF7ZAQR9ntxg@mail.gmail.com>
Date: Mon, 3 Jul 2023 12:19:26 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
Cc: Mark Brown <broonie@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Magnus Damm <magnus.damm@...il.com>,
linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org,
linux-renesas-soc@...r.kernel.org,
Chris Paterson <Chris.Paterson2@...esas.com>,
Biju Das <biju.das@...renesas.com>
Subject: Re: [PATCH v2 3/5] spi: Add support for Renesas CSI
Hi Fabrizio,
On Thu, Jun 22, 2023 at 1:34 PM Fabrizio Castro
<fabrizio.castro.jz@...esas.com> wrote:
> The RZ/V2M SoC comes with the Clocked Serial Interface (CSI)
> IP, which is a master/slave SPI controller.
>
> This commit adds a driver to support CSI master mode.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> ---
>
> v2: edited includes in drivers/spi/spi-rzv2m-csi.c
Thanks for your patch!
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -825,6 +825,12 @@ config SPI_RSPI
> help
> SPI driver for Renesas RSPI and QSPI blocks.
>
> +config SPI_RZV2M_CSI
> + tristate "Renesas RZV2M CSI controller"
RZ/V2M (patch sent)
> + depends on ARCH_RENESAS || COMPILE_TEST
> + help
> + SPI driver for Renesas RZ/V2M Clocked Serial Interface (CSI)
> +
> config SPI_QCOM_QSPI
> tristate "QTI QSPI controller"
> depends on ARCH_QCOM || COMPILE_TEST
> --- /dev/null
> +++ b/drivers/spi/spi-rzv2m-csi.c
> @@ -0,0 +1,667 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Renesas RZ/V2M Clocked Serial Interface (CSI) driver
> + *
> + * Copyright (C) 2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/count_zeros.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/spi/spi.h>
> +
> +/* Registers */
> +#define CSI_MODE 0x00 /* CSI mode control */
> +#define CSI_CLKSEL 0x04 /* CSI clock select */
> +#define CSI_CNT 0x08 /* CSI control */
> +#define CSI_INT 0x0C /* CSI interrupt status */
> +#define CSI_IFIFOL 0x10 /* CSI receive FIFO level display */
> +#define CSI_OFIFOL 0x14 /* CSI transmit FIFO level display */
> +#define CSI_IFIFO 0x18 /* CSI receive window */
> +#define CSI_OFIFO 0x1C /* CSI transmit window */
> +#define CSI_FIFOTRG 0x20 /* CSI FIFO trigger level */
> +
> +/* CSI_MODE */
> +#define CSI_MODE_CSIE BIT(7)
> +#define CSI_MODE_TRMD BIT(6)
> +#define CSI_MODE_CCL BIT(5)
> +#define CSI_MODE_DIR BIT(4)
> +#define CSI_MODE_CSOT BIT(0)
> +
> +#define CSI_MODE_SETUP 0x00000040
> +
> +/* CSI_CLKSEL */
> +#define CSI_CLKSEL_CKP BIT(17)
> +#define CSI_CLKSEL_DAP BIT(16)
> +#define CSI_CLKSEL_SLAVE BIT(15)
> +#define CSI_CLKSEL_CKS GENMASK(14, 1)
> +
> +/* CSI_CNT */
> +#define CSI_CNT_CSIRST BIT(28)
> +#define CSI_CNT_R_TRGEN BIT(19)
> +#define CSI_CNT_UNDER_E BIT(13)
> +#define CSI_CNT_OVERF_E BIT(12)
> +#define CSI_CNT_TREND_E BIT(9)
> +#define CSI_CNT_CSIEND_E BIT(8)
> +#define CSI_CNT_T_TRGR_E BIT(4)
> +#define CSI_CNT_R_TRGR_E BIT(0)
> +
> +/* CSI_INT */
> +#define CSI_INT_UNDER BIT(13)
> +#define CSI_INT_OVERF BIT(12)
> +#define CSI_INT_TREND BIT(9)
> +#define CSI_INT_CSIEND BIT(8)
> +#define CSI_INT_T_TRGR BIT(4)
> +#define CSI_INT_R_TRGR BIT(0)
> +
> +/* CSI_FIFOTRG */
> +#define CSI_FIFOTRG_R_TRG GENMASK(2, 0)
> +
> +#define CSI_FIFO_SIZE_BYTES 32
> +#define CSI_FIFO_HALF_SIZE 16
> +#define CSI_EN_DIS_TIMEOUT_US 100
> +#define CSI_CKS_MAX 0x3FFF
> +
> +#define UNDERRUN_ERROR BIT(0)
> +#define OVERFLOW_ERROR BIT(1)
> +#define TX_TIMEOUT_ERROR BIT(2)
> +#define RX_TIMEOUT_ERROR BIT(3)
> +
> +#define CSI_MAX_SPI_SCKO 8000000
> +
> +struct rzv2m_csi_priv {
> + void __iomem *base;
> + struct clk *csiclk;
> + struct clk *pclk;
> + struct device *dev;
> + struct spi_controller *controller;
> + const u8 *txbuf;
> + u8 *rxbuf;
> + int buffer_len;
> + int bytes_sent;
> + int bytes_received;
> + int bytes_to_transfer;
> + int words_to_transfer;
All these ints should be unsigned.
> + unsigned char bytes_per_word;
3-byte gap
> + wait_queue_head_t wait;
> + u8 errors;
3 byte gap
> + u32 status;
> +};
> +
> +static int rzv2m_csi_fill_txfifo(struct rzv2m_csi_priv *csi)
> +{
> + int i;
unsigned int
> +
> + if (readl(csi->base + CSI_OFIFOL))
> + return -EIO;
> +
> + if (csi->bytes_per_word == 2) {
> + u16 *buf = (u16 *)csi->txbuf;
I think you can get rid of the casts by making rxbuf a const void *.
> +
> + for (i = 0; i < csi->words_to_transfer; i++)
> + writel(buf[i], csi->base + CSI_OFIFO);
> + } else {
> + u8 *buf = (u8 *)csi->txbuf;
> +
> + for (i = 0; i < csi->words_to_transfer; i++)
> + writel(buf[i], csi->base + CSI_OFIFO);
> + }
> +
> + csi->txbuf += csi->bytes_to_transfer;
> + csi->bytes_sent += csi->bytes_to_transfer;
> +
> + return 0;
> +}
> +
> +static int rzv2m_csi_read_rxfifo(struct rzv2m_csi_priv *csi)
> +{
> + int i;
> +
> + if (readl(csi->base + CSI_IFIFOL) != csi->bytes_to_transfer)
> + return -EIO;
> +
> + if (csi->bytes_per_word == 2) {
> + u16 *buf = (u16 *)csi->rxbuf;
Similar for rxbuf.
> +
> + for (i = 0; i < csi->words_to_transfer; i++)
> + buf[i] = (u16)readl(csi->base + CSI_IFIFO);
> + } else {
> + u8 *buf = (u8 *)csi->rxbuf;
> +
> + for (i = 0; i < csi->words_to_transfer; i++)
> + buf[i] = (u8)readl(csi->base + CSI_IFIFO);
> + }
> +
> + csi->rxbuf += csi->bytes_to_transfer;
> + csi->bytes_received += csi->bytes_to_transfer;
> +
> + return 0;
> +}
> +
> +static inline void rzv2m_csi_calc_current_transfer(struct rzv2m_csi_priv *csi)
> +{
> + int bytes_transferred = max_t(int, csi->bytes_received, csi->bytes_sent);
> + int bytes_remaining = csi->buffer_len - bytes_transferred;
> + int to_transfer;
unsigned...
> +
> + if (csi->txbuf)
> + /*
> + * Leaving a little bit of headroom in the FIFOs makes it very
> + * hard to raise an overflow error (which is only possible
> + * when IP transmits and receives at the same time).
> + */
> + to_transfer = min_t(int, CSI_FIFO_HALF_SIZE, bytes_remaining);
> + else
> + to_transfer = min_t(int, CSI_FIFO_SIZE_BYTES, bytes_remaining);
Why min_t(int, ...)? Both values are int.
It would be better to make both unsigned, though.
> +
> + if (csi->bytes_per_word == 2)
> + to_transfer >>= 1;
> +
> + /*
> + * We can only choose a trigger level from a predefined set of values.
> + * This will pick a value that is the greatest possible integer that's
> + * less than or equal to the number of bytes we need to transfer.
> + * This may result in multiple smaller transfers.
> + */
> + csi->words_to_transfer = x_trg_words[to_transfer - 1];
> +
> + if (csi->bytes_per_word == 2)
> + csi->bytes_to_transfer = csi->words_to_transfer << 1;
> + else
> + csi->bytes_to_transfer = csi->words_to_transfer;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
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
Powered by blists - more mailing lists