From: "Keiji Hayashibara" <hayashibara.keiji@socionext.com>
To: "'Masahiro Yamada'" <yamada.masahiro@socionext.com>
Cc: "Mark Brown" <broonie@kernel.org>,
"Rob Herring" <robh+dt@kernel.org>,
"Mark Rutland" <mark.rutland@arm.com>,
linux-spi@vger.kernel.org,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
DTML <devicetree@vger.kernel.org>,
"Masami Hiramatsu" <masami.hiramatsu@linaro.org>,
"Jassi Brar" <jaswinder.singh@linaro.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Hayashi, Kunihiko/林 邦彦" <hayashi.kunihiko@socionext.com>
Subject: RE: [PATCH v2 2/2] spi: add SPI controller driver for UniPhier SoC
Date: Thu, 26 Jul 2018 18:49:42 +0900 [thread overview]
Message-ID: <001c01d424c5$fa4a7d50$eedf77f0$@socionext.com> (raw)
In-Reply-To: <CAK7LNAS+T17JPb72b2aaE_g0D8kTqUZngcn-0yxyHcP8NusGYg@mail.gmail.com>
Hello Yamada-san,
> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> Sent: Thursday, July 26, 2018 6:13 PM
> To: Hayashibara, Keiji/林原 啓二 <hayashibara.keiji@socionext.com>
> 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@socionext.com>:
> > 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@socionext.com>
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > ---
> > 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
WARNING: multiple messages have this Message-ID (diff)
From: hayashibara.keiji@socionext.com (Keiji Hayashibara)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/2] spi: add SPI controller driver for UniPhier SoC
Date: Thu, 26 Jul 2018 18:49:42 +0900 [thread overview]
Message-ID: <001c01d424c5$fa4a7d50$eedf77f0$@socionext.com> (raw)
In-Reply-To: <CAK7LNAS+T17JPb72b2aaE_g0D8kTqUZngcn-0yxyHcP8NusGYg@mail.gmail.com>
Hello Yamada-san,
> From: Masahiro Yamada [mailto:yamada.masahiro at socionext.com]
> Sent: Thursday, July 26, 2018 6:13 PM
> To: Hayashibara, Keiji/?? ?? <hayashibara.keiji@socionext.com>
> 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@socionext.com>:
> > 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@socionext.com>
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > ---
> > 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
next prev parent reply other threads:[~2018-07-26 9:49 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-26 7:09 [PATCH v2 0/2] add SPI controller driver for UniPhier SoCs Keiji Hayashibara
2018-07-26 7:09 ` Keiji Hayashibara
2018-07-26 7:09 ` [PATCH v2 1/2] dt-bindings: spi: add DT bindings for UniPhier SPI controller Keiji Hayashibara
2018-07-26 7:09 ` Keiji Hayashibara
2018-07-30 21:47 ` Rob Herring
2018-07-30 21:47 ` Rob Herring
2018-07-30 23:44 ` Keiji Hayashibara
2018-07-30 23:44 ` Keiji Hayashibara
2018-07-26 7:09 ` [PATCH v2 2/2] spi: add SPI controller driver for UniPhier SoC Keiji Hayashibara
2018-07-26 7:09 ` Keiji Hayashibara
2018-07-26 8:46 ` Andy Shevchenko
2018-07-26 8:46 ` Andy Shevchenko
2018-07-26 9:38 ` Keiji Hayashibara
2018-07-26 9:38 ` Keiji Hayashibara
2018-07-26 10:57 ` Radu Pirea
2018-07-26 10:57 ` Radu Pirea
2018-07-30 1:15 ` Keiji Hayashibara
2018-07-30 1:15 ` Keiji Hayashibara
2018-07-30 1:15 ` Keiji Hayashibara
2018-07-26 13:44 ` Andy Shevchenko
2018-07-26 13:44 ` Andy Shevchenko
2018-07-30 1:46 ` Keiji Hayashibara
2018-07-30 1:46 ` Keiji Hayashibara
2018-07-26 17:01 ` Trent Piepho
2018-07-26 17:01 ` Trent Piepho
2018-07-30 5:30 ` Keiji Hayashibara
2018-07-30 5:30 ` Keiji Hayashibara
2018-07-30 5:30 ` Keiji Hayashibara
2018-07-30 8:39 ` Andy Shevchenko
2018-07-30 8:39 ` Andy Shevchenko
2018-07-26 9:12 ` Masahiro Yamada
2018-07-26 9:12 ` Masahiro Yamada
2018-07-26 9:49 ` Keiji Hayashibara [this message]
2018-07-26 9:49 ` Keiji Hayashibara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001c01d424c5$fa4a7d50$eedf77f0$@socionext.com' \
--to=hayashibara.keiji@socionext.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=hayashi.kunihiko@socionext.com \
--cc=jaswinder.singh@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=masami.hiramatsu@linaro.org \
--cc=robh+dt@kernel.org \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.