Hi,
this patchset adds a driver for the SPIFC (SPI flash controller)
available in Amlogic Meson6 and Meson8 SoCs. The driver has been
tested on a Meson8 based device to communicate with a Macronix
mx25l1606e serial flash.
The first patch adds a field to the spi_transfer structure that allows
the driver to know in transfer_one() whether the transfer is the last
one and change CS accordingly, without the need to reimplement
transfer_one_message().
Changes since v1 [ https://lkml.org/lkml/2014/11/9/35 ]:
- decrease busy wait timeout
- remove check on xfer->speed_hz value in meson_spifc_transfer_one()
- implement transfer_one() instead of transfer_one_message()
- remove useless memset of devdata
- use fixed regmap config
- implement PM operations
- don't set master->bus_num
- allow modular build of the driver
Beniamino Galvani (4):
spi: Add 'last' flag to spi_transfer structure
spi: meson: Add device tree bindings documentation for SPIFC
spi: meson: Add support for Amlogic Meson SPIFC
ARM: dts: meson: add node for SPIFC
.../devicetree/bindings/spi/spi-meson.txt | 22 +
arch/arm/boot/dts/meson.dtsi | 9 +
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-meson-spifc.c | 461 +++++++++++++++++++++
drivers/spi/spi.c | 6 +
include/linux/spi/spi.h | 2 +
7 files changed, 508 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-meson.txt
create mode 100644 drivers/spi/spi-meson-spifc.c
--
1.9.1
Some drivers need to know whether the current transfer is the last in
the message in order to properly handle CS. A common way to achieve
this is to reimplement transfer_one_message() but this leads to
undesirable code duplication.
This patch adds a 'last' field to the spi_transfer structure and
populates it before passing the structure to the driver.
Signed-off-by: Beniamino Galvani <[email protected]>
---
drivers/spi/spi.c | 6 ++++++
include/linux/spi/spi.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ebcb33d..fc7f02d 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -887,6 +887,7 @@ static void spi_pump_messages(struct kthread_work *work)
{
struct spi_master *master =
container_of(work, struct spi_master, pump_messages);
+ struct spi_transfer *xfer;
unsigned long flags;
bool was_busy = false;
int ret;
@@ -941,6 +942,11 @@ static void spi_pump_messages(struct kthread_work *work)
}
}
+ list_for_each_entry(xfer, &master->cur_msg->transfers, transfer_list) {
+ xfer->last = list_is_last(&xfer->transfer_list,
+ &master->cur_msg->transfers);
+ }
+
if (!was_busy)
trace_spi_master_busy(master);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 46d188a..37f055a 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -535,6 +535,7 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum);
* (SPI_NBITS_SINGLE) is used.
* @rx_nbits: number of bits used for reading. If 0 the default
* (SPI_NBITS_SINGLE) is used.
+ * @last: whether the transfer is the last in the message.
* @len: size of rx and tx buffers (in bytes)
* @speed_hz: Select a speed other than the device default for this
* transfer. If 0 the default (from @spi_device) is used.
@@ -620,6 +621,7 @@ struct spi_transfer {
unsigned cs_change:1;
unsigned tx_nbits:3;
unsigned rx_nbits:3;
+ unsigned last:1;
#define SPI_NBITS_SINGLE 0x01 /* 1bit transfer */
#define SPI_NBITS_DUAL 0x02 /* 2bits transfer */
#define SPI_NBITS_QUAD 0x04 /* 4bits transfer */
--
1.9.1
This adds documentation of device tree bindings for the Amlogic Meson
SPIFC (SPI Flash Controller).
Signed-off-by: Beniamino Galvani <[email protected]>
---
.../devicetree/bindings/spi/spi-meson.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-meson.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-meson.txt b/Documentation/devicetree/bindings/spi/spi-meson.txt
new file mode 100644
index 0000000..bb52a86
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-meson.txt
@@ -0,0 +1,22 @@
+Amlogic Meson SPI controllers
+
+* SPIFC (SPI Flash Controller)
+
+The Meson SPIFC is a controller optimized for communication with SPI
+NOR memories, without DMA support and a 64-byte unified transmit /
+receive buffer.
+
+Required properties:
+ - compatible: should be "amlogic,meson6-spifc"
+ - reg: physical base address and length of the controller registers
+ - clocks: phandle of the input clock for the baud rate generator
+ - #address-cells: should be 1
+ - #size-cells: should be 0
+
+ spi@c1108c80 {
+ compatible = "amlogic,meson6-spifc";
+ reg = <0xc1108c80 0x80>;
+ clocks = <&clk81>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
--
1.9.1
This adds a node for the SPI Flash Controller to the Amlogic Meson
DTS.
Signed-off-by: Beniamino Galvani <[email protected]>
Acked-by: Mark Brown <[email protected]>
---
arch/arm/boot/dts/meson.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index e6539ea..d28b16e 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -106,5 +106,14 @@
clocks = <&clk81>;
status = "disabled";
};
+
+ spifc: spi@c1108c80 {
+ compatible = "amlogic,meson6-spifc";
+ reg = <0xc1108c80 0x80>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&clk81>;
+ status = "disabled";
+ };
};
}; /* end of / */
--
1.9.1
This is a driver for the Amlogic Meson SPIFC (SPI flash controller),
which is one of the two SPI controllers available on the SoC. It
doesn't support DMA and has a 64-byte unified transmit/receive buffer.
The device is optimized for interfacing with SPI NOR memories and
allows the execution of standard operations such as read, page
program, sector erase, etc. in a simplified way, toggling a bit in a
dedicated register. The driver doesn't use those predefined commands
and relies only on custom transfers.
Signed-off-by: Beniamino Galvani <[email protected]>
---
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-meson-spifc.c | 461 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 469 insertions(+)
create mode 100644 drivers/spi/spi-meson-spifc.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 84e7c9e..70f20b9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -301,6 +301,13 @@ config SPI_FSL_ESPI
From MPC8536, 85xx platform uses the controller, and all P10xx,
P20xx, P30xx,P40xx, P50xx uses this controller.
+config SPI_MESON_SPIFC
+ tristate "Amlogic Meson SPIFC controller"
+ depends on ARCH_MESON || COMPILE_TEST
+ help
+ This enables master mode support for the SPIFC (SPI flash
+ controller) available in Amlogic Meson SoCs.
+
config SPI_OC_TINY
tristate "OpenCores tiny SPI"
depends on GPIOLIB
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 78f24ca..9b8a747 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o
obj-$(CONFIG_SPI_GPIO) += spi-gpio.o
obj-$(CONFIG_SPI_IMX) += spi-imx.o
obj-$(CONFIG_SPI_LM70_LLP) += spi-lm70llp.o
+obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o
obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
obj-$(CONFIG_SPI_MPC52xx_PSC) += spi-mpc52xx-psc.o
obj-$(CONFIG_SPI_MPC52xx) += spi-mpc52xx.o
diff --git a/drivers/spi/spi-meson-spifc.c b/drivers/spi/spi-meson-spifc.c
new file mode 100644
index 0000000..68be0e0
--- /dev/null
+++ b/drivers/spi/spi-meson-spifc.c
@@ -0,0 +1,461 @@
+/*
+ * Driver for Amlogic Meson SPI flash controller (SPIFC)
+ *
+ * Copyright (C) 2014 Beniamino Galvani <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+
+/* register map */
+#define REG_CMD 0x00
+#define REG_ADDR 0x04
+#define REG_CTRL 0x08
+#define REG_CTRL1 0x0c
+#define REG_STATUS 0x10
+#define REG_CTRL2 0x14
+#define REG_CLOCK 0x18
+#define REG_USER 0x1c
+#define REG_USER1 0x20
+#define REG_USER2 0x24
+#define REG_USER3 0x28
+#define REG_USER4 0x2c
+#define REG_SLAVE 0x30
+#define REG_SLAVE1 0x34
+#define REG_SLAVE2 0x38
+#define REG_SLAVE3 0x3c
+#define REG_C0 0x40
+#define REG_B8 0x60
+#define REG_MAX 0x7c
+
+/* register fields */
+#define CMD_USER BIT(18)
+#define CTRL_ENABLE_AHB BIT(17)
+#define CLOCK_SOURCE BIT(31)
+#define CLOCK_DIV_SHIFT 12
+#define CLOCK_DIV_MASK (0x3f << CLOCK_DIV_SHIFT)
+#define CLOCK_CNT_HIGH_SHIFT 6
+#define CLOCK_CNT_HIGH_MASK (0x3f << CLOCK_CNT_HIGH_SHIFT)
+#define CLOCK_CNT_LOW_SHIFT 0
+#define CLOCK_CNT_LOW_MASK (0x3f << CLOCK_CNT_LOW_SHIFT)
+#define USER_DIN_EN_MS BIT(0)
+#define USER_CMP_MODE BIT(2)
+#define USER_UC_DOUT_SEL BIT(27)
+#define USER_UC_DIN_SEL BIT(28)
+#define USER_UC_MASK ((BIT(5) - 1) << 27)
+#define USER1_BN_UC_DOUT_SHIFT 17
+#define USER1_BN_UC_DOUT_MASK (0xff << 16)
+#define USER1_BN_UC_DIN_SHIFT 8
+#define USER1_BN_UC_DIN_MASK (0xff << 8)
+#define USER4_CS_ACT BIT(30)
+#define SLAVE_TRST_DONE BIT(4)
+#define SLAVE_OP_MODE BIT(30)
+#define SLAVE_SW_RST BIT(31)
+
+#define SPIFC_BUFFER_SIZE 64
+
+/**
+ * struct meson_spifc
+ * @master: the SPI master
+ * @regmap: regmap for device registers
+ * @clk: input clock of the built-in baud rate generator
+ * @device: the device structure
+ */
+struct meson_spifc {
+ struct spi_master *master;
+ struct regmap *regmap;
+ struct clk *clk;
+ struct device *dev;
+};
+
+static struct regmap_config spifc_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = REG_MAX,
+};
+
+/**
+ * meson_spifc_wait_ready() - wait for the current operation to terminate
+ * @spifc: the Meson SPI device
+ * Return: 0 on success, a negative value on error
+ */
+static int meson_spifc_wait_ready(struct meson_spifc *spifc)
+{
+ unsigned long deadline = jiffies + msecs_to_jiffies(5);
+ u32 data;
+
+ do {
+ regmap_read(spifc->regmap, REG_SLAVE, &data);
+ if (data & SLAVE_TRST_DONE)
+ return 0;
+ cond_resched();
+ } while (!time_after(jiffies, deadline));
+
+ return -ETIMEDOUT;
+}
+
+/**
+ * meson_spifc_drain_buffer() - copy data from device buffer to memory
+ * @spifc: the Meson SPI device
+ * @buf: the destination buffer
+ * @len: number of bytes to copy
+ */
+static void meson_spifc_drain_buffer(struct meson_spifc *spifc, u8 *buf,
+ int len)
+{
+ u32 data;
+ int i = 0;
+
+ while (i < len) {
+ regmap_read(spifc->regmap, REG_C0 + i, &data);
+
+ if (len - i >= 4) {
+ *((u32 *)buf) = data;
+ buf += 4;
+ } else {
+ memcpy(buf, &data, len - i);
+ break;
+ }
+ i += 4;
+ }
+}
+
+/**
+ * meson_spifc_fill_buffer() - copy data from memory to device buffer
+ * @spifc: the Meson SPI device
+ * @buf: the source buffer
+ * @len: number of bytes to copy
+ */
+static void meson_spifc_fill_buffer(struct meson_spifc *spifc, const u8 *buf,
+ int len)
+{
+ u32 data;
+ int i = 0;
+
+ while (i < len) {
+ if (len - i >= 4)
+ data = *(u32 *)buf;
+ else
+ memcpy(&data, buf, len - i);
+
+ regmap_write(spifc->regmap, REG_C0 + i, data);
+
+ buf += 4;
+ i += 4;
+ }
+}
+
+/**
+ * meson_spifc_setup_speed() - program the clock divider
+ * @spifc: the Meson SPI device
+ * @speed: desired speed in Hz
+ */
+void meson_spifc_setup_speed(struct meson_spifc *spifc, u32 speed)
+{
+ unsigned long parent, value;
+ int n;
+
+ parent = clk_get_rate(spifc->clk);
+ n = max_t(int, parent / speed - 1, 1);
+
+ dev_dbg(spifc->dev, "parent %lu, speed %u, n %d\n", parent,
+ speed, n);
+
+ value = (n << CLOCK_DIV_SHIFT) & CLOCK_DIV_MASK;
+ value |= (n << CLOCK_CNT_LOW_SHIFT) & CLOCK_CNT_LOW_MASK;
+ value |= (((n + 1) / 2 - 1) << CLOCK_CNT_HIGH_SHIFT) &
+ CLOCK_CNT_HIGH_MASK;
+
+ regmap_write(spifc->regmap, REG_CLOCK, value);
+}
+
+/**
+ * meson_spifc_txrx() - transfer a chunk of data
+ * @spifc: the Meson SPI device
+ * @xfer: the current SPI transfer
+ * @offset: offset of the data to transfer
+ * @len: length of the data to transfer
+ * @last_xfer: whether this is the last transfer of the message
+ * @last_chunk: whether this is the last chunk of the transfer
+ * Return: 0 on success, a negative value on error
+ */
+static int meson_spifc_txrx(struct meson_spifc *spifc,
+ struct spi_transfer *xfer,
+ int offset, int len, bool last_xfer,
+ bool last_chunk)
+{
+ bool keep_cs = true;
+ int ret;
+
+ if (xfer->tx_buf)
+ meson_spifc_fill_buffer(spifc, xfer->tx_buf + offset, len);
+
+ /* enable DOUT stage */
+ regmap_update_bits(spifc->regmap, REG_USER, USER_UC_MASK,
+ USER_UC_DOUT_SEL);
+ regmap_write(spifc->regmap, REG_USER1,
+ (8 * len - 1) << USER1_BN_UC_DOUT_SHIFT);
+
+ /* enable data input during DOUT */
+ regmap_update_bits(spifc->regmap, REG_USER, USER_DIN_EN_MS,
+ USER_DIN_EN_MS);
+
+ if (last_chunk) {
+ if (last_xfer)
+ keep_cs = xfer->cs_change;
+ else
+ keep_cs = !xfer->cs_change;
+ }
+
+ regmap_update_bits(spifc->regmap, REG_USER4, USER4_CS_ACT,
+ keep_cs ? USER4_CS_ACT : 0);
+
+ /* clear transition done bit */
+ regmap_update_bits(spifc->regmap, REG_SLAVE, SLAVE_TRST_DONE, 0);
+ /* start transfer */
+ regmap_update_bits(spifc->regmap, REG_CMD, CMD_USER, CMD_USER);
+
+ ret = meson_spifc_wait_ready(spifc);
+
+ if (!ret && xfer->rx_buf)
+ meson_spifc_drain_buffer(spifc, xfer->rx_buf + offset, len);
+
+ return ret;
+}
+
+/**
+ * meson_spifc_transfer_one() - perform a single transfer
+ * @master: the SPI master
+ * @spi: the SPI device
+ * @xfer: the current SPI transfer
+ * Return: 0 on success, a negative value on error
+ */
+static int meson_spifc_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+ int len, done = 0, ret = 0;
+
+ meson_spifc_setup_speed(spifc, xfer->speed_hz);
+
+ regmap_update_bits(spifc->regmap, REG_CTRL, CTRL_ENABLE_AHB, 0);
+
+ while (done < xfer->len && !ret) {
+ len = min_t(int, xfer->len - done, SPIFC_BUFFER_SIZE);
+ ret = meson_spifc_txrx(spifc, xfer, done, len,
+ xfer->last, done + len >= xfer->len);
+ done += len;
+ }
+
+ regmap_update_bits(spifc->regmap, REG_CTRL, CTRL_ENABLE_AHB,
+ CTRL_ENABLE_AHB);
+
+ return ret;
+}
+
+/**
+ * meson_spifc_hw_init() - reset and initialize the SPI controller
+ * @spifc: the Meson SPI device
+ */
+static void meson_spifc_hw_init(struct meson_spifc *spifc)
+{
+ /* reset device */
+ regmap_update_bits(spifc->regmap, REG_SLAVE, SLAVE_SW_RST,
+ SLAVE_SW_RST);
+ /* disable compatible mode */
+ regmap_update_bits(spifc->regmap, REG_USER, USER_CMP_MODE, 0);
+ /* set master mode */
+ regmap_update_bits(spifc->regmap, REG_SLAVE, SLAVE_OP_MODE, 0);
+}
+
+static int meson_spifc_probe(struct platform_device *pdev)
+{
+ struct spi_master *master;
+ struct meson_spifc *spifc;
+ struct resource *res;
+ void __iomem *base;
+ unsigned int rate;
+ int ret = 0;
+
+ master = spi_alloc_master(&pdev->dev, sizeof(struct meson_spifc));
+ if (!master)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, master);
+
+ spifc = spi_master_get_devdata(master);
+ spifc->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(spifc->dev, res);
+ if (IS_ERR(base)) {
+ ret = PTR_ERR(base);
+ goto out_err;
+ }
+
+ spifc->regmap = devm_regmap_init_mmio(spifc->dev, base,
+ &spifc_regmap_config);
+ if (IS_ERR(spifc->regmap)) {
+ ret = PTR_ERR(spifc->regmap);
+ goto out_err;
+ }
+
+ spifc->clk = devm_clk_get(spifc->dev, NULL);
+ if (IS_ERR(spifc->clk)) {
+ dev_err(spifc->dev, "missing clock\n");
+ ret = PTR_ERR(spifc->clk);
+ goto out_err;
+ }
+
+ ret = clk_prepare_enable(spifc->clk);
+ if (ret) {
+ dev_err(spifc->dev, "can't prepare clock\n");
+ goto out_err;
+ }
+
+ rate = clk_get_rate(spifc->clk);
+
+ master->num_chipselect = 1;
+ master->dev.of_node = pdev->dev.of_node;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->auto_runtime_pm = true;
+ master->transfer_one = meson_spifc_transfer_one;
+ master->min_speed_hz = rate >> 6;
+ master->max_speed_hz = rate >> 1;
+
+ meson_spifc_hw_init(spifc);
+
+ pm_runtime_set_active(spifc->dev);
+ pm_runtime_enable(spifc->dev);
+
+ ret = devm_spi_register_master(spifc->dev, master);
+ if (ret) {
+ dev_err(spifc->dev, "failed to register spi master\n");
+ goto out_clk;
+ }
+
+ return 0;
+out_clk:
+ clk_disable_unprepare(spifc->clk);
+out_err:
+ spi_master_put(master);
+ return ret;
+}
+
+static int meson_spifc_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+
+ pm_runtime_get_sync(&pdev->dev);
+ clk_disable_unprepare(spifc->clk);
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int meson_spifc_suspend(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+ int ret;
+
+ ret = spi_master_suspend(master);
+ if (ret)
+ return ret;
+
+ if (!pm_runtime_suspended(dev))
+ clk_disable_unprepare(spifc->clk);
+
+ return 0;
+}
+
+static int meson_spifc_resume(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+ int ret;
+
+ if (!pm_runtime_suspended(dev)) {
+ ret = clk_prepare_enable(spifc->clk);
+ if (ret)
+ return ret;
+ }
+
+ meson_spifc_hw_init(spifc);
+
+ ret = spi_master_resume(master);
+ if (ret)
+ clk_disable_unprepare(spifc->clk);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int meson_spifc_runtime_suspend(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+
+ clk_disable_unprepare(spifc->clk);
+
+ return 0;
+}
+
+static int meson_spifc_runtime_resume(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct meson_spifc *spifc = spi_master_get_devdata(master);
+
+ return clk_prepare_enable(spifc->clk);
+}
+#endif /* CONFIG_PM_RUNTIME */
+
+static const struct dev_pm_ops meson_spifc_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(meson_spifc_suspend, meson_spifc_resume)
+ SET_RUNTIME_PM_OPS(meson_spifc_runtime_suspend,
+ meson_spifc_runtime_resume,
+ NULL)
+};
+
+static const struct of_device_id meson_spifc_dt_match[] = {
+ { .compatible = "amlogic,meson6-spifc", },
+ { },
+};
+
+static struct platform_driver meson_spifc_driver = {
+ .probe = meson_spifc_probe,
+ .remove = meson_spifc_remove,
+ .driver = {
+ .name = "meson-spifc",
+ .of_match_table = of_match_ptr(meson_spifc_dt_match),
+ .pm = &meson_spifc_pm_ops,
+ },
+};
+
+module_platform_driver(meson_spifc_driver);
+
+MODULE_AUTHOR("Beniamino Galvani <[email protected]>");
+MODULE_DESCRIPTION("Amlogic Meson SPIFC driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
On Mon, Nov 17, 2014 at 11:47:48PM +0100, Beniamino Galvani wrote:
> + list_for_each_entry(xfer, &master->cur_msg->transfers, transfer_list) {
> + xfer->last = list_is_last(&xfer->transfer_list,
> + &master->cur_msg->transfers);
> + }
It's incredibly sad to iterate through the entire list in order to find
the last entry, especially given that it's a doubly linked list and this
is a bit of a hot path. We should look at the previous entry for the
list head instead, or perhaps better yet by doing this as part of
spi_validate() which already itereates over the entire list and is where
we do other similar fixups.
Though looking at this I'm not sure that a flag is the best approach at
all - why not just have the driver call list_is_last() in the transfer
function or ideally provide an inline function that does that so that we
can change the implementation later?
On Mon, Nov 17, 2014 at 11:47:50PM +0100, Beniamino Galvani wrote:
> This is a driver for the Amlogic Meson SPIFC (SPI flash controller),
> which is one of the two SPI controllers available on the SoC. It
> doesn't support DMA and has a 64-byte unified transmit/receive buffer.
This is all good.
On Tue, Nov 18, 2014 at 02:06:58PM +0000, Mark Brown wrote:
> It's incredibly sad to iterate through the entire list in order to find
> the last entry, especially given that it's a doubly linked list and this
> is a bit of a hot path. We should look at the previous entry for the
> list head instead, or perhaps better yet by doing this as part of
> spi_validate() which already itereates over the entire list and is where
> we do other similar fixups.
>
> Though looking at this I'm not sure that a flag is the best approach at
> all - why not just have the driver call list_is_last() in the transfer
> function or ideally provide an inline function that does that so that we
> can change the implementation later?
I didn't realize that the master structure passed to transfer_one()
has a reference to the current message and thus to the transfer
list. Then yes, the additional flag in the transfer structure probably
doesn't make much sense.
Would it be better to introduce something like:
static inline bool
spi_transfer_is_last(struct spi_master *master, struct spi_transfer *xfer)
{
return list_is_last(&xfer->transfer_list, &master->cur_msg->transfers);
}
or open code it in the driver?
Beniamino
On Tue, Nov 18, 2014 at 11:55:46PM +0100, Beniamino Galvani wrote:
> Would it be better to introduce something like:
> static inline bool
> spi_transfer_is_last(struct spi_master *master, struct spi_transfer *xfer)
> {
> return list_is_last(&xfer->transfer_list, &master->cur_msg->transfers);
> }
> or open code it in the driver?
Adding the helper function seems better, it means we can change the
implementation later.