2020-12-28 03:01:25

by Qing Zhang

[permalink] [raw]
Subject: [PATCH v6 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support

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: Qing Zhang <[email protected]>
---

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

v4:
- names used in the manual
- rename ls7a_spi_do_transfer to ls7a_spi_setup_transfer
- change read the spcr and sper outside of this function
- mode configuration moved to prepare instead
- remove redundancy code about unprepare/prepare_message
- used 0x4 instead of 0x1,WFEMPTY instead of RFEMPTY

v5:
- remove unnecessary blank lines

v6:
- keep blank line before the last "return xxx"

---
drivers/spi/Kconfig | 7 ++
drivers/spi/Makefile | 1 +
drivers/spi/spi-ls7a.c | 282 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 290 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..8592b85
--- /dev/null
+++ b/drivers/spi/spi-ls7a.c
@@ -0,0 +1,282 @@
+// 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 SFC_PARAM 0x04
+#define SFC_SOFTCS 0x05
+#define SFC_TIMING 0x06
+
+struct ls7a_spi {
+ struct spi_master *master;
+ void __iomem *base;
+ unsigned int hz;
+ 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;
+ struct spi_device *spi = msg->spi;
+ unsigned char val;
+
+ ls7a_spi = spi_master_get_devdata(master);
+
+ if (ls7a_spi->mode != spi->mode) {
+ 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);
+ val = ls7a_spi_read_reg(ls7a_spi, SPER);
+ ls7a_spi_write_reg(ls7a_spi, SPER, val);
+ ls7a_spi->mode = spi->mode;
+ }
+
+ 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, SFC_SOFTCS) & ~(0x11 << spi->chip_select);
+
+ if (!!(spi->mode & SPI_CS_HIGH) == enable)
+ ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x1 << spi->chip_select) | cs);
+ else
+ ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x11 << spi->chip_select) | cs);
+}
+
+static int ls7a_spi_setup_transfer(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;
+ unsigned char spcr, sper;
+ const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
+ struct ls7a_spi *ls7a_spi;
+
+ ls7a_spi = spi_master_get_devdata(spi->master);
+
+ if (t) {
+ hz = t->speed_hz;
+ if (!hz)
+ hz = spi->max_speed_hz;
+ } else {
+ hz = spi->max_speed_hz;
+ }
+
+ if (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;
+ spcr = div_tmp & 3;
+ sper = (div_tmp >> 2) & 3;
+
+ val = ls7a_spi_read_reg(ls7a_spi, SPCR);
+ ls7a_spi_write_reg(ls7a_spi, SPCR, (val & ~3) | spcr);
+ val = ls7a_spi_read_reg(ls7a_spi, SPER);
+ ls7a_spi_write_reg(ls7a_spi, SPER, (val & ~3) | sper);
+ }
+
+ return 0;
+}
+
+static int ls7a_spi_setup(struct spi_device *spi)
+{
+ return ls7a_spi_setup_transfer(spi, NULL);
+}
+
+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) & 0x4) == 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)
+{
+ int status = 0;
+
+ status = ls7a_spi_setup_transfer(spi, t);
+ if (status < 0)
+ return status;
+
+ if (t->len)
+ 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, SFC_TIMING, 0x01);
+ ls7a_spi_write_reg(spi, SFC_PARAM, 0x40);
+ spi->mode = 0;
+
+ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+ master->set_cs = ls7a_spi_set_cs;
+ master->prepare_message = ls7a_spi_prepare_message;
+ master->setup = ls7a_spi_setup;
+ master->transfer_one = ls7a_spi_transfer_one;
+ 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 <[email protected]>");
+MODULE_AUTHOR("Qing Zhang <[email protected]>");
+MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0


2020-12-28 03:02:04

by Qing Zhang

[permalink] [raw]
Subject: [PATCH v6 2/4] spi: ls7a: Add YAML schemas

Switch the DT binding to a YAML schema to enable the DT validation.

Reviewed-by: Huacai Chen <[email protected]>
Signed-off-by: Qing Zhang <[email protected]>
---

v4:
- fix warnings/errors about running 'make dt_binding_check'

v5:
- remove num-chipelects

v6:
- No changes

---
.../devicetree/bindings/spi/loongson,spi-ls7a.yaml | 44 ++++++++++++++++++++++
1 file changed, 44 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..b90b28b
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/loongson,spi-ls7a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson LS7A PCH SPI Controller
+
+maintainers:
+ - Qing Zhang <[email protected]>
+
+description: |
+ This controller can be found in Loongson-3 systems with LS7A PCH.
+
+properties:
+ compatible:
+ const: loongson,ls7a-spi
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ pci {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ spi@16,0 {
+ compatible = "pci0014,7a0b.0",
+ "pci0014,7a0b",
+ "pciclass088000",
+ "pciclass0800";
+
+ reg = <0xb000 0x0 0x0 0x0 0x0>;
+ };
+ };
+
+...
--
2.1.0

2020-12-28 03:02:12

by Qing Zhang

[permalink] [raw]
Subject: [PATCH v6 3/4] MIPS: Loongson64: DTS: Add SPI support to LS7A

Add spi support.

Reviewed-by: Huacai Chen <[email protected]>
Signed-off-by: Qing Zhang <[email protected]>
---

v2:
- Add spi about pci device DT

v3:
- Remove spiflash node

v4:
- Remove useless compatible

v5:
- Remove num-chipselects

v6:
- No changes

---
arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
index f99a7a1..dba717d 100644
--- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
@@ -405,6 +405,15 @@
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>;
+ };
};

isa {
--
2.1.0

2020-12-28 03:02:56

by Qing Zhang

[permalink] [raw]
Subject: [PATCH v6 4/4] MIPS: Loongson: Enable Loongson LS7A SPI in loongson3_defconfig

This is now supported, enable for Loongson systems.

Reviewed-by: Huacai Chen <[email protected]>
Signed-off-by: Qing Zhang <[email protected]>
---

v2:
- Modify CONFIG_SPI_LOONGSON to CONFIG_SPI_LS7A

v3:
- No changes

v4:
- No changes

v5:
- No changes

v6:
- 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

2020-12-28 08:43:45

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support

Am 28.12.20 um 03:59 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: Qing Zhang <[email protected]>
> ---
>
> 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
>
> v4:
> - names used in the manual
> - rename ls7a_spi_do_transfer to ls7a_spi_setup_transfer
> - change read the spcr and sper outside of this function
> - mode configuration moved to prepare instead
> - remove redundancy code about unprepare/prepare_message
> - used 0x4 instead of 0x1,WFEMPTY instead of RFEMPTY
>
> v5:
> - remove unnecessary blank lines
>
> v6:
> - keep blank line before the last "return xxx"
>
> ---
> drivers/spi/Kconfig | 7 ++
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-ls7a.c | 282 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 290 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..8592b85
> --- /dev/null
> +++ b/drivers/spi/spi-ls7a.c
> @@ -0,0 +1,282 @@
> +// 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 SFC_PARAM 0x04
> +#define SFC_SOFTCS 0x05
> +#define SFC_TIMING 0x06
> +
> +struct ls7a_spi {
> + struct spi_master *master;
> + void __iomem *base;
> + unsigned int hz;
> + 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;
> + struct spi_device *spi = msg->spi;
> + unsigned char val;
> +
> + ls7a_spi = spi_master_get_devdata(master);
> +
> + if (ls7a_spi->mode != spi->mode) {
> + 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);
> + val = ls7a_spi_read_reg(ls7a_spi, SPER);
> + ls7a_spi_write_reg(ls7a_spi, SPER, val);
> + ls7a_spi->mode = spi->mode;
> + }
> +
> + 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, SFC_SOFTCS) & ~(0x11 << spi->chip_select);
> +
> + if (!!(spi->mode & SPI_CS_HIGH) == enable)
> + ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x1 << spi->chip_select) | cs);
> + else
> + ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x11 << spi->chip_select) | cs);
> +}
> +
> +static int ls7a_spi_setup_transfer(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;
> + unsigned char spcr, sper;
> + const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
> + struct ls7a_spi *ls7a_spi;
> +
> + ls7a_spi = spi_master_get_devdata(spi->master);
> +
> + if (t) {
> + hz = t->speed_hz;
> + if (!hz)
> + hz = spi->max_speed_hz;
> + } else {
> + hz = spi->max_speed_hz;
> + }
> +
> + if (hz & (ls7a_spi->hz != hz)) {
> + clk = 100000000;

Default clock frequency should not be here. As i already noted, LS7A has
configurable clock source for SPI core. If you can implement proper clk
framework support now, then assign it at least in the probe, not here.

Did you actually compared the calculated frequency by software and
measured actual frequency on the SPI bus? I still assume, you have 50MHz
input, not 100MHz.

> +
> + 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;
> + spcr = div_tmp & 3;
> + sper = (div_tmp >> 2) & 3;
> +
> + val = ls7a_spi_read_reg(ls7a_spi, SPCR);
> + ls7a_spi_write_reg(ls7a_spi, SPCR, (val & ~3) | spcr);
> + val = ls7a_spi_read_reg(ls7a_spi, SPER);
> + ls7a_spi_write_reg(ls7a_spi, SPER, (val & ~3) | sper);
> + }
> +
> + return 0;
> +}
> +
> +static int ls7a_spi_setup(struct spi_device *spi)
> +{
> + return ls7a_spi_setup_transfer(spi, NULL);
> +}
> +
> +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) & 0x4) == 1)

Please use define instead of magin number. It means, here should be
WFEMPTY instead of 0x4. The same is for all other part in this driver.

Beside, what is the size of this FIFO? Currently you are transferring
8bits at time with busy waiting. So, FIFO is not used. Since the SPI bus
is slow, and the CPU on LS7A system is fast, this driver can probably be
used as coffee heater.
To make use of the FIFO, you need to test if FIFO is full, before
writing to it. And if it is full, use interrupt or at least proper
polling function. For example: readb_poll_timeout()

Timeout is needed to avoid blocked system, just in case SPI failed.

> + ;
> + } 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)

val < 0 means, you expect here an error, but it is ignored. with
poll_timout you will be able to get a real error, please do not ignore it.

> + 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)
> +{
> + int status = 0;
> +
> + status = ls7a_spi_setup_transfer(spi, t);
> + if (status < 0)
> + return status;
> +
> + if (t->len)
> + 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, SFC_TIMING, 0x01);

are there any reason to configure time for the SPI_FLASH mode?

> + ls7a_spi_write_reg(spi, SFC_PARAM, 0x40);

And then disable SPI_FLASH mode at this step?

The main reason I requested you to avoid magic numbers and add some
comments is: to make you and reviewers understand what is happening here.

> + spi->mode = 0;
> +
> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> + master->set_cs = ls7a_spi_set_cs;
> + master->prepare_message = ls7a_spi_prepare_message;
> + master->setup = ls7a_spi_setup;
> + master->transfer_one = ls7a_spi_transfer_one;
> + 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 <[email protected]>");
> +MODULE_AUTHOR("Qing Zhang <[email protected]>");
> +MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
> +MODULE_LICENSE("GPL v2");
>


--
Regards,
Oleksij

2021-01-05 14:19:01

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] spi: LS7A: Add Loongson LS7A SPI controller driver support

On Mon, Dec 28, 2020 at 10:59:38AM +0800, Qing Zhang wrote:

> + if (!!(spi->mode & SPI_CS_HIGH) == enable)
> + ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x1 << spi->chip_select) | cs);
> + else
> + ls7a_spi_write_reg(ls7a_spi, SFC_SOFTCS, (0x11 << spi->chip_select) | cs);
> +}

The core will handle inverting the chip select for the driver when
SIP_CS_HIGH so doing it again here will cause bugs. Just use the value
of enable directly. Otherwise this looks good.


Attachments:
(No filename) (480.00 B)
signature.asc (499.00 B)
Download all attachments

2021-01-08 03:42:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] spi: ls7a: Add YAML schemas

On Mon, Dec 28, 2020 at 10:59:39AM +0800, Qing Zhang wrote:
> Switch the DT binding to a YAML schema to enable the DT validation.
>
> Reviewed-by: Huacai Chen <[email protected]>
> Signed-off-by: Qing Zhang <[email protected]>
> ---
>
> v4:
> - fix warnings/errors about running 'make dt_binding_check'
>
> v5:
> - remove num-chipelects
>
> v6:
> - No changes
>
> ---
> .../devicetree/bindings/spi/loongson,spi-ls7a.yaml | 44 ++++++++++++++++++++++
> 1 file changed, 44 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..b90b28b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/loongson,spi-ls7a.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/loongson,spi-ls7a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson LS7A PCH SPI Controller
> +
> +maintainers:
> + - Qing Zhang <[email protected]>
> +
> +description: |
> + This controller can be found in Loongson-3 systems with LS7A PCH.

allOf:
- $ref: spi-controller.yaml#

> +
> +properties:
> + compatible:
> + const: loongson,ls7a-spi
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pci {
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + spi@16,0 {
> + compatible = "pci0014,7a0b.0",
> + "pci0014,7a0b",
> + "pciclass088000",
> + "pciclass0800";

Doesn't match the schema. If this is on PCI bus, then the example is
correct. Though you could drop some of the strings. And I think leading
0s should be omitted. So 'pci14,7a0b'. But please double check that.

Rob