* [PATCH 0/2] add SPI controller driver for UniPhier SoCs @ 2018-07-19 6:51 Keiji Hayashibara 2018-07-19 6:51 ` [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller Keiji Hayashibara 2018-07-19 6:51 ` [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara 0 siblings, 2 replies; 8+ messages in thread From: Keiji Hayashibara @ 2018-07-19 6:51 UTC (permalink / raw) To: broonie, robh+dt, mark.rutland, yamada.masahiro, linux-spi, linux-arm-kernel, devicetree Cc: masami.hiramatsu, jaswinder.singh, linux-kernel, hayashibara.keiji This series adds support for SPI controller driver implemented on UniPhier SoCs. Keiji Hayashibara (1): spi: add SPI controller driver for UniPhier SoC Kunihiko Hayashi (1): dt-bindings: spi: add DT bindings for UniPhier SPI controller .../devicetree/bindings/spi/spi-uniphier.txt | 26 + drivers/spi/Kconfig | 7 + drivers/spi/Makefile | 1 + drivers/spi/spi-uniphier.c | 532 +++++++++++++++++++++ 4 files changed, 566 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-uniphier.txt create mode 100644 drivers/spi/spi-uniphier.c -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller 2018-07-19 6:51 [PATCH 0/2] add SPI controller driver for UniPhier SoCs Keiji Hayashibara @ 2018-07-19 6:51 ` Keiji Hayashibara 2018-07-25 19:17 ` Rob Herring 2018-07-19 6:51 ` [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara 1 sibling, 1 reply; 8+ messages in thread From: Keiji Hayashibara @ 2018-07-19 6:51 UTC (permalink / raw) To: broonie, robh+dt, mark.rutland, yamada.masahiro, linux-spi, linux-arm-kernel, devicetree Cc: masami.hiramatsu, jaswinder.singh, linux-kernel, hayashibara.keiji, Kunihiko Hayashi From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Add DT bindings for SPI controller implemented in UniPhier SoCs. Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com> --- .../devicetree/bindings/spi/spi-uniphier.txt | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-uniphier.txt diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt b/Documentation/devicetree/bindings/spi/spi-uniphier.txt new file mode 100644 index 0000000..9c8c4a1 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt @@ -0,0 +1,26 @@ +Socionext UniPhier SPI controller driver + +UniPhier SoCs have two types of SPI controllers; SCSSI supports a +single channel, and MCSSI supports multiple channels. +Both of them support the SPI master mode only. + +Required properties: + - compatible: should be + "socionext,uniphier-scssi" - for SCSSI device + "socionext,uniphier-mcssi" - for MCSSI device + - reg: address and length of the spi master registers + - #address-cells: must be <1>, see spi-bus.txt + - #size-cells: must be <0>, see spi-bus.txt + - clocks: A phandle to the clock for the device. + - resets: A phandle to the reset control for the device. + +Example: + +spi0: spi@54006000 { + compatible = "socionext,uniphier-scssi"; + reg = <0x54006000 0x100>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&peri_clk 11>; + resets = <&peri_rst 11>; +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller 2018-07-19 6:51 ` [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller Keiji Hayashibara @ 2018-07-25 19:17 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2018-07-25 19:17 UTC (permalink / raw) To: Keiji Hayashibara Cc: broonie, mark.rutland, yamada.masahiro, linux-spi, linux-arm-kernel, devicetree, masami.hiramatsu, jaswinder.singh, linux-kernel, Kunihiko Hayashi On Thu, Jul 19, 2018 at 03:51:56PM +0900, Keiji Hayashibara wrote: > From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > Add DT bindings for SPI controller implemented in UniPhier SoCs. > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com> > --- > .../devicetree/bindings/spi/spi-uniphier.txt | 26 ++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/spi-uniphier.txt Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC 2018-07-19 6:51 [PATCH 0/2] add SPI controller driver for UniPhier SoCs Keiji Hayashibara 2018-07-19 6:51 ` [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller Keiji Hayashibara @ 2018-07-19 6:51 ` Keiji Hayashibara 2018-07-19 16:51 ` Mark Brown 2018-07-19 19:45 ` Trent Piepho 1 sibling, 2 replies; 8+ messages in thread From: Keiji Hayashibara @ 2018-07-19 6:51 UTC (permalink / raw) To: broonie, robh+dt, mark.rutland, yamada.masahiro, linux-spi, linux-arm-kernel, devicetree Cc: masami.hiramatsu, jaswinder.singh, linux-kernel, hayashibara.keiji, Kunihiko Hayashi 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. Currently 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@socionext.com> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com> --- drivers/spi/Kconfig | 7 + drivers/spi/Makefile | 1 + drivers/spi/spi-uniphier.c | 532 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 540 insertions(+) create mode 100644 drivers/spi/spi-uniphier.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index ad5d68e..a3cb0b4 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -688,6 +688,13 @@ 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 driver supports the SPI controller on Socionext + UniPhier SoCs. + 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..92718ee --- /dev/null +++ b/drivers/spi/spi-uniphier.c @@ -0,0 +1,532 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * spi-uniphier.c - Socionext UniPhier SPI controller driver + * + * Copyright 2012 Panasonic Corporation + * Copyright 2016-2018 Socionext Inc. + */ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.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; + 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; + + 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 8 + +#define BYTES_PER_WORD(x) \ +({ \ + int __x; \ + __x = (x <= 8) ? 1 : \ + (x <= 16) ? 2 : 4; \ + __x; \ +}) + +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_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->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->speed_hz != t->speed_hz) { + ret = uniphier_spi_set_baudrate(spi, t->speed_hz); + if (ret) + return ret; + priv->speed_hz = t->speed_hz; + } + + /* 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 i, loop; + u32 val = 0; + + loop = BYTES_PER_WORD(priv->bits_per_word); + if (priv->tx_bytes < loop) + loop = priv->tx_bytes; + + priv->tx_bytes -= loop; + + if (priv->tx_buf) + for (i = 0; i < loop; i++) { + val |= (*(const u8 *)priv->tx_buf) + << (BITS_PER_BYTE * i); + (const u8 *)priv->tx_buf++; + } + + writel(val, priv->base + SSI_TXDR); +} + +static void uniphier_spi_recv(struct uniphier_spi_priv *priv) +{ + int i, loop; + u32 val; + + loop = BYTES_PER_WORD(priv->bits_per_word); + if (priv->rx_bytes < loop) + loop = priv->rx_bytes; + + priv->rx_bytes -= loop; + + val = readl(priv->base + SSI_RXDR); + + if (priv->rx_buf) + for (i = 0; i < loop; i++) { + val = val >> (BITS_PER_BYTE * i); + *(u8 *)priv->rx_buf = val & GENMASK(7, 0); + (u8 *)priv->rx_buf++; + } +} + +static void uniphier_spi_fill_tx_fifo(struct uniphier_spi_priv *priv) +{ + unsigned int tx_count; + int bytes_per_word = BYTES_PER_WORD(priv->bits_per_word); + u32 val; + + tx_count = priv->tx_bytes / bytes_per_word; + if (tx_count > SSI_FIFO_DEPTH) + tx_count = SSI_FIFO_DEPTH; + + /* set fifo threthold */ + 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 int uniphier_spi_setup(struct spi_device *spi) +{ + struct uniphier_spi_priv *priv = spi_master_get_devdata(spi->master); + u32 val1, val2; + + if (priv->mode == spi->mode) + return 0; + + priv->mode = spi->mode; + + /* 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); + + 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; + +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->bits_per_word = 0; + priv->mode = 0; + priv->speed_hz = 0; + + 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); + + 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->setup = uniphier_spi_setup; + 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; + +out_disable_clk: + clk_disable_unprepare(priv->clk); + +out_master_put: + spi_master_put(master); + return ret; +} + +static int uniphier_spi_remove(struct platform_device *pdev) +{ + struct uniphier_spi_priv *priv = platform_get_drvdata(pdev); + + clk_disable_unprepare(priv->clk); + + return 0; +} + +static const struct of_device_id uniphier_spi_match[] = { + { .compatible = "socionext,uniphier-scssi", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, uniphier_spi_match); + +static struct platform_driver uniphier_spi_driver = { + .probe = uniphier_spi_probe, + .remove = uniphier_spi_remove, + .driver = { + .name = "uniphier-spi", + .of_match_table = uniphier_spi_match, + }, +}; +module_platform_driver(uniphier_spi_driver); + +MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>"); +MODULE_AUTHOR("Keiji Hayashibara <hayashibara.keiji@socionext.com>"); +MODULE_DESCRIPTION("Socionext UniPhier SPI controller driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC 2018-07-19 6:51 ` [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara @ 2018-07-19 16:51 ` Mark Brown 2018-07-20 0:33 ` Keiji Hayashibara 2018-07-19 19:45 ` Trent Piepho 1 sibling, 1 reply; 8+ messages in thread From: Mark Brown @ 2018-07-19 16:51 UTC (permalink / raw) To: Keiji Hayashibara Cc: robh+dt, mark.rutland, yamada.masahiro, linux-spi, linux-arm-kernel, devicetree, masami.hiramatsu, jaswinder.singh, linux-kernel, Kunihiko Hayashi [-- Attachment #1: Type: text/plain, Size: 1980 bytes --] On Thu, Jul 19, 2018 at 03:51:57PM +0900, Keiji Hayashibara wrote: This all looks good, just a small number of fairly minor things - mostly style points. > @@ -0,0 +1,532 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * spi-uniphier.c - Socionext UniPhier SPI controller driver > + * > + * Copyright 2012 Panasonic Corporation > + * Copyright 2016-2018 Socionext Inc. > + */ Please make the entire comment a C++ one, it makes things look a bit more joined up/intentional. > +#define BYTES_PER_WORD(x) \ > +({ \ > + int __x; \ > + __x = (x <= 8) ? 1 : \ > + (x <= 16) ? 2 : 4; \ > + __x; \ > +}) Could this be replaced with an inline function? The usage seems fine but it's a bit big for a macro. The end result should be similar. > +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)) { > + } > + > + return IRQ_HANDLED; > + > +done: > + complete(&priv->xfer_done); > + return IRQ_HANDLED; This will unconditionally report IRQ_HANDLED even if none of the flags were set by the hardware which will cause problems if something goes wrong - the interrupt will continually be serviced and the interrupt framework won't be able to mitigate or provide diagnostics. It's better to return IRQ_NONE if nothing is detected from the hardware. > +static const struct of_device_id uniphier_spi_match[] = { > + { .compatible = "socionext,uniphier-scssi", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, uniphier_spi_match); The binding document also listed socionext,uniphier-mcssi as a compatible but this driver doesn't match that. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC 2018-07-19 16:51 ` Mark Brown @ 2018-07-20 0:33 ` Keiji Hayashibara 0 siblings, 0 replies; 8+ messages in thread From: Keiji Hayashibara @ 2018-07-20 0:33 UTC (permalink / raw) To: 'Mark Brown' Cc: robh+dt, mark.rutland, Yamada, Masahiro/山田 真弘, linux-spi, linux-arm-kernel, devicetree, masami.hiramatsu, jaswinder.singh, linux-kernel, Hayashi, Kunihiko/林 邦彦 Hi Mark, > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Friday, July 20, 2018 1:52 AM > Subject: Re: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC > > On Thu, Jul 19, 2018 at 03:51:57PM +0900, Keiji Hayashibara wrote: > > This all looks good, just a small number of fairly minor things - mostly style points. > > > @@ -0,0 +1,532 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * spi-uniphier.c - Socionext UniPhier SPI controller driver > > + * > > + * Copyright 2012 Panasonic Corporation > > + * Copyright 2016-2018 Socionext Inc. > > + */ > > Please make the entire comment a C++ one, it makes things look a bit more joined up/intentional. OK. I will modify to C++ style. > > +#define BYTES_PER_WORD(x) \ > > +({ \ > > + int __x; \ > > + __x = (x <= 8) ? 1 : \ > > + (x <= 16) ? 2 : 4; \ > > + __x; \ > > +}) > > Could this be replaced with an inline function? The usage seems fine but it's a bit big for a macro. The end > result should be similar. OK. I will replace with an inline function. > > > +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)) { > > > + } > > + > > + return IRQ_HANDLED; > > + > > +done: > > + complete(&priv->xfer_done); > > + return IRQ_HANDLED; > > This will unconditionally report IRQ_HANDLED even if none of the flags were set by the hardware which will cause > problems if something goes wrong - the interrupt will continually be serviced and the interrupt framework won't > be able to mitigate or provide diagnostics. It's better to return IRQ_NONE if nothing is detected from the hardware. I agree. I will modify it. > > +static const struct of_device_id uniphier_spi_match[] = { > > + { .compatible = "socionext,uniphier-scssi", }, > > + { /* sentinel */ }, > > +}; > > +MODULE_DEVICE_TABLE(of, uniphier_spi_match); > > The binding document also listed socionext,uniphier-mcssi as a compatible but this driver doesn't match that. This driver doesn't support uniphier-mcssi, and support uniphier-scssi only. I described in the commit comment, but I will also describe it in the binding document. Thank you. ----------------- Best Regards, Keiji Hayashibara ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC 2018-07-19 6:51 ` [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara 2018-07-19 16:51 ` Mark Brown @ 2018-07-19 19:45 ` Trent Piepho 2018-07-20 2:20 ` Keiji Hayashibara 1 sibling, 1 reply; 8+ messages in thread From: Trent Piepho @ 2018-07-19 19:45 UTC (permalink / raw) To: robh+dt, devicetree, hayashibara.keiji, broonie, mark.rutland, linux-spi, yamada.masahiro, linux-arm-kernel Cc: linux-kernel, masami.hiramatsu, jaswinder.singh, hayashi.kunihiko On Thu, 2018-07-19 at 15:51 +0900, Keiji Hayashibara wrote: > > +config SPI_UNIPHIER > + tristate "Socionext UniPhier SPI Controller" > + depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF > + help > + This driver supports the SPI controller on Socionext > + UniPhier SoCs. Perhaps add the bit that this is for the SCSSI and not MCSSI here? > > + > +#define BYTES_PER_WORD(x) \ > +({ \ > + int __x; \ > + __x = (x <= 8) ? 1 : \ > + (x <= 16) ? 2 : 4; \ > + __x; \ > +}) Or: static inline 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_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->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->speed_hz != t->speed_hz) { > + ret = uniphier_spi_set_baudrate(spi, t->speed_hz); > + if (ret) > + return ret; > + priv->speed_hz = t->speed_hz; > + } > + > + /* 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 i, loop; > + u32 val = 0; > + > + loop = BYTES_PER_WORD(priv->bits_per_word); > + if (priv->tx_bytes < loop) > + loop = priv->tx_bytes; > + > + priv->tx_bytes -= loop; > + > + if (priv->tx_buf) > + for (i = 0; i < loop; i++) { > + val |= (*(const u8 *)priv->tx_buf) > + << (BITS_PER_BYTE * i); priv->tx_buf is already a const u8*, no need to cast it. Also in recv, no need to cast the pointer. It'll just hide errors if someone changes the type of the field. > + (const u8 *)priv->tx_buf++; > + } > + > + writel(val, priv->base + SSI_TXDR); > +} The loop to read the data will likely be somewhat slow. It might be faster to use: val = get_unaligned_le32(priv->tx_buf); To support different sizes a switch can be used: switch (MIN(BYTES_PER_WORD(priv->bits_per_word), priv->tx_bytes)) { 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; } However, I don't think either the existing code or this code is correctly handling word sizes that are not an even number of bytes. I think it needs to left shift the data, but of course it also depends on what the uniphier hardware expected in the TXDR register. > +static void uniphier_spi_recv(struct uniphier_spi_priv *priv) > +{ > + int i, loop; > + u32 val; > + > + loop = BYTES_PER_WORD(priv->bits_per_word); > + if (priv->rx_bytes < loop) > + loop = priv->rx_bytes; > + > + priv->rx_bytes -= loop; > + > + val = readl(priv->base + SSI_RXDR); > + > + if (priv->rx_buf) > + for (i = 0; i < loop; i++) { > + val = val >> (BITS_PER_BYTE * i); > + *(u8 *)priv->rx_buf = val & GENMASK(7, 0); > + (u8 *)priv->rx_buf++; > + } > +}+static void uniphier_spi_fill_tx_fifo(struct uniphier_spi_priv *priv) > +{ > + unsigned int tx_count; > + int bytes_per_word = BYTES_PER_WORD(priv->bits_per_word); > + u32 val; > + > + tx_count = priv->tx_bytes / bytes_per_word; > + if (tx_count > SSI_FIFO_DEPTH) > + tx_count = SSI_FIFO_DEPTH; > + > + /* set fifo threthold */ > + 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); > +} If you have 24 bits per word, 3 words, that's 9 bytes. BYTES_PER_WORD(24) is 4. tx_count = 9/4 = 2. Looks like your tx_count rounds incorrectly, as it will only send 8 of the 9 bytes. > +static int uniphier_spi_setup(struct spi_device *spi) > +{ > + > + 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); Did you see this in the spi docs? Unless each SPI slave has its own configuration registers, don't change them right away ... otherwise drivers could corrupt I/O that's in progress for other SPI devices. ** BUG ALERT: for some reason the first version of ** many spi_master drivers seems to get this wrong. ** When you code setup(), ASSUME that the controller ** is actively processing transfers for another device. You have one chipselect, so maybe this is ok. Until you want to support more than one chipselect. With gpio lines as chip selects, there's really no reason any spi master can't support multiple slaves. ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC 2018-07-19 19:45 ` Trent Piepho @ 2018-07-20 2:20 ` Keiji Hayashibara 0 siblings, 0 replies; 8+ messages in thread From: Keiji Hayashibara @ 2018-07-20 2:20 UTC (permalink / raw) To: 'Trent Piepho', robh+dt, devicetree, broonie, mark.rutland, linux-spi, Yamada, Masahiro/山田 真弘, linux-arm-kernel Cc: linux-kernel, masami.hiramatsu, jaswinder.singh, Hayashi, Kunihiko/林 邦彦 Hi Trent, > -----Original Message----- > From: Trent Piepho [mailto:tpiepho@impinj.com] > Sent: Friday, July 20, 2018 4:46 AM > Subject: Re: [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC > > On Thu, 2018-07-19 at 15:51 +0900, Keiji Hayashibara wrote: > > > > +config SPI_UNIPHIER > > + tristate "Socionext UniPhier SPI Controller" > > + depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF > > + help > > + This driver supports the SPI controller on Socionext > > + UniPhier SoCs. > > Perhaps add the bit that this is for the SCSSI and not MCSSI here? OK. I will add it. > > > > + > > +#define BYTES_PER_WORD(x) \ > > +({ \ > > + int __x; \ > > + __x = (x <= 8) ? 1 : \ > > + (x <= 16) ? 2 : 4; \ > > + __x; \ > > +}) > > Or: > > static inline bytes_per_word(unsigned int bits) { > return bits <= 8 ? 1 : (bits <= 16 ? 2 : 4); } I will modify. > > > + > > +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_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->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->speed_hz != t->speed_hz) { > > + ret = uniphier_spi_set_baudrate(spi, t->speed_hz); > > + if (ret) > > + return ret; > > + priv->speed_hz = t->speed_hz; > > + } > > + > > + /* 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 i, loop; > > + u32 val = 0; > > + > > + loop = BYTES_PER_WORD(priv->bits_per_word); > > + if (priv->tx_bytes < loop) > > + loop = priv->tx_bytes; > > + > > + priv->tx_bytes -= loop; > > + > > + if (priv->tx_buf) > > + for (i = 0; i < loop; i++) { > > + val |= (*(const u8 *)priv->tx_buf) > > + << (BITS_PER_BYTE * i); > > priv->tx_buf is already a const u8*, no need to cast it. Also in recv, > no need to cast the pointer. It'll just hide errors if someone changes the type of the field. I agree. > > + (const u8 *)priv->tx_buf++; > > + } > > + > > + writel(val, priv->base + SSI_TXDR); > > +} > > The loop to read the data will likely be somewhat slow. It might be faster to use: > > val = get_unaligned_le32(priv->tx_buf); > > To support different sizes a switch can be used: > > switch (MIN(BYTES_PER_WORD(priv->bits_per_word), priv->tx_bytes)) { > 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; > } > > However, I don't think either the existing code or this code is correctly handling word sizes that are not an > even number of bytes. I think it needs to left shift the data, but of course it also depends on what the uniphier > hardware expected in the TXDR register. I agree. This code is simple for no loop. I will modify to this code. > > > +static void uniphier_spi_recv(struct uniphier_spi_priv *priv) { > > + int i, loop; > > + u32 val; > > + > > + loop = BYTES_PER_WORD(priv->bits_per_word); > > + if (priv->rx_bytes < loop) > > + loop = priv->rx_bytes; > > + > > + priv->rx_bytes -= loop; > > + > > + val = readl(priv->base + SSI_RXDR); > > + > > + if (priv->rx_buf) > > + for (i = 0; i < loop; i++) { > > + val = val >> (BITS_PER_BYTE * i); > > + *(u8 *)priv->rx_buf = val & GENMASK(7, 0); > > + (u8 *)priv->rx_buf++; > > + } > > > > > +}+static void uniphier_spi_fill_tx_fifo(struct uniphier_spi_priv > > +*priv) { > > + unsigned int tx_count; > > + int bytes_per_word = BYTES_PER_WORD(priv->bits_per_word); > > + u32 val; > > + > > + tx_count = priv->tx_bytes / bytes_per_word; > > + if (tx_count > SSI_FIFO_DEPTH) > > + tx_count = SSI_FIFO_DEPTH; > > + > > + /* set fifo threthold */ > > + 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); > > +} > > If you have 24 bits per word, 3 words, that's 9 bytes. > BYTES_PER_WORD(24) is 4. tx_count = 9/4 = 2. Looks like your tx_count rounds incorrectly, as it will only send > 8 of the 9 bytes. Oh, I will fix this bug. > > > +static int uniphier_spi_setup(struct spi_device *spi) { > > > + > > + 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); > > Did you see this in the spi docs? > > Unless each SPI slave has its own configuration registers, don't > change them right away ... otherwise drivers could corrupt I/O > that's in progress for other SPI devices. > > ** BUG ALERT: for some reason the first version of > ** many spi_master drivers seems to get this wrong. > ** When you code setup(), ASSUME that the controller > ** is actively processing transfers for another device. > > You have one chipselect, so maybe this is ok. Until you want to support more than one chipselect. > > With gpio lines as chip selects, there's really no reason any spi master can't support multiple slaves. I agree. I will re-think about this. Thank you for your advice. ----------------- Best Regards, Keiji Hayashibara ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-25 19:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-19 6:51 [PATCH 0/2] add SPI controller driver for UniPhier SoCs Keiji Hayashibara 2018-07-19 6:51 ` [PATCH 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller Keiji Hayashibara 2018-07-25 19:17 ` Rob Herring 2018-07-19 6:51 ` [PATCH 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara 2018-07-19 16:51 ` Mark Brown 2018-07-20 0:33 ` Keiji Hayashibara 2018-07-19 19:45 ` Trent Piepho 2018-07-20 2:20 ` Keiji Hayashibara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).