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 <[email protected]>
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
---
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 <[email protected]>");
+MODULE_AUTHOR("Qing Zhang <[email protected]>");
+MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
This is now supported, enable for Loongson systems.
Signed-off-by: Qing Zhang <[email protected]>
---
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
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.
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 <[email protected]>
> 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
>
> ---
> 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 <[email protected]>");
> +MODULE_AUTHOR("Qing Zhang <[email protected]>");
> +MODULE_DESCRIPTION("Loongson LS7A SPI controller driver");
> +MODULE_LICENSE("GPL v2");
--
Regards,
Oleksij