[PATCH resend 0/2] spi: add driver for ar934x spi controller

[PATCH resend 0/2] spi: add driver for ar934x spi controller

[Chuanhong Guo]

This controller is a superset of the already supported qca,ar7100-spi.
Besides the bit-bang mode in spi-ath79.c, this new controller added
a new "shift register" mode, allowing faster spi operations.
This mode doesn't need all the bit-bang code in spi-ath79.c and needs
a different clock setup, so I decided to write a new driver for it
instead of extending current spi-ath79 driver.

Resend the patchset because I forgot to CC documentation maintainers
Chuanhong Guo (2):
  spi: add driver for ar934x spi controller
  dt-binding: spi: add bindings for spi-ar934x

 .../bindings/spi/qca,ar934x-spi.yaml          |  40 +++
 drivers/spi/Kconfig                           |   7 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-ar934x.c                      | 230 ++++++++++++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/qca,ar934x-spi.yaml
 create mode 100644 drivers/spi/spi-ar934x.c

-- 
2.24.1

[PATCH resend 1/2] spi: add driver for ar934x spi controller

[Chuanhong Guo]

This patch adds driver for SPI controller found in Qualcomm Atheros
AR934x/QCA95xx SoCs.
This controller is a superset of the already supported qca,ar7100-spi.
Besides the bit-bang mode in spi-ath79.c, this new controller added
a new "shift register" mode, allowing faster spi operations.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/Kconfig      |   7 ++
 drivers/spi/Makefile     |   1 +
 drivers/spi/spi-ar934x.c | 230 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/spi/spi-ar934x.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index d6ed0c355954..0434614d8201 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -62,6 +62,13 @@ config SPI_ALTERA
 	help
 	  This is the driver for the Altera SPI Controller.
 
+config SPI_AR934X
+	tristate "Qualcomm Atheros AR934X/QCA95XX SPI controller driver"
+	depends on ATH79 || COMPILE_TEST
+	help
+	  This enables support for the SPI controller present on the
+	  Qualcomm Atheros AR934X/QCA95XX SoCs.
+
 config SPI_ATH79
 	tristate "Atheros AR71XX/AR724X/AR913X SPI controller driver"
 	depends on ATH79 || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9b65ec5afc5e..9892bdfab15e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
+obj-$(CONFIG_SPI_AR934X)		+= spi-ar934x.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATMEL_QUADSPI)		+= atmel-quadspi.o
diff --git a/drivers/spi/spi-ar934x.c b/drivers/spi/spi-ar934x.c
new file mode 100644
index 000000000000..6b42382148ed
--- /dev/null
+++ b/drivers/spi/spi-ar934x.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SPI controller driver for Qualcomm Atheros AR934x/QCA95xx SoCs
+ *
+ * Copyright (C) 2020 Chuanhong Guo <gch981213@gmail.com>
+ *
+ * Based on spi-mt7621.c:
+ * Copyright (C) 2011 Sergiy <piratfm@gmail.com>
+ * Copyright (C) 2011-2013 Gabor Juhos <juhosg@openwrt.org>
+ * Copyright (C) 2014-2015 Felix Fietkau <nbd@nbd.name>
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME "spi-ar934x"
+
+#define AR934X_SPI_REG_FS		0x00
+#define AR934X_SPI_ENABLE		BIT(0)
+
+#define AR934X_SPI_REG_CTRL		0x04
+#define AR934X_SPI_CLK_MASK		GENMASK(5, 0)
+
+#define AR934X_SPI_DATAOUT		0x10
+
+#define AR934X_SPI_REG_SHIFT_CTRL	0x14
+#define AR934X_SPI_SHIFT_EN		BIT(31)
+#define AR934X_SPI_SHIFT_CS(n)		BIT(28 + (n))
+#define AR934X_SPI_SHIFT_TERM		26
+#define AR934X_SPI_SHIFT_VAL(cs, term, count)			\
+	(AR934X_SPI_SHIFT_EN | AR934X_SPI_SHIFT_CS(cs) |	\
+	(term) << AR934X_SPI_SHIFT_TERM | (count))
+
+#define AR934X_SPI_DATAIN 0x18
+
+struct ar934x_spi {
+	struct spi_controller *ctlr;
+	void __iomem *base;
+	struct clk *clk;
+	unsigned int clk_freq;
+};
+
+static inline int ar934x_spi_clk_div(struct ar934x_spi *sp, unsigned int freq)
+{
+	int div = DIV_ROUND_UP(sp->clk_freq, freq * 2) - 1;
+
+	if (div < 0)
+		return 0;
+	else if (div > AR934X_SPI_CLK_MASK)
+		return -EINVAL;
+	else
+		return div;
+}
+
+static int ar934x_spi_setup(struct spi_device *spi)
+{
+	struct ar934x_spi *sp = spi_controller_get_devdata(spi->master);
+
+	if ((spi->max_speed_hz == 0) ||
+	    (spi->max_speed_hz > (sp->clk_freq / 2))) {
+		spi->max_speed_hz = sp->clk_freq / 2;
+	} else if (spi->max_speed_hz < (sp->clk_freq / 128)) {
+		dev_err(&spi->dev, "spi clock is too low\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ar934x_spi_transfer_one(struct spi_controller *master,
+				   struct spi_message *m)
+{
+	struct ar934x_spi *sp = spi_controller_get_devdata(master);
+	struct spi_transfer *t = NULL;
+	struct spi_device *spi = m->spi;
+	unsigned long trx_done, trx_cur;
+	int stat = 0;
+	u8 term = 0;
+	int div, i;
+	u32 reg;
+	const u8 *tx_buf;
+	u8 *buf;
+
+	m->actual_length = 0;
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->speed_hz)
+			div = ar934x_spi_clk_div(sp, t->speed_hz);
+		else
+			div = ar934x_spi_clk_div(sp, spi->max_speed_hz);
+		if (div < 0) {
+			stat = -EIO;
+			goto msg_done;
+		}
+
+		reg = ioread32(sp->base + AR934X_SPI_REG_CTRL);
+		reg &= ~AR934X_SPI_CLK_MASK;
+		reg |= div;
+		iowrite32(reg, sp->base + AR934X_SPI_REG_CTRL);
+		iowrite32(0, sp->base + AR934X_SPI_DATAOUT);
+
+		for (trx_done = 0; trx_done < t->len; trx_done += 4) {
+			trx_cur = t->len - trx_done;
+			if (trx_cur > 4)
+				trx_cur = 4;
+			else if (list_is_last(&t->transfer_list, &m->transfers))
+				term = 1;
+
+			if (t->tx_buf) {
+				tx_buf = t->tx_buf + trx_done;
+				reg = tx_buf[0];
+				for (i = 1; i < trx_cur; i++)
+					reg = reg << 8 | tx_buf[i];
+				iowrite32(reg, sp->base + AR934X_SPI_DATAOUT);
+			}
+
+			reg = AR934X_SPI_SHIFT_VAL(spi->chip_select, term,
+						   trx_cur * 8);
+			iowrite32(reg, sp->base + AR934X_SPI_REG_SHIFT_CTRL);
+			stat = readl_poll_timeout(
+				sp->base + AR934X_SPI_REG_SHIFT_CTRL, reg,
+				!(reg & AR934X_SPI_SHIFT_EN), 0, 5);
+			if (stat < 0)
+				goto msg_done;
+
+			if (t->rx_buf) {
+				reg = ioread32(sp->base + AR934X_SPI_DATAIN);
+				buf = t->rx_buf + trx_done;
+				for (i = 0; i < trx_cur; i++) {
+					buf[trx_cur - i - 1] = reg & 0xff;
+					reg >>= 8;
+				}
+			}
+		}
+		m->actual_length += t->len;
+	}
+
+msg_done:
+	m->status = stat;
+	spi_finalize_current_message(master);
+
+	return 0;
+}
+
+static const struct of_device_id ar934x_spi_match[] = {
+	{ .compatible = "qca,ar934x-spi" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ar934x_spi_match);
+
+static int ar934x_spi_probe(struct platform_device *pdev)
+{
+	struct spi_controller *ctlr;
+	struct ar934x_spi *sp;
+	void __iomem *base;
+	struct clk *clk;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "failed to get clock\n");
+		return PTR_ERR(clk);
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		return ret;
+
+	ctlr = spi_alloc_master(&pdev->dev, sizeof(*sp));
+	if (!ctlr) {
+		dev_info(&pdev->dev, "failed to allocate spi controller\n");
+		return -ENOMEM;
+	}
+
+	iowrite32(AR934X_SPI_ENABLE, base + AR934X_SPI_REG_FS);
+
+	ctlr->mode_bits = SPI_LSB_FIRST;
+	ctlr->setup = ar934x_spi_setup;
+	ctlr->transfer_one_message = ar934x_spi_transfer_one;
+	ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
+	ctlr->dev.of_node = pdev->dev.of_node;
+	ctlr->num_chipselect = 3;
+
+	dev_set_drvdata(&pdev->dev, ctlr);
+
+	sp = spi_controller_get_devdata(ctlr);
+	sp->base = base;
+	sp->clk = clk;
+	sp->clk_freq = clk_get_rate(clk);
+	sp->ctlr = ctlr;
+
+	return devm_spi_register_controller(&pdev->dev, ctlr);
+}
+
+static int ar934x_spi_remove(struct platform_device *pdev)
+{
+	struct spi_controller *ctlr;
+	struct ar934x_spi *sp;
+
+	ctlr = dev_get_drvdata(&pdev->dev);
+	sp = spi_controller_get_devdata(ctlr);
+
+	clk_disable_unprepare(sp->clk);
+
+	return 0;
+}
+
+static struct platform_driver ar934x_spi_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = ar934x_spi_match,
+	},
+	.probe = ar934x_spi_probe,
+	.remove = ar934x_spi_remove,
+};
+
+module_platform_driver(ar934x_spi_driver);
+
+MODULE_DESCRIPTION("SPI controller driver for Qualcomm Atheros AR934x/QCA95xx");
+MODULE_AUTHOR("Chuanhong Guo <gch981213@gmail.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.24.1

Re: [PATCH resend 1/2] spi: add driver for ar934x spi controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Thu, Feb 06, 2020 at 04:44:42PM +0800, Chuanhong Guo wrote:

This looks good, just a couple of comments below:

> --- /dev/null
> +++ b/drivers/spi/spi-ar934x.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SPI controller driver for Qualcomm Atheros AR934x/QCA95xx SoCs

Please make the entire comment block a C++ one so things look
more intentional.

> +static int ar934x_spi_transfer_one(struct spi_controller *master,
> +				   struct spi_message *m)
> +{
> +	struct ar934x_spi *sp = spi_controller_get_devdata(master);
> +	struct spi_transfer *t = NULL;

...

> +
> +	m->actual_length = 0;
> +	list_for_each_entry(t, &m->transfers, transfer_list) {

It looks like this could just be a transfer_one() operation
instead of transfer_one_message() (which is what this is in spite
of the name)?  There's nothing custom outside this loop that I
can see.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH resend 1/2] spi: add driver for ar934x spi controller

[Chuanhong Guo]

On Thu, Feb 6, 2020 at 7:31 PM Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Thu, Feb 06, 2020 at 04:44:42PM +0800, Chuanhong Guo wrote:
>
> This looks good, just a couple of comments below:
>
> > --- /dev/null
> > +++ b/drivers/spi/spi-ar934x.c
> > @@ -0,0 +1,230 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SPI controller driver for Qualcomm Atheros AR934x/QCA95xx SoCs
>
> Please make the entire comment block a C++ one so things look
> more intentional.

Got it. I'll do this in v2.

>
> > +static int ar934x_spi_transfer_one(struct spi_controller *master,
> > +                                struct spi_message *m)
> > +{
> > +     struct ar934x_spi *sp = spi_controller_get_devdata(master);
> > +     struct spi_transfer *t = NULL;
>
> ...
>
> > +
> > +     m->actual_length = 0;
> > +     list_for_each_entry(t, &m->transfers, transfer_list) {
>
> It looks like this could just be a transfer_one() operation
> instead of transfer_one_message() (which is what this is in spite
> of the name)?  There's nothing custom outside this loop that I
> can see.

Chipselect is also handled during transfer. Controller asserts
corresponding chipselect in SHIFT_CTRL register, and if SHIFT_TERM bit
is set, controller will deassert chipselect after current transfer is
done. I need to know whether this is the last transfer and set
SHIFT_TERM accordingly.

Regards,
Chuanhong Guo

Re: [PATCH resend 1/2] spi: add driver for ar934x spi controller

[Chuanhong Guo]

On Thu, Feb 6, 2020 at 8:30 PM Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > It looks like this could just be a transfer_one() operation
> > instead of transfer_one_message() (which is what this is in spite
> > of the name)?  There's nothing custom outside this loop that I
> > can see.
>
> Chipselect is also handled during transfer. Controller asserts
> corresponding chipselect in SHIFT_CTRL register, and if SHIFT_TERM bit
> is set, controller will deassert chipselect after current transfer is
> done. I need to know whether this is the last transfer and set
> SHIFT_TERM accordingly.

Oh, I remembered that I saw transfer_one function name somewhere and
thought maybe I could shorten the function name a bit. I'll correct
this back to ar934x_spi_transfer_one_message in v2.

Regards,
Chuanhong Guo

Re: [PATCH resend 1/2] spi: add driver for ar934x spi controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Thu, Feb 06, 2020 at 08:33:33PM +0800, Chuanhong Guo wrote:
> On Thu, Feb 6, 2020 at 8:30 PM Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> > Chipselect is also handled during transfer. Controller asserts
> > corresponding chipselect in SHIFT_CTRL register, and if SHIFT_TERM bit
> > is set, controller will deassert chipselect after current transfer is
> > done. I need to know whether this is the last transfer and set
> > SHIFT_TERM accordingly.

> Oh, I remembered that I saw transfer_one function name somewhere and
> thought maybe I could shorten the function name a bit. I'll correct
> this back to ar934x_spi_transfer_one_message in v2.

OK, sounds good - I see the chip select handling now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH resend 2/2] dt-binding: spi: add bindings for spi-ar934x

[Chuanhong Guo]

Add binding documentation for SPI controller in Qualcomm Atheros
AR934x/QCA95xx SoCs.

Signed-off-by: Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../bindings/spi/qca,ar934x-spi.yaml          | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/qca,ar934x-spi.yaml

diff --git a/Documentation/devicetree/bindings/spi/qca,ar934x-spi.yaml b/Documentation/devicetree/bindings/spi/qca,ar934x-spi.yaml
new file mode 100644
index 000000000000..8f0c520a571c
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/qca,ar934x-spi.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/qca,ar934x-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Atheros AR934x/QCA95xx SoC SPI controller
+
+maintainers:
+  - Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    const: qca,ar934x-spi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+examples:
+  - |
+    spi: spi@1f000000 {
+        compatible = "qca,ar934x-spi";
+        reg = <0x1f000000 0x1c>;
+        clocks = <&pll ATH79_CLK_AHB>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
-- 
2.24.1