* [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support
@ 2020-12-14 5:58 Qing Zhang
2020-12-14 5:58 ` [PATCH v3 2/4] spi: ls7a: Add YAML schemas Qing Zhang
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Qing Zhang @ 2020-12-14 5:58 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Thomas Bogendoerfer
Cc: linux-spi, Huacai Chen, Jiaxun Yang, devicetree, linux-mips,
linux-kernel, Juxin Gao
The SPI controller has the following characteristics:
- Full-duplex synchronous serial data transmission
- Support up to 4 variable length byte transmission
- Main mode support
- Mode failure generates an error flag and issues an interrupt request
- Double buffer receiver
- Serial clock with programmable polarity and phase
- SPI can be controlled in wait mode
- Support boot from SPI
Use mtd_debug tool to earse/write/read /dev/mtd0 on development.
eg:
[root@linux mtd-utils-1.0.0]# mtd_debug erase /dev/mtd0 0x20000 0x40000
Erased 262144 bytes from address 0x00020000 in flash
[root@linux mtd-utils-1.0.0]# mtd_debug write /dev/mtd0 0x20000 13 1.img
Copied 13 bytes from 1.img to address 0x00020000 in flash
[root@linux mtd-utils-1.0.0]# mtd_debug read /dev/mtd0 0x20000 13 2.img
Copied 13 bytes from address 0x00020000 in flash to 2.img
[root@linux mtd-utils-1.0.0]# cmp -l 1.img 2.img
Signed-off-by: Juxin Gao <gaojuxin@loongson.cn>
Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
---
v2:
- keep Kconfig and Makefile sorted
- make the entire comment a C++ one so things look more intentional
- Fix unclear indentation
- make conditional statements to improve legibility
- Don't use static inline
- the core handle message queue
- Add a new binding document
- Fix probe part mixed pdev and PCI
v3:
- expose set_cs to the core and let it handle things
- replace transfer_one_message to transfer_one
- replace spi_alloc_master to devm_spi_alloc_master
- split out into prepare/unprepare_message
- releases pci regions before unregister master
---
drivers/spi/Kconfig | 7 ++
drivers/spi/Makefile | 1 +
drivers/spi/spi-ls7a.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 301 insertions(+)
create mode 100644 drivers/spi/spi-ls7a.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index aadaea0..af7c0d4 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -413,6 +413,13 @@ config SPI_LP8841_RTC
Say N here unless you plan to run the kernel on an ICP DAS
LP-8x4x industrial computer.
+config SPI_LS7A
+ tristate "Loongson LS7A SPI Controller Support"
+ depends on CPU_LOONGSON64 || COMPILE_TEST
+ help
+ This drivers supports the Loongson LS7A SPI controller in master
+ SPI mode.
+
config SPI_MPC52xx
tristate "Freescale MPC52xx SPI (non-PSC) controller support"
depends on PPC_MPC52xx
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 6fea582..d015cf2 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o
obj-$(CONFIG_SPI_JCORE) += spi-jcore.o
obj-$(CONFIG_SPI_LM70_LLP) += spi-lm70llp.o
obj-$(CONFIG_SPI_LP8841_RTC) += spi-lp8841-rtc.o
+obj-$(CONFIG_SPI_LS7A) += spi-ls7a.o
obj-$(CONFIG_SPI_MESON_SPICC) += spi-meson-spicc.o
obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o
obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
diff --git a/drivers/spi/spi-ls7a.c b/drivers/spi/spi-ls7a.c
new file mode 100644
index 0000000..d3b7e86
--- /dev/null
+++ b/drivers/spi/spi-ls7a.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Loongson LS7A SPI Controller driver
+//
+// Copyright (C) 2020 Loongson Technology Corporation Limited.
+//
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/spi/spi.h>
+
+/* define spi register */
+#define SPCR 0x00
+#define SPSR 0x01
+#define FIFO 0x02
+#define SPER 0x03
+#define PARA 0x04
+#define SFCS 0x05
+#define TIMI 0x06
+
+struct ls7a_spi {
+ spinlock_t lock;
+ struct spi_master *master;
+ void __iomem *base;
+ unsigned int hz;
+ unsigned char spcr, sper;
+ unsigned int mode;
+};
+
+static void ls7a_spi_write_reg(struct ls7a_spi *spi,
+ unsigned char reg,
+ unsigned char data)
+{
+ writeb(data, spi->base + reg);
+}
+
+static char ls7a_spi_read_reg(struct ls7a_spi *spi, unsigned char reg)
+{
+ return readb(spi->base + reg);
+}
+
+static int ls7a_spi_prepare_message(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct ls7a_spi *ls7a_spi;
+ int param;
+
+ ls7a_spi = spi_master_get_devdata(master);
+
+ spin_lock(&ls7a_spi->lock);
+ param = ls7a_spi_read_reg(ls7a_spi, PARA);
+ ls7a_spi_write_reg(ls7a_spi, PARA, param &= ~1);
+ spin_unlock(&ls7a_spi->lock);
+
+ return 0;
+}
+
+static int ls7a_spi_unprepare_message(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct ls7a_spi *ls7a_spi;
+ int param = 0;
+
+ ls7a_spi = spi_master_get_devdata(master);
+
+ spin_lock(&ls7a_spi->lock);
+ ls7a_spi_write_reg(ls7a_spi, PARA, param);
+ spin_unlock(&ls7a_spi->lock);
+
+ return 0;
+}
+
+static void ls7a_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ struct ls7a_spi *ls7a_spi;
+ int cs;
+
+ ls7a_spi = spi_master_get_devdata(spi->master);
+
+ cs = ls7a_spi_read_reg(ls7a_spi, SFCS) & ~(0x11 << spi->chip_select);
+
+ if (!!(spi->mode & SPI_CS_HIGH) == enable)
+ ls7a_spi_write_reg(ls7a_spi, SFCS, (0x1 << spi->chip_select) | cs);
+ else
+ ls7a_spi_write_reg(ls7a_spi, SFCS, (0x11 << spi->chip_select) | cs);
+}
+
+static int ls7a_spi_do_transfer(struct ls7a_spi *ls7a_spi,
+ struct spi_device *spi,
+ struct spi_transfer *t)
+{
+ unsigned int hz;
+ unsigned int div, div_tmp;
+ unsigned int bit;
+ unsigned long clk;
+ unsigned char val;
+ const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
+
+ if (t) {
+ hz = t->speed_hz;
+ if (!hz)
+ hz = spi->max_speed_hz;
+ } else {
+ hz = spi->max_speed_hz;
+ }
+
+ if (((spi->mode ^ ls7a_spi->mode) & (SPI_CPOL | SPI_CPHA))
+ || (hz && ls7a_spi->hz != hz)) {
+ clk = 100000000;
+
+ div = DIV_ROUND_UP(clk, hz);
+ if (div < 2)
+ div = 2;
+ if (div > 4096)
+ div = 4096;
+
+ bit = fls(div) - 1;
+ if ((1<<bit) == div)
+ bit--;
+ div_tmp = rdiv[bit];
+
+ dev_dbg(&spi->dev, "clk = %ld hz = %d div_tmp = %d bit = %d\n",
+ clk, hz, div_tmp, bit);
+
+ ls7a_spi->hz = hz;
+ ls7a_spi->spcr = div_tmp & 3;
+ ls7a_spi->sper = (div_tmp >> 2) & 3;
+
+ val = ls7a_spi_read_reg(ls7a_spi, SPCR);
+ val &= ~0xc;
+ if (spi->mode & SPI_CPOL)
+ val |= 8;
+ if (spi->mode & SPI_CPHA)
+ val |= 4;
+ ls7a_spi_write_reg(ls7a_spi, SPCR, (val & ~3) | ls7a_spi->spcr);
+ val = ls7a_spi_read_reg(ls7a_spi, SPER);
+ ls7a_spi_write_reg(ls7a_spi, SPER, (val & ~3) | ls7a_spi->sper);
+ ls7a_spi->mode = spi->mode;
+ }
+ return 0;
+}
+
+static int ls7a_spi_write_read_8bit(struct spi_device *spi,
+ const u8 **tx_buf, u8 **rx_buf,
+ unsigned int num)
+{
+ struct ls7a_spi *ls7a_spi;
+
+ ls7a_spi = spi_master_get_devdata(spi->master);
+
+ if (tx_buf && *tx_buf) {
+ ls7a_spi_write_reg(ls7a_spi, FIFO, *((*tx_buf)++));
+
+ while ((ls7a_spi_read_reg(ls7a_spi, SPSR) & 0x1) == 1)
+ ;
+ } else {
+ ls7a_spi_write_reg(ls7a_spi, FIFO, 0);
+
+ while ((ls7a_spi_read_reg(ls7a_spi, SPSR) & 0x1) == 1)
+ ;
+ }
+
+ if (rx_buf && *rx_buf)
+ *(*rx_buf)++ = ls7a_spi_read_reg(ls7a_spi, FIFO);
+ else
+ ls7a_spi_read_reg(ls7a_spi, FIFO);
+
+ return 1;
+}
+
+static unsigned int ls7a_spi_write_read(struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ unsigned int count;
+ const u8 *tx = xfer->tx_buf;
+
+ u8 *rx = xfer->rx_buf;
+
+ count = xfer->len;
+
+ do {
+ if (ls7a_spi_write_read_8bit(spi, &tx, &rx, count) < 0)
+ goto out;
+ count--;
+ } while (count);
+
+out:
+ return xfer->len - count;
+}
+
+static int ls7a_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *t)
+{
+ struct ls7a_spi *ls7a_spi;
+ int status;
+
+ ls7a_spi = spi_master_get_devdata(master);
+
+ status = ls7a_spi_do_transfer(ls7a_spi, spi, t);
+ if (status < 0)
+ return status;
+
+ ls7a_spi_write_read(spi, t);
+
+ return status;
+}
+
+static int ls7a_spi_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct spi_master *master;
+ struct ls7a_spi *spi;
+ int ret;
+
+ master = devm_spi_alloc_master(&pdev->dev, sizeof(*spi));
+ if (!master)
+ return -ENOMEM;
+
+ spi = spi_master_get_devdata(master);
+ ret = pcim_enable_device(pdev);
+ if (ret)
+ goto err_free_master;
+
+ ret = pci_request_regions(pdev, "ls7a-spi");
+ if (ret)
+ goto err_free_master;
+
+ spi->base = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
+ if (!spi->base) {
+ ret = -EINVAL;
+ goto err_free_master;
+ }
+ ls7a_spi_write_reg(spi, SPCR, 0x51);
+ ls7a_spi_write_reg(spi, SPER, 0x00);
+ ls7a_spi_write_reg(spi, TIMI, 0x01);
+ ls7a_spi_write_reg(spi, PARA, 0x40);
+ spi->mode = 0;
+
+ spin_lock_init(&spi->lock);
+
+ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+ master->prepare_message = ls7a_spi_prepare_message;
+ master->set_cs = ls7a_spi_set_cs;
+ master->transfer_one = ls7a_spi_transfer_one;
+ master->unprepare_message = ls7a_spi_unprepare_message;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->num_chipselect = 4;
+ master->dev.of_node = pdev->dev.of_node;
+
+ spi->master = master;
+
+ pci_set_drvdata(pdev, master);
+
+ ret = spi_register_master(master);
+ if (ret)
+ goto err_free_master;
+
+ return 0;
+
+err_free_master:
+ pci_release_regions(pdev);
+ return ret;
+}
+
+static void ls7a_spi_pci_remove(struct pci_dev *pdev)
+{
+ struct spi_master *master = pci_get_drvdata(pdev);
+
+ spi_unregister_master(master);
+ pci_release_regions(pdev);
+}
+
+static const struct pci_device_id ls7a_spi_pci_id_table[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) },
+ { 0, }
+};
+
+MODULE_DEVICE_TABLE(pci, ls7a_spi_pci_id_table);
+
+static struct pci_driver ls7a_spi_pci_driver = {
+ .name = "ls7a-spi",
+ .id_table = ls7a_spi_pci_id_table,
+ .probe = ls7a_spi_pci_probe,
+ .remove = ls7a_spi_pci_remove,
+};
+
+module_pci_driver(ls7a_spi_pci_driver);
+
+MODULE_AUTHOR("Juxin Gao <gaojuxin@loongson.cn>");
+MODULE_AUTHOR("Qing Zhang <zhangqing@loongson.cn>");
+MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/4] spi: ls7a: Add YAML schemas
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
@ 2020-12-14 5:58 ` Qing Zhang
2020-12-14 14:20 ` Rob Herring
2020-12-14 5:58 ` [PATCH v3 3/4] MIPS: Loongson64: DTS: Add SPI support to LS7A Qing Zhang
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Qing Zhang @ 2020-12-14 5:58 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Thomas Bogendoerfer
Cc: linux-spi, Huacai Chen, Jiaxun Yang, devicetree, linux-mips,
linux-kernel
Switch the DT binding to a YAML schema to enable the DT validation.
Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
---
.../devicetree/bindings/spi/loongson,spi-ls7a.yaml | 49 ++++++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
diff --git a/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
new file mode 100644
index 0000000..41691c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-ls7a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson LS7A PCH SPI Controller
+
+all0f:
+ - $ref: "spi-controller.yaml#"
+
+maintainers:
+ - Qing Zhang <zhangqing@loongson.cn>
+
+description: |
+ This controller can be found in Loongson-3 systems with LS7A PCH.
+
+properties:
+ "#address-cells": true
+ "#size-cells": true
+
+ compatible:
+ const: loongson,ls7a-spi
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - num-chipselects
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ spi@16,0 {
+ compatible = "pci0014,7a0b.0",
+ "pci0014,7a0b",
+ "pciclass088000",
+ "pciclass0800";
+
+ reg = <0xb000 0x0 0x0 0x0 0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-chipselects = <0>;
+ };
+
+...
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/4] MIPS: Loongson64: DTS: Add SPI support to LS7A
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
2020-12-14 5:58 ` [PATCH v3 2/4] spi: ls7a: Add YAML schemas Qing Zhang
@ 2020-12-14 5:58 ` Qing Zhang
2020-12-14 5:58 ` [PATCH v3 4/4] MIPS: Loongson: Enable Loongson LS7A SPI in loongson3_defconfig Qing Zhang
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Qing Zhang @ 2020-12-14 5:58 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Thomas Bogendoerfer
Cc: linux-spi, Huacai Chen, Jiaxun Yang, devicetree, linux-mips,
linux-kernel
add spi support.
Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
---
v2:
- Add spi about pci device DT
v3:
- Remove spiflash node
---
arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
index f99a7a1..d91857c 100644
--- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
@@ -405,6 +405,18 @@
interrupt-map-mask = <0 0 0 0>;
interrupt-map = <0 0 0 0 &pic 39 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ spi@16,0 {
+ compatible = "pci0014,7a0b.0",
+ "pci0014,7a0b",
+ "pciclass088000",
+ "pciclass0880";
+
+ reg = <0xb000 0x0 0x0 0x0 0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-chipselects = <0>;
+ };
};
isa {
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 4/4] MIPS: Loongson: Enable Loongson LS7A SPI in loongson3_defconfig
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
2020-12-14 5:58 ` [PATCH v3 2/4] spi: ls7a: Add YAML schemas Qing Zhang
2020-12-14 5:58 ` [PATCH v3 3/4] MIPS: Loongson64: DTS: Add SPI support to LS7A Qing Zhang
@ 2020-12-14 5:58 ` Qing Zhang
2020-12-14 17:48 ` [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Mark Brown
2020-12-14 18:32 ` Oleksij Rempel
4 siblings, 0 replies; 7+ messages in thread
From: Qing Zhang @ 2020-12-14 5:58 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Thomas Bogendoerfer
Cc: linux-spi, Huacai Chen, Jiaxun Yang, devicetree, linux-mips,
linux-kernel
This is now supported, enable for Loongson systems.
Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
---
v2:
- Modify CONFIG_SPI_LOONGSON to CONFIG_SPI_LS7A
v3:
- No changes
---
arch/mips/configs/loongson3_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
index 38a817e..28784cb 100644
--- a/arch/mips/configs/loongson3_defconfig
+++ b/arch/mips/configs/loongson3_defconfig
@@ -271,6 +271,9 @@ CONFIG_HW_RANDOM=y
CONFIG_RAW_DRIVER=m
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_PIIX4=y
+CONFIG_SPI=y
+CONFIG_SPI_MASTER=y
+CONFIG_SPI_LS7A=y
CONFIG_GPIO_LOONGSON=y
CONFIG_SENSORS_LM75=m
CONFIG_SENSORS_LM93=m
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/4] spi: ls7a: Add YAML schemas
2020-12-14 5:58 ` [PATCH v3 2/4] spi: ls7a: Add YAML schemas Qing Zhang
@ 2020-12-14 14:20 ` Rob Herring
0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-12-14 14:20 UTC (permalink / raw)
To: Qing Zhang
Cc: linux-spi, linux-kernel, Huacai Chen, Rob Herring, linux-mips,
Mark Brown, Thomas Bogendoerfer, Jiaxun Yang, devicetree
On Mon, 14 Dec 2020 13:58:52 +0800, Qing Zhang wrote:
> Switch the DT binding to a YAML schema to enable the DT validation.
>
> Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
> ---
> .../devicetree/bindings/spi/loongson,spi-ls7a.yaml | 49 ++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml:10:4: [warning] wrong indentation: expected 2 but found 3 (indentation)
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml: Additional properties are not allowed ('all0f' was unexpected)
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml: Additional properties are not allowed ('all0f' was unexpected)
./Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/spi/loongson,spi-ls7a.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dts:25.13-44: Warning (reg_format): /example-0/spi@16,0:reg: property has invalid length (20 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.example.dt.yaml: example-0: spi@16,0:reg:0: [45056, 0, 0, 0, 0] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
See https://patchwork.ozlabs.org/patch/1415811
The base for the patch is generally the last rc1. Any dependencies
should be noted.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
` (2 preceding siblings ...)
2020-12-14 5:58 ` [PATCH v3 4/4] MIPS: Loongson: Enable Loongson LS7A SPI in loongson3_defconfig Qing Zhang
@ 2020-12-14 17:48 ` Mark Brown
2020-12-14 18:32 ` Oleksij Rempel
4 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2020-12-14 17:48 UTC (permalink / raw)
To: Qing Zhang
Cc: Rob Herring, Thomas Bogendoerfer, linux-spi, Huacai Chen,
Jiaxun Yang, devicetree, linux-mips, linux-kernel, Juxin Gao
[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]
On Mon, Dec 14, 2020 at 01:58:51PM +0800, Qing Zhang wrote:
> +static int ls7a_spi_do_transfer(struct ls7a_spi *ls7a_spi,
> + struct spi_device *spi,
> + struct spi_transfer *t)
This does a lot of configuration, as far as I can see only the clock
rate can actually vary per transfer though? The mode configuration
looks like it should be moved to prepare instead, the divider settings
can be done with a read/modify/write. It's also not clear to me why
spcr and sper are being stored in the driver data, we never read the
values outside of this function.
> +static int ls7a_spi_transfer_one(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *t)
> +{
> + struct ls7a_spi *ls7a_spi;
> + int status;
> +
> + ls7a_spi = spi_master_get_devdata(master);
> +
> + status = ls7a_spi_do_transfer(ls7a_spi, spi, t);
> + if (status < 0)
> + return status;
> +
> + ls7a_spi_write_read(spi, t);
This is kind of confusing - _do_transfer() doesn't actually do the
transfer so far as I can see, write_read() does the transfer? Probably
both functions should be renamed, or even just inlined here - it's
really configuring the clocks and transferring the data.
Otherwise this looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
` (3 preceding siblings ...)
2020-12-14 17:48 ` [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Mark Brown
@ 2020-12-14 18:32 ` Oleksij Rempel
4 siblings, 0 replies; 7+ messages in thread
From: Oleksij Rempel @ 2020-12-14 18:32 UTC (permalink / raw)
To: Qing Zhang, Mark Brown, Rob Herring, Thomas Bogendoerfer
Cc: linux-spi, Huacai Chen, Jiaxun Yang, devicetree, linux-mips,
linux-kernel, Juxin Gao, ,
Du Huanpeng
Hi
After quick search I found (nearly?) identical SPI controller on many
other Loongson products. For example: ls1b, ls2k, 3A4000, etc...
Probably it will help you to make right decisions now, if you compare
this docs:
http://www.loongson.cn/product/
Am 14.12.20 um 06:58 schrieb Qing Zhang:
> The SPI controller has the following characteristics:
>
> - Full-duplex synchronous serial data transmission
> - Support up to 4 variable length byte transmission
> - Main mode support
> - Mode failure generates an error flag and issues an interrupt request
> - Double buffer receiver
> - Serial clock with programmable polarity and phase
> - SPI can be controlled in wait mode
> - Support boot from SPI
>
> Use mtd_debug tool to earse/write/read /dev/mtd0 on development.
>
> eg:
>
> [root@linux mtd-utils-1.0.0]# mtd_debug erase /dev/mtd0 0x20000 0x40000
> Erased 262144 bytes from address 0x00020000 in flash
> [root@linux mtd-utils-1.0.0]# mtd_debug write /dev/mtd0 0x20000 13 1.img
> Copied 13 bytes from 1.img to address 0x00020000 in flash
> [root@linux mtd-utils-1.0.0]# mtd_debug read /dev/mtd0 0x20000 13 2.img
> Copied 13 bytes from address 0x00020000 in flash to 2.img
> [root@linux mtd-utils-1.0.0]# cmp -l 1.img 2.img
>
> Signed-off-by: Juxin Gao <gaojuxin@loongson.cn>
> Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
> ---
>
> v2:
> - keep Kconfig and Makefile sorted
> - make the entire comment a C++ one so things look more intentional
> - Fix unclear indentation
> - make conditional statements to improve legibility
> - Don't use static inline
> - the core handle message queue
> - Add a new binding document
> - Fix probe part mixed pdev and PCI
>
> v3:
> - expose set_cs to the core and let it handle things
> - replace transfer_one_message to transfer_one
> - replace spi_alloc_master to devm_spi_alloc_master
> - split out into prepare/unprepare_message
> - releases pci regions before unregister master
>
> ---
> drivers/spi/Kconfig | 7 ++
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-ls7a.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 301 insertions(+)
> create mode 100644 drivers/spi/spi-ls7a.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index aadaea0..af7c0d4 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -413,6 +413,13 @@ config SPI_LP8841_RTC
> Say N here unless you plan to run the kernel on an ICP DAS
> LP-8x4x industrial computer.
>
> +config SPI_LS7A
> + tristate "Loongson LS7A SPI Controller Support"
> + depends on CPU_LOONGSON64 || COMPILE_TEST
LS1B is 32bit
> + help
> + This drivers supports the Loongson LS7A SPI controller in master
> + SPI mode.
> +
> config SPI_MPC52xx
> tristate "Freescale MPC52xx SPI (non-PSC) controller support"
> depends on PPC_MPC52xx
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 6fea582..d015cf2 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o
> obj-$(CONFIG_SPI_JCORE) += spi-jcore.o
> obj-$(CONFIG_SPI_LM70_LLP) += spi-lm70llp.o
> obj-$(CONFIG_SPI_LP8841_RTC) += spi-lp8841-rtc.o
> +obj-$(CONFIG_SPI_LS7A) += spi-ls7a.o
> obj-$(CONFIG_SPI_MESON_SPICC) += spi-meson-spicc.o
> obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o
> obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
> diff --git a/drivers/spi/spi-ls7a.c b/drivers/spi/spi-ls7a.c
> new file mode 100644
> index 0000000..d3b7e86
> --- /dev/null
> +++ b/drivers/spi/spi-ls7a.c
> @@ -0,0 +1,293 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Loongson LS7A SPI Controller driver
> +//
> +// Copyright (C) 2020 Loongson Technology Corporation Limited.
> +//
> +
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/spi/spi.h>
> +
> +/* define spi register */
> +#define SPCR 0x00
> +#define SPSR 0x01
> +#define FIFO 0x02
> +#define SPER 0x03
> +#define PARA 0x04
Please use names used in the manual. In this case instead of PARA was
used SFC_PARAM
> +#define SFCS 0x05
> +#define TIMI 0x06
Please, use driver specific names. Not too generic. For example:
LSXX_SPI_SPCR
Please, define bits and bit masks from this regs and replace all magic
numbers. It will help to read this code. It will be good if you can
provide some comments to this bits in english. If I see it correctly,
there are no translated documentations.
> +
> +struct ls7a_spi {
> + spinlock_t lock;
> + struct spi_master *master;
> + void __iomem *base;
> + unsigned int hz;
> + unsigned char spcr, sper;
> + unsigned int mode;
> +};
> +
> +static void ls7a_spi_write_reg(struct ls7a_spi *spi,
> + unsigned char reg,
> + unsigned char data)
> +{
> + writeb(data, spi->base + reg);
In this driver "spi" is used in different caseses:
- struct ls7a_spi *spi
- struct spi_device *spi
this is confusing. I personally prefer to call driver privat stucture as
"priv"
> +}
> +
> +static char ls7a_spi_read_reg(struct ls7a_spi *spi, unsigned char reg)
> +{
> + return readb(spi->base + reg);
> +}
> +
> +static int ls7a_spi_prepare_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct ls7a_spi *ls7a_spi;
> + int param;
> +
> + ls7a_spi = spi_master_get_devdata(master);
> +
> + spin_lock(&ls7a_spi->lock);
> + param = ls7a_spi_read_reg(ls7a_spi, PARA
> + ls7a_spi_write_reg(ls7a_spi, PARA, param &= ~1);
You are switching the SPI controller from SPI Flash mode, to plain SPI.
This was already done on the probe. Are there any reason for doing this
again?
> + spin_unlock(&ls7a_spi->lock);
> +
> + return 0;
> +}
> +
> +static int ls7a_spi_unprepare_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct ls7a_spi *ls7a_spi;
> + int param = 0;
> +
> + ls7a_spi = spi_master_get_devdata(master);
> +
> + spin_lock(&ls7a_spi->lock);
> + ls7a_spi_write_reg(ls7a_spi, PARA, param);
here we set all bit to 0, why? there is not enable bit, if i see it
correctly
> + spin_unlock(&ls7a_spi->lock);
> +
> + return 0;
> +}
> +
> +static void ls7a_spi_set_cs(struct spi_device *spi, bool enable)
> +{
> + struct ls7a_spi *ls7a_spi;
> + int cs;
> +
> + ls7a_spi = spi_master_get_devdata(spi->master);
> +
> + cs = ls7a_spi_read_reg(ls7a_spi, SFCS) & ~(0x11 << spi->chip_select);
> +
> + if (!!(spi->mode & SPI_CS_HIGH) == enable)
> + ls7a_spi_write_reg(ls7a_spi, SFCS, (0x1 << spi->chip_select) | cs);
> + else
> + ls7a_spi_write_reg(ls7a_spi, SFCS, (0x11 << spi->chip_select) | cs);
> +}
> +
> +static int ls7a_spi_do_transfer(struct ls7a_spi *ls7a_spi,
> + struct spi_device *spi,
> + struct spi_transfer *t)
The functiono name is confusing. What ever is done in this function is
not realated to this name.
> +{
> + unsigned int hz;
> + unsigned int div, div_tmp;
> + unsigned int bit;
> + unsigned long clk;
> + unsigned char val;
> + const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
Please use dividers instead of positions, it is easier to read.
> +
> + if (t) {
> + hz = t->speed_hz;
> + if (!hz)
> + hz = spi->max_speed_hz;
> + } else {
> + hz = spi->max_speed_hz;
> + }
> + if (((spi->mode ^ ls7a_spi->mode) & (SPI_CPOL | SPI_CPHA))
> + || (hz && ls7a_spi->hz != hz)) {
Please, do not put complete function in to the if statement. If the
statment is not true, just return.
> + clk = 100000000;
This code is made with assumption, there is fixed 100MHz clock provider.
According to the manual, the LS7A clock structure looks like this:
100MHz-->DIV2--->MISC Block
\-->SPI
If some one will change the DIV2, nasty things will happen. Other
Loongson products have divers and configurable PLLs as well, it will be
better if you start to use clk framework instead of hard coding clock speed.
> + div = DIV_ROUND_UP(clk, hz);
> + if (div < 2)
> + div = 2;
> + if (div > 4096)
> + div = 4096;
> +
> + bit = fls(div) - 1;
> + if ((1<<bit) == div)
> + bit--;
> + div_tmp = rdiv[bit];
This divider calculation assums that you have liniar divider. According
to the manual, your dividers are:
2, 4, 16, 32, 8, 64,...
This order looks identical in all loongsoon manuals, so I assume there
is a bug in the hardware.
> +
> + dev_dbg(&spi->dev, "clk = %ld hz = %d div_tmp = %d bit = %d\n",
> + clk, hz, div_tmp, bit);
> +
> + ls7a_spi->hz = hz;
After taking best possible divider, the outgoung clk is not equal to the
requested clock. Please store actual used speed by the controller to the
transfer->effective_speed_hz
See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/spi/spi-imx.c?h=v5.10&id=bf253e6bf6b876a4ce74db7dcf8a13b80d84aa5f
> + ls7a_spi->spcr = div_tmp & 3;
> + ls7a_spi->sper = (div_tmp >> 2) & 3;
> +
> + val = ls7a_spi_read_reg(ls7a_spi, SPCR);
> + val &= ~0xc;
> + if (spi->mode & SPI_CPOL)
> + val |= 8;
> + if (spi->mode & SPI_CPHA)
> + val |= 4;
Please, no magi numbers. Use defines.
> + ls7a_spi_write_reg(ls7a_spi, SPCR, (val & ~3) | ls7a_spi->spcr);
> + val = ls7a_spi_read_reg(ls7a_spi, SPER);
> + ls7a_spi_write_reg(ls7a_spi, SPER, (val & ~3) | ls7a_spi->sper);
> + ls7a_spi->mode = spi->mode;
> + }
> + return 0;
> +}
> +
> +static int ls7a_spi_write_read_8bit(struct spi_device *spi,
> + const u8 **tx_buf, u8 **rx_buf,
> + unsigned int num)
> +{
> + struct ls7a_spi *ls7a_spi;
> +
> + ls7a_spi = spi_master_get_devdata(spi->master);
> +
> + if (tx_buf && *tx_buf) {
Is it OK to write to FIFO without checking if it is actually empty?
> + ls7a_spi_write_reg(ls7a_spi, FIFO, *((*tx_buf)++));
> +
> + while ((ls7a_spi_read_reg(ls7a_spi, SPSR) & 0x1) == 1)
> + ;
Should be here used 0x4 instead of 0x1? WFEMPTY instead of RFEMPTY?
If SPI controller will fail for some reason, this while loop will block
complete system. Please, abort on time out.
> + } else {
> + ls7a_spi_write_reg(ls7a_spi, FIFO, 0);
> +
> + while ((ls7a_spi_read_reg(ls7a_spi, SPSR) & 0x1) == 1)
> + ;
> + }
Should we checl if RxFIFO is not empty before reading it?
> + if (rx_buf && *rx_buf)
> + *(*rx_buf)++ = ls7a_spi_read_reg(ls7a_spi, FIFO);
> + else
> + ls7a_spi_read_reg(ls7a_spi, FIFO);
> +
> + return 1;
> +}
> +
> +static unsigned int ls7a_spi_write_read(struct spi_device *spi,
> + struct spi_transfer *xfer)
> +{
> + unsigned int count;
> + const u8 *tx = xfer->tx_buf;
> +
> + u8 *rx = xfer->rx_buf;
> +
> + count = xfer->len;
> +
> + do {
> + if (ls7a_spi_write_read_8bit(spi, &tx, &rx, count) < 0)
> + goto out;
> + count--;
> + } while (count);
> +
> +out:
> + return xfer->len - count;
> +}
> +
> +static int ls7a_spi_transfer_one(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *t)
> +{
> + struct ls7a_spi *ls7a_spi;
> + int status;
> +
> + ls7a_spi = spi_master_get_devdata(master);
> +
> + status = ls7a_spi_do_transfer(ls7a_spi, spi, t);
> + if (status < 0)
> + return status;
> +
> + ls7a_spi_write_read(spi, t);
return value is ignored
> +
> + return status;
> +}
> +
> +static int ls7a_spi_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct spi_master *master;
> + struct ls7a_spi *spi;
> + int ret;
> +
> + master = devm_spi_alloc_master(&pdev->dev, sizeof(*spi));
> + if (!master)
> + return -ENOMEM;
> +
> + spi = spi_master_get_devdata(master);
> + ret = pcim_enable_device(pdev);
> + if (ret)
> + goto err_free_master;
> +
> + ret = pci_request_regions(pdev, "ls7a-spi");
> + if (ret)
> + goto err_free_master;
> +
> + spi->base = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
> + if (!spi->base) {
> + ret = -EINVAL;
> + goto err_free_master;
> + }
> + ls7a_spi_write_reg(spi, SPCR, 0x51);
> + ls7a_spi_write_reg(spi, SPER, 0x00);
> + ls7a_spi_write_reg(spi, TIMI, 0x01);
> + ls7a_spi_write_reg(spi, PARA, 0x40);
Please do not use magic number, add comments what you are doing here.
> + spi->mode = 0;
> +
> + spin_lock_init(&spi->lock);
> +
> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> + master->prepare_message = ls7a_spi_prepare_message;
> + master->set_cs = ls7a_spi_set_cs;
> + master->transfer_one = ls7a_spi_transfer_one;
> + master->unprepare_message = ls7a_spi_unprepare_message;
> + master->bits_per_word_mask = SPI_BPW_MASK(8);
> + master->num_chipselect = 4;
> + master->dev.of_node = pdev->dev.of_node;
> +
> + spi->master = master;
> +
> + pci_set_drvdata(pdev, master);
> +
> + ret = spi_register_master(master);
> + if (ret)
> + goto err_free_master;
> +
> + return 0;
> +
> +err_free_master:
> + pci_release_regions(pdev);
> + return ret;
> +}
> +
> +static void ls7a_spi_pci_remove(struct pci_dev *pdev)
> +{
> + struct spi_master *master = pci_get_drvdata(pdev);
> +
> + spi_unregister_master(master);
> + pci_release_regions(pdev);
> +}
> +
> +static const struct pci_device_id ls7a_spi_pci_id_table[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) },
> + { 0, }
> +};
On ls1b and ls2k this SPI controoler is a simple MEMIO.
You will need to add here probe as platform device as well.
> +MODULE_DEVICE_TABLE(pci, ls7a_spi_pci_id_table);
> +
> +static struct pci_driver ls7a_spi_pci_driver = {
> + .name = "ls7a-spi",
> + .id_table = ls7a_spi_pci_id_table,
> + .probe = ls7a_spi_pci_probe,
> + .remove = ls7a_spi_pci_remove,
> +};
> +
> +module_pci_driver(ls7a_spi_pci_driver);
> +
> +MODULE_AUTHOR("Juxin Gao <gaojuxin@loongson.cn>");
> +MODULE_AUTHOR("Qing Zhang <zhangqing@loongson.cn>");
> +MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
> +MODULE_LICENSE("GPL v2");
--
Regards,
Oleksij
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-14 18:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 5:58 [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Qing Zhang
2020-12-14 5:58 ` [PATCH v3 2/4] spi: ls7a: Add YAML schemas Qing Zhang
2020-12-14 14:20 ` Rob Herring
2020-12-14 5:58 ` [PATCH v3 3/4] MIPS: Loongson64: DTS: Add SPI support to LS7A Qing Zhang
2020-12-14 5:58 ` [PATCH v3 4/4] MIPS: Loongson: Enable Loongson LS7A SPI in loongson3_defconfig Qing Zhang
2020-12-14 17:48 ` [PATCH v3 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support Mark Brown
2020-12-14 18:32 ` Oleksij Rempel
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).