Hello Yamada-san, > From: Masahiro Yamada [mailto:yamada.masahiro@xxxxxxxxxxxxx] > Sent: Thursday, July 26, 2018 6:13 PM > To: Hayashibara, Keiji/林原 啓二 <hayashibara.keiji@xxxxxxxxxxxxx> > Subject: Re: [PATCH v2 2/2] spi: add SPI controller driver for UniPhier SoC > > Hi. > > > 2018-07-26 16:09 GMT+09:00 Keiji Hayashibara <hayashibara.keiji@xxxxxxxxxxxxx>: > > Add SPI controller driver implemented in Socionext UniPhier SoCs. > > > > UniPhier SoCs have two types SPI controllers; SCSSI supports a single > > channel, and MCSSI supports multiple channels. > > This driver supports SCSSI only. > > > > This controller has 32bit TX/RX FIFO with depth of eight entry, and > > supports the SPI master mode only. > > > > This commit is implemented in PIO transfer mode, not DMA transfer. > > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx> > > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@xxxxxxxxxxxxx> > > --- > > drivers/spi/Kconfig | 13 ++ > > drivers/spi/Makefile | 1 + > > drivers/spi/spi-uniphier.c | 539 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 553 insertions(+) > > create mode 100644 drivers/spi/spi-uniphier.c > > > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index > > ad5d68e..671d078 100644 > > --- a/drivers/spi/Kconfig > > +++ b/drivers/spi/Kconfig > > @@ -688,6 +688,19 @@ config SPI_TXX9 > > help > > SPI driver for Toshiba TXx9 MIPS SoCs > > > > +config SPI_UNIPHIER > > + tristate "Socionext UniPhier SPI Controller" > > + depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF > > + help > > + This enables a driver for the Socionext UniPhier SoC SCSSI SPI controller. > > + > > + UniPhier SoCs have SCSSI and MCSSI SPI controllers. > > + Every UniPhier SoC has SCSSI which supports single channel. > > + Older UniPhier Pro4/Pro5 also has MCSSI which support multiple channels. > > + This driver supports SCSSI only. > > + > > + If your SoC supports SCSSI, say Y here. > > + > > config SPI_XCOMM > > tristate "Analog Devices AD-FMCOMMS1-EBZ SPI-I2C-bridge driver" > > depends on I2C > > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index > > cb1f437..a90d559 100644 > > --- a/drivers/spi/Makefile > > +++ b/drivers/spi/Makefile > > @@ -101,6 +101,7 @@ spi-thunderx-objs := spi-cavium.o spi-cavium-thunderx.o > > obj-$(CONFIG_SPI_THUNDERX) += spi-thunderx.o > > obj-$(CONFIG_SPI_TOPCLIFF_PCH) += spi-topcliff-pch.o > > obj-$(CONFIG_SPI_TXX9) += spi-txx9.o > > +obj-$(CONFIG_SPI_UNIPHIER) += spi-uniphier.o > > obj-$(CONFIG_SPI_XCOMM) += spi-xcomm.o > > obj-$(CONFIG_SPI_XILINX) += spi-xilinx.o > > obj-$(CONFIG_SPI_XLP) += spi-xlp.o > > diff --git a/drivers/spi/spi-uniphier.c b/drivers/spi/spi-uniphier.c > > new file mode 100644 index 0000000..2e80bb0 > > --- /dev/null > > +++ b/drivers/spi/spi-uniphier.c > > @@ -0,0 +1,539 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +// spi-uniphier.c - Socionext UniPhier SPI controller driver > > +// Copyright 2012 Panasonic Corporation > > +// Copyright 2016-2018 Socionext Inc. > > + > > +#include <asm/unaligned.h> > > +#include <linux/kernel.h> > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/clk.h> > > +#include <linux/interrupt.h> > > +#include <linux/io.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_platform.h> > > +#include <linux/platform_device.h> > > +#include <linux/spi/spi.h> > > + > > +#define SSI_TIMEOUT 2000 /* ms */ > > +#define SSI_MAX_CLK_DIVIDER 254 > > +#define SSI_MIN_CLK_DIVIDER 4 > > + > > +struct uniphier_spi_priv { > > + void __iomem *base; > > + int irq; > > > irq is unnecessary because it is only used in probe(). > > Use a stack variable. I see. I will modify it. > > > > + struct clk *clk; > > + struct spi_master *master; > > + struct completion xfer_done; > > + > > + int error; > > + unsigned int tx_bytes; > > + unsigned int rx_bytes; > > + const u8 *tx_buf; > > + u8 *rx_buf; > > + > > + bool is_save_param; > > + u8 bits_per_word; > > + u16 mode; > > + u32 speed_hz; > > +}; > > + > > +#define SSI_CTL 0x0 > > +#define SSI_CTL_EN BIT(0) > > + > > +#define SSI_CKS 0x4 > > +#define SSI_CKS_CKRAT_MASK GENMASK(7, 0) > > +#define SSI_CKS_CKPHS BIT(14) > > +#define SSI_CKS_CKINIT BIT(13) > > +#define SSI_CKS_CKDLY BIT(12) > > + > > +#define SSI_TXWDS 0x8 > > +#define SSI_TXWDS_WDLEN_MASK GENMASK(13, 8) > > +#define SSI_TXWDS_TDTF_MASK GENMASK(7, 6) > > +#define SSI_TXWDS_DTLEN_MASK GENMASK(5, 0) > > + > > +#define SSI_RXWDS 0xc > > +#define SSI_RXWDS_DTLEN_MASK GENMASK(5, 0) > > + > > +#define SSI_FPS 0x10 > > +#define SSI_FPS_FSPOL BIT(15) > > +#define SSI_FPS_FSTRT BIT(14) > > + > > +#define SSI_SR 0x14 > > +#define SSI_SR_RNE BIT(0) > > + > > +#define SSI_IE 0x18 > > +#define SSI_IE_RCIE BIT(3) > > +#define SSI_IE_RORIE BIT(0) > > + > > +#define SSI_IS 0x1c > > +#define SSI_IS_RXRS BIT(9) > > +#define SSI_IS_RCID BIT(3) > > +#define SSI_IS_RORID BIT(0) > > + > > +#define SSI_IC 0x1c > > +#define SSI_IC_TCIC BIT(4) > > +#define SSI_IC_RCIC BIT(3) > > +#define SSI_IC_RORIC BIT(0) > > + > > +#define SSI_FC 0x20 > > +#define SSI_FC_TXFFL BIT(12) > > +#define SSI_FC_TXFTH_MASK GENMASK(11, 8) > > +#define SSI_FC_RXFFL BIT(4) > > +#define SSI_FC_RXFTH_MASK GENMASK(3, 0) > > + > > +#define SSI_TXDR 0x24 > > +#define SSI_RXDR 0x24 > > + > > +#define SSI_FIFO_DEPTH 8U > > + > > +static inline unsigned int bytes_per_word(unsigned int bits) { > > + return bits <= 8 ? 1 : (bits <= 16 ? 2 : 4); } > > + > > +static inline void uniphier_spi_irq_enable(struct spi_device *spi, > > +u32 mask) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val; > > + > > + val = readl(priv->base + SSI_IE); > > + val |= mask; > > + writel(val, priv->base + SSI_IE); } > > + > > +static inline void uniphier_spi_irq_disable(struct spi_device *spi, > > +u32 mask) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val; > > + > > + val = readl(priv->base + SSI_IE); > > + val &= ~mask; > > + writel(val, priv->base + SSI_IE); } > > + > > +static void uniphier_spi_set_mode(struct spi_device *spi) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val1, val2; > > + > > + /* > > + * clock setting > > + * CKPHS capture timing. 0:rising edge, 1:falling edge > > + * CKINIT clock initial level. 0:low, 1:high > > + * CKDLY clock delay. 0:no delay, 1:delay depending on FSTRT > > + * (FSTRT=0: 1 clock, FSTRT=1: 0.5 clock) > > + * > > + * frame setting > > + * FSPOL frame signal porarity. 0: low, 1: high > > + * FSTRT start frame timing > > + * 0: rising edge of clock, 1: falling edge of clock > > + */ > > + switch (spi->mode & (SPI_CPOL | SPI_CPHA)) { > > + case SPI_MODE_0: > > + /* CKPHS=1, CKINIT=0, CKDLY=1, FSTRT=0 */ > > + val1 = SSI_CKS_CKPHS | SSI_CKS_CKDLY; > > + val2 = 0; > > + break; > > + case SPI_MODE_1: > > + /* CKPHS=0, CKINIT=0, CKDLY=0, FSTRT=1 */ > > + val1 = 0; > > + val2 = SSI_FPS_FSTRT; > > + break; > > + case SPI_MODE_2: > > + /* CKPHS=0, CKINIT=1, CKDLY=1, FSTRT=1 */ > > + val1 = SSI_CKS_CKINIT | SSI_CKS_CKDLY; > > + val2 = SSI_FPS_FSTRT; > > + break; > > + case SPI_MODE_3: > > + /* CKPHS=1, CKINIT=1, CKDLY=0, FSTRT=0 */ > > + val1 = SSI_CKS_CKPHS | SSI_CKS_CKINIT; > > + val2 = 0; > > + break; > > + } > > + > > + if (!(spi->mode & SPI_CS_HIGH)) > > + val2 |= SSI_FPS_FSPOL; > > + > > + writel(val1, priv->base + SSI_CKS); > > + writel(val2, priv->base + SSI_FPS); > > + > > + val1 = 0; > > + if (spi->mode & SPI_LSB_FIRST) > > + val1 |= FIELD_PREP(SSI_TXWDS_TDTF_MASK, 1); > > + writel(val1, priv->base + SSI_TXWDS); > > + writel(val1, priv->base + SSI_RXWDS); } > > + > > +static void uniphier_spi_set_transfer_size(struct spi_device *spi, > > +int size) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val; > > + > > + val = readl(priv->base + SSI_TXWDS); > > + val &= ~(SSI_TXWDS_WDLEN_MASK | SSI_TXWDS_DTLEN_MASK); > > + val |= FIELD_PREP(SSI_TXWDS_WDLEN_MASK, size); > > + val |= FIELD_PREP(SSI_TXWDS_DTLEN_MASK, size); > > + writel(val, priv->base + SSI_TXWDS); > > + > > + val = readl(priv->base + SSI_RXWDS); > > + val &= ~SSI_RXWDS_DTLEN_MASK; > > + val |= FIELD_PREP(SSI_RXWDS_DTLEN_MASK, size); > > + writel(val, priv->base + SSI_RXWDS); } > > + > > +static int uniphier_spi_set_baudrate(struct spi_device *spi, unsigned > > +int speed) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val, ckrat; > > + > > + /* > > + * the supported rates are even numbers from 4 to 254. (4,6,8...254) > > + * round up as we look for equal or less speed > > + */ > > + ckrat = DIV_ROUND_UP(clk_get_rate(priv->clk), speed); > > + ckrat = roundup(ckrat, 2); > > + > > + /* check if requested speed is too small */ > > + if (ckrat > SSI_MAX_CLK_DIVIDER) > > + return -EINVAL; > > + > > + if (ckrat < SSI_MIN_CLK_DIVIDER) > > + ckrat = SSI_MIN_CLK_DIVIDER; > > + > > + val = readl(priv->base + SSI_CKS); > > + val &= ~SSI_CKS_CKRAT_MASK; > > + val |= ckrat & SSI_CKS_CKRAT_MASK; > > + writel(val, priv->base + SSI_CKS); > > + > > + return 0; > > +} > > + > > +static int uniphier_spi_setup_transfer(struct spi_device *spi, > > + struct spi_transfer *t) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val; > > + int ret; > > + > > + priv->error = 0; > > + priv->tx_buf = t->tx_buf; > > + priv->rx_buf = t->rx_buf; > > + priv->tx_bytes = priv->rx_bytes = t->len; > > + > > + if (!priv->is_save_param || priv->mode != spi->mode) { > > + uniphier_spi_set_mode(spi); > > + priv->mode = spi->mode; > > + } > > + > > + if (!priv->is_save_param || priv->bits_per_word != t->bits_per_word) { > > + uniphier_spi_set_transfer_size(spi, t->bits_per_word); > > + priv->bits_per_word = t->bits_per_word; > > + } > > + > > + if (!priv->is_save_param || priv->speed_hz != t->speed_hz) { > > + ret = uniphier_spi_set_baudrate(spi, t->speed_hz); > > + if (ret) > > + return ret; > > + priv->speed_hz = t->speed_hz; > > + } > > + > > + if (!priv->is_save_param) > > + priv->is_save_param = true; > > + > > + /* reset FIFOs */ > > + val = SSI_FC_TXFFL | SSI_FC_RXFFL; > > + writel(val, priv->base + SSI_FC); > > + > > + return 0; > > +} > > + > > +static void uniphier_spi_send(struct uniphier_spi_priv *priv) { > > + int wsize; > > + u32 val = 0; > > + > > + wsize = min(bytes_per_word(priv->bits_per_word), priv->tx_bytes); > > + priv->tx_bytes -= wsize; > > + > > + if (priv->tx_buf) { > > + switch (wsize) { > > + case 1: > > + val = *priv->tx_buf; > > + break; > > + case 2: > > + val = get_unaligned_le16(priv->tx_buf); > > + break; > > + case 4: > > + val = get_unaligned_le32(priv->tx_buf); > > + break; > > + } > > + > > + priv->tx_buf += wsize; > > + } > > + > > + writel(val, priv->base + SSI_TXDR); } > > + > > +static void uniphier_spi_recv(struct uniphier_spi_priv *priv) { > > + int rsize; > > + u32 val; > > + > > + rsize = min(bytes_per_word(priv->bits_per_word), priv->rx_bytes); > > + priv->rx_bytes -= rsize; > > + > > + val = readl(priv->base + SSI_RXDR); > > + > > + if (priv->rx_buf) { > > + switch (rsize) { > > + case 1: > > + *priv->rx_buf = (u8)val; > > Is this cast necessary? I added it explicitly, but I will modify it. > > > + break; > > + case 2: > > + put_unaligned_le16(val, priv->rx_buf); > > + break; > > + case 4: > > + put_unaligned_le32(val, priv->rx_buf); > > + break; > > + } > > + > > + priv->rx_buf += rsize; > > + } > > +} > > + > > +static void uniphier_spi_fill_tx_fifo(struct uniphier_spi_priv *priv) > > +{ > > + unsigned int tx_count; > > + u32 val; > > + > > + tx_count = DIV_ROUND_UP(priv->tx_bytes, > > + bytes_per_word(priv->bits_per_word)); > > + tx_count = min(tx_count, SSI_FIFO_DEPTH); > > + > > + /* set fifo threthold */ > > > Typo. threshold I will modify it. > > > > + val = readl(priv->base + SSI_FC); > > + val &= ~(SSI_FC_TXFTH_MASK | SSI_FC_RXFTH_MASK); > > + val |= FIELD_PREP(SSI_FC_TXFTH_MASK, tx_count); > > + val |= FIELD_PREP(SSI_FC_RXFTH_MASK, tx_count); > > + writel(val, priv->base + SSI_FC); > > + > > + while (tx_count--) > > + uniphier_spi_send(priv); } > > + > > +static void uniphier_spi_set_cs(struct spi_device *spi, bool enable) > > +{ > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); > > + u32 val; > > + > > + val = readl(priv->base + SSI_FPS); > > + > > + if (enable) > > + val |= SSI_FPS_FSPOL; > > + else > > + val &= ~SSI_FPS_FSPOL; > > + > > + writel(val, priv->base + SSI_FPS); } > > + > > +static int uniphier_spi_transfer_one(struct spi_master *master, > > + struct spi_device *spi, > > + struct spi_transfer *t) { > > + struct uniphier_spi_priv *priv = spi_master_get_devdata(master); > > + int status; > > + > > + status = uniphier_spi_setup_transfer(spi, t); > > + if (status < 0) > > + return status; > > + > > + reinit_completion(&priv->xfer_done); > > + > > + uniphier_spi_fill_tx_fifo(priv); > > + > > + uniphier_spi_irq_enable(spi, SSI_IE_RCIE | SSI_IE_RORIE); > > + > > + status = wait_for_completion_timeout(&priv->xfer_done, > > + > > + msecs_to_jiffies(SSI_TIMEOUT)); > > + > > + uniphier_spi_irq_disable(spi, SSI_IE_RCIE | SSI_IE_RORIE); > > + > > + if (status < 0) > > + return status; > > + > > + return priv->error; > > +} > > + > > +static int uniphier_spi_prepare_transfer_hardware(struct spi_master > > +*master) { > > + struct uniphier_spi_priv *priv = > > +spi_master_get_devdata(master); > > + > > + writel(SSI_CTL_EN, priv->base + SSI_CTL); > > + > > + return 0; > > +} > > + > > +static int uniphier_spi_unprepare_transfer_hardware(struct spi_master > > +*master) { > > + struct uniphier_spi_priv *priv = > > +spi_master_get_devdata(master); > > + > > + writel(0, priv->base + SSI_CTL); > > + > > + return 0; > > +} > > + > > +static irqreturn_t uniphier_spi_handler(int irq, void *dev_id) { > > + struct uniphier_spi_priv *priv = dev_id; > > + u32 val, stat; > > + > > + stat = readl(priv->base + SSI_IS); > > + val = SSI_IC_TCIC | SSI_IC_RCIC | SSI_IC_RORIC; > > + writel(val, priv->base + SSI_IC); > > + > > + /* rx fifo overrun */ > > + if (stat & SSI_IS_RORID) { > > + priv->error = -EIO; > > + goto done; > > + } > > + > > + /* rx complete */ > > + if ((stat & SSI_IS_RCID) && (stat & SSI_IS_RXRS)) { > > + while ((readl(priv->base + SSI_SR) & SSI_SR_RNE) && > > + (priv->rx_bytes - priv->tx_bytes) > 0) > > + uniphier_spi_recv(priv); > > + > > + if ((readl(priv->base + SSI_SR) & SSI_SR_RNE) || > > + (priv->rx_bytes != priv->tx_bytes)) { > > + priv->error = -EIO; > > + goto done; > > + } else if (priv->rx_bytes == 0) > > + goto done; > > + > > + /* next tx transfer */ > > + uniphier_spi_fill_tx_fifo(priv); > > + > > + return IRQ_HANDLED; > > + } > > + > > + return IRQ_NONE; > > + > > +done: > > + complete(&priv->xfer_done); > > + return IRQ_HANDLED; > > +} > > + > > +static int uniphier_spi_probe(struct platform_device *pdev) { > > + struct uniphier_spi_priv *priv; > > + struct spi_master *master; > > + struct resource *res; > > + unsigned long clksrc; > > + int ret; > > + > > + master = spi_alloc_master(&pdev->dev, sizeof(*priv)); > > + if (!master) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, master); > > + > > + priv = spi_master_get_devdata(master); > > + priv->master = master; > > + priv->is_save_param = false; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->base = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(priv->base)) { > > + ret = PTR_ERR(priv->base); > > + goto out_master_put; > > + } > > + > > + priv->clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(priv->clk)) { > > + dev_err(&pdev->dev, "failed to get clock\n"); > > + ret = PTR_ERR(priv->clk); > > + goto out_master_put; > > + } > > + > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) > > + goto out_master_put; > > + > > + priv->irq = platform_get_irq(pdev, 0); > > + if (priv->irq < 0) { > > + dev_err(&pdev->dev, "failed to get IRQ\n"); > > + ret = -ENXIO; > > + goto out_disable_clk; > > + } > > + > > + ret = devm_request_irq(&pdev->dev, priv->irq, uniphier_spi_handler, > > + 0, "uniphier-spi", priv); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to request IRQ\n"); > > + goto out_disable_clk; > > + } > > + > > + init_completion(&priv->xfer_done); > > + > > + clksrc = clk_get_rate(priv->clk); > > > How about 'clk_rate' ? OK. I will replace clksrc to clk_rate. > > > > + master->max_speed_hz = DIV_ROUND_UP(clksrc, SSI_MIN_CLK_DIVIDER); > > + master->min_speed_hz = DIV_ROUND_UP(clksrc, SSI_MAX_CLK_DIVIDER); > > + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST; > > + master->dev.of_node = pdev->dev.of_node; > > + master->bus_num = pdev->id; > > + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32); > > + > > + master->set_cs = uniphier_spi_set_cs; > > + master->transfer_one = uniphier_spi_transfer_one; > > + master->prepare_transfer_hardware > > + = uniphier_spi_prepare_transfer_hardware; > > + master->unprepare_transfer_hardware > > + = uniphier_spi_unprepare_transfer_hardware; > > + master->num_chipselect = 1; > > + > > + ret = devm_spi_register_master(&pdev->dev, master); > > + if (ret) > > + goto out_disable_clk; > > + > > + return ret; > > > I think > > return 0; > > will be clearer. I think so. I will modify it. Thank you. ----------------- Best Regards, Keiji Hayashibara -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
