* [PATCH v3 1/2] dt-bindings: Document the STM32 QSPI bindings
From: Ludovic Barre @ 2017-04-12 17:06 UTC (permalink / raw)
To: Cyrille Pitchen, Marek Vasut
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Alexandre Torgue, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492016785-24976-1-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>
From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
This patch adds documentation of device tree bindings for the STM32
QSPI controller.
Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
---
.../devicetree/bindings/mtd/stm32-quadspi.txt | 43 ++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
new file mode 100644
index 0000000..ddd18c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
@@ -0,0 +1,43 @@
+* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
+
+Required properties:
+- compatible: should be "st,stm32f469-qspi"
+- reg: the first contains the register location and length.
+ the second contains the memory mapping address and length
+- reg-names: should contain the reg names "qspi" "qspi_mm"
+- interrupts: should contain the interrupt for the device
+- clocks: the phandle of the clock needed by the QSPI controller
+- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
+
+Optional properties:
+- resets: must contain the phandle to the reset controller.
+
+A spi flash must be a child of the nor_flash node and could have some
+properties. Also see jedec,spi-nor.txt.
+
+Required properties:
+- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
+- spi-max-frequency: max frequency of spi bus
+
+Optional property:
+- spi-rx-bus-width: see ../spi/spi-bus.txt for the description
+
+Example:
+
+qspi: spi@a0001000 {
+ compatible = "st,stm32f469-qspi";
+ reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
+ reg-names = "qspi", "qspi_mm";
+ interrupts = <91>;
+ resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
+ clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_qspi0>;
+
+ flash@0 {
+ reg = <0>;
+ spi-rx-bus-width = <4>;
+ spi-max-frequency = <108000000>;
+ ...
+ };
+};
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Ludovic Barre @ 2017-04-12 17:06 UTC (permalink / raw)
To: Cyrille Pitchen, Marek Vasut
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Alexandre Torgue, Rob Herring, linux-mtd,
linux-kernel, devicetree
In-Reply-To: <1492016785-24976-1-git-send-email-ludovic.Barre@st.com>
From: Ludovic Barre <ludovic.barre@st.com>
The quadspi is a specialized communication interface targeting single,
dual or quad SPI Flash memories.
It can operate in any of the following modes:
-indirect mode: all the operations are performed using the quadspi
registers
-read memory-mapped mode: the external Flash memory is mapped to the
microcontroller address space and is seen by the system as if it was
an internal memory
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/stm32-quadspi.c | 693 ++++++++++++++++++++++++++++++++++++
3 files changed, 701 insertions(+)
create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 7252087..bfdfb1e 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
To compile this driver as a module, choose M here: the module
will be called intel-spi-platform.
+config SPI_STM32_QUADSPI
+ tristate "STM32 Quad SPI controller"
+ depends on ARCH_STM32
+ help
+ This enables support for the STM32 Quad SPI controller.
+ We only connect the NOR to this controller.
+
endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 72238a7..285aab8 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
obj-$(CONFIG_SPI_INTEL_SPI) += intel-spi.o
obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM) += intel-spi-platform.o
+obj-$(CONFIG_SPI_STM32_QUADSPI) += stm32-quadspi.o
\ No newline at end of file
diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
new file mode 100644
index 0000000..9e90dee
--- /dev/null
+++ b/drivers/mtd/spi-nor/stm32-quadspi.c
@@ -0,0 +1,693 @@
+/*
+ * stm32_quadspi.c
+ *
+ * Copyright (C) 2017, Ludovic Barre
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define QUADSPI_CR 0x00
+#define CR_EN BIT(0)
+#define CR_ABORT BIT(1)
+#define CR_DMAEN BIT(2)
+#define CR_TCEN BIT(3)
+#define CR_SSHIFT BIT(4)
+#define CR_DFM BIT(6)
+#define CR_FSEL BIT(7)
+#define CR_FTHRES_SHIFT 8
+#define CR_FTHRES_MASK GENMASK(12, 8)
+#define CR_FTHRES(n) (((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
+#define CR_TEIE BIT(16)
+#define CR_TCIE BIT(17)
+#define CR_FTIE BIT(18)
+#define CR_SMIE BIT(19)
+#define CR_TOIE BIT(20)
+#define CR_PRESC_SHIFT 24
+#define CR_PRESC_MASK GENMASK(31, 24)
+#define CR_PRESC(n) (((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
+
+#define QUADSPI_DCR 0x04
+#define DCR_CSHT_SHIFT 8
+#define DCR_CSHT_MASK GENMASK(10, 8)
+#define DCR_CSHT(n) (((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
+#define DCR_FSIZE_SHIFT 16
+#define DCR_FSIZE_MASK GENMASK(20, 16)
+#define DCR_FSIZE(n) (((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
+
+#define QUADSPI_SR 0x08
+#define SR_TEF BIT(0)
+#define SR_TCF BIT(1)
+#define SR_FTF BIT(2)
+#define SR_SMF BIT(3)
+#define SR_TOF BIT(4)
+#define SR_BUSY BIT(5)
+#define SR_FLEVEL_SHIFT 8
+#define SR_FLEVEL_MASK GENMASK(13, 8)
+
+#define QUADSPI_FCR 0x0c
+#define FCR_CTCF BIT(1)
+
+#define QUADSPI_DLR 0x10
+
+#define QUADSPI_CCR 0x14
+#define CCR_INST_SHIFT 0
+#define CCR_INST_MASK GENMASK(7, 0)
+#define CCR_INST(n) (((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
+#define CCR_IMODE_NONE (0 << 8)
+#define CCR_IMODE_1 (1 << 8)
+#define CCR_IMODE_2 (2 << 8)
+#define CCR_IMODE_4 (3 << 8)
+#define CCR_ADMODE_NONE (0 << 10)
+#define CCR_ADMODE_1 (1 << 10)
+#define CCR_ADMODE_2 (2 << 10)
+#define CCR_ADMODE_4 (3 << 10)
+#define CCR_ADSIZE_SHIFT 12
+#define CCR_ADSIZE_MASK GENMASK(13, 12)
+#define CCR_ADSIZE(n) (((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
+#define CCR_ABMODE_NONE (0 << 14)
+#define CCR_ABMODE_1 (1 << 14)
+#define CCR_ABMODE_2 (2 << 14)
+#define CCR_ABMODE_4 (3 << 14)
+#define CCR_ABSIZE_8 (0 << 16)
+#define CCR_ABSIZE_16 (1 << 16)
+#define CCR_ABSIZE_24 (2 << 16)
+#define CCR_ABSIZE_32 (3 << 16)
+#define CCR_DCYC_SHIFT 18
+#define CCR_DCYC_MASK GENMASK(22, 18)
+#define CCR_DCYC(n) (((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
+#define CCR_DMODE_NONE (0 << 24)
+#define CCR_DMODE_1 (1 << 24)
+#define CCR_DMODE_2 (2 << 24)
+#define CCR_DMODE_4 (3 << 24)
+#define CCR_FMODE_INDW (0 << 26)
+#define CCR_FMODE_INDR (1 << 26)
+#define CCR_FMODE_APM (2 << 26)
+#define CCR_FMODE_MM (3 << 26)
+
+#define QUADSPI_AR 0x18
+#define QUADSPI_ABR 0x1c
+#define QUADSPI_DR 0x20
+#define QUADSPI_PSMKR 0x24
+#define QUADSPI_PSMAR 0x28
+#define QUADSPI_PIR 0x2c
+#define QUADSPI_LPTR 0x30
+#define LPTR_DFT_TIMEOUT 0x10
+
+#define FSIZE_VAL(size) (__fls(size) - 1)
+
+#define STM32_MAX_MMAP_SZ SZ_256M
+#define STM32_MAX_NORCHIP 2
+
+#define STM32_QSPI_FIFO_TIMEOUT_US 30000
+#define STM32_QSPI_BUSY_TIMEOUT_US 100000
+
+struct stm32_qspi_flash {
+ struct spi_nor nor;
+ u32 cs;
+ u32 fsize;
+ u32 presc;
+ u32 read_mode;
+ struct stm32_qspi *qspi;
+};
+
+struct stm32_qspi {
+ struct device *dev;
+ void __iomem *io_base;
+ void __iomem *mm_base;
+ resource_size_t mm_size;
+ u32 nor_num;
+ struct clk *clk;
+ u32 clk_rate;
+ struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
+ struct completion cmd_completion;
+
+ /*
+ * to protect device configuration, could be different between
+ * 2 flash access (bk1, bk2)
+ */
+ struct mutex lock;
+};
+
+struct stm32_qspi_cmd {
+ u8 addr_width;
+ u8 dummy;
+ bool tx_data;
+ u8 opcode;
+ u32 framemode;
+ u32 qspimode;
+ u32 addr;
+ size_t len;
+ void *buf;
+};
+
+static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
+{
+ u32 cr;
+ int err = 0;
+
+ if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
+ return 0;
+
+ reinit_completion(&qspi->cmd_completion);
+ cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+ writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
+
+ if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
+ msecs_to_jiffies(1000)))
+ err = -ETIMEDOUT;
+
+ writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+ return err;
+}
+
+static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
+{
+ u32 sr;
+
+ return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
+ !(sr & SR_BUSY), 10,
+ STM32_QSPI_BUSY_TIMEOUT_US);
+}
+
+static void stm32_qspi_set_framemode(struct spi_nor *nor,
+ struct stm32_qspi_cmd *cmd, bool read)
+{
+ u32 dmode = CCR_DMODE_1;
+
+ cmd->framemode = CCR_IMODE_1;
+
+ if (read) {
+ switch (nor->flash_read) {
+ case SPI_NOR_NORMAL:
+ case SPI_NOR_FAST:
+ dmode = CCR_DMODE_1;
+ break;
+ case SPI_NOR_DUAL:
+ dmode = CCR_DMODE_2;
+ break;
+ case SPI_NOR_QUAD:
+ dmode = CCR_DMODE_4;
+ break;
+ }
+ }
+
+ cmd->framemode |= cmd->tx_data ? dmode : 0;
+ cmd->framemode |= cmd->addr_width ? CCR_ADMODE_1 : 0;
+}
+
+static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
+{
+ *val = readb_relaxed(addr);
+}
+
+static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
+{
+ writeb_relaxed(*val, addr);
+}
+
+static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
+ const struct stm32_qspi_cmd *cmd)
+{
+ void (*tx_fifo)(u8 *, void __iomem *);
+ u32 len = cmd->len, sr;
+ u8 *buf = cmd->buf;
+ int ret;
+
+ if (cmd->qspimode == CCR_FMODE_INDW)
+ tx_fifo = stm32_qspi_write_fifo;
+ else
+ tx_fifo = stm32_qspi_read_fifo;
+
+ while (len--) {
+ ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
+ sr, (sr & SR_FTF), 10,
+ STM32_QSPI_FIFO_TIMEOUT_US);
+ if (ret) {
+ dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
+ break;
+ }
+ tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
+ }
+
+ return ret;
+}
+
+static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
+ const struct stm32_qspi_cmd *cmd)
+{
+ memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
+ return 0;
+}
+
+static int stm32_qspi_tx(struct stm32_qspi *qspi,
+ const struct stm32_qspi_cmd *cmd)
+{
+ if (!cmd->tx_data)
+ return 0;
+
+ if (cmd->qspimode == CCR_FMODE_MM)
+ return stm32_qspi_tx_mm(qspi, cmd);
+
+ return stm32_qspi_tx_poll(qspi, cmd);
+}
+
+static int stm32_qspi_send(struct stm32_qspi_flash *flash,
+ const struct stm32_qspi_cmd *cmd)
+{
+ struct stm32_qspi *qspi = flash->qspi;
+ u32 ccr, dcr, cr;
+ int err;
+
+ err = stm32_qspi_wait_nobusy(qspi);
+ if (err)
+ goto abort;
+
+ dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
+ dcr |= DCR_FSIZE(flash->fsize);
+ writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
+
+ cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+ cr &= ~CR_PRESC_MASK & ~CR_FSEL;
+ cr |= CR_PRESC(flash->presc);
+ cr |= flash->cs ? CR_FSEL : 0;
+ writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+ if (cmd->tx_data)
+ writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
+
+ ccr = cmd->framemode | cmd->qspimode;
+
+ if (cmd->dummy)
+ ccr |= CCR_DCYC(cmd->dummy);
+
+ if (cmd->addr_width)
+ ccr |= CCR_ADSIZE(cmd->addr_width - 1);
+
+ ccr |= CCR_INST(cmd->opcode);
+ writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
+
+ if (cmd->addr_width && cmd->qspimode != CCR_FMODE_MM)
+ writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
+
+ err = stm32_qspi_tx(qspi, cmd);
+ if (err)
+ goto abort;
+
+ if (cmd->qspimode != CCR_FMODE_MM) {
+ err = stm32_qspi_wait_cmd(qspi);
+ if (err)
+ goto abort;
+ writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
+ }
+
+ return err;
+
+abort:
+ cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
+ writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+ dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
+ return err;
+}
+
+static int stm32_qspi_read_reg(struct spi_nor *nor,
+ u8 opcode, u8 *buf, int len)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct device *dev = flash->qspi->dev;
+ struct stm32_qspi_cmd cmd;
+
+ dev_dbg(dev, "read_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = opcode;
+ cmd.tx_data = true;
+ cmd.len = len;
+ cmd.buf = buf;
+ cmd.qspimode = CCR_FMODE_INDR;
+
+ stm32_qspi_set_framemode(nor, &cmd, false);
+
+ return stm32_qspi_send(flash, &cmd);
+}
+
+static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
+ u8 *buf, int len)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct device *dev = flash->qspi->dev;
+ struct stm32_qspi_cmd cmd;
+
+ dev_dbg(dev, "write_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = opcode;
+ cmd.tx_data = !!(buf && len > 0);
+ cmd.len = len;
+ cmd.buf = buf;
+ cmd.qspimode = CCR_FMODE_INDW;
+
+ stm32_qspi_set_framemode(nor, &cmd, false);
+
+ return stm32_qspi_send(flash, &cmd);
+}
+
+static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
+ u_char *buf)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct stm32_qspi *qspi = flash->qspi;
+ struct stm32_qspi_cmd cmd;
+ int err;
+
+ dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
+ nor->read_opcode, buf, (u32)from, len);
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = nor->read_opcode;
+ cmd.addr_width = nor->addr_width;
+ cmd.addr = (u32)from;
+ cmd.tx_data = true;
+ cmd.dummy = nor->read_dummy;
+ cmd.len = len;
+ cmd.buf = buf;
+ cmd.qspimode = flash->read_mode;
+
+ stm32_qspi_set_framemode(nor, &cmd, true);
+ err = stm32_qspi_send(flash, &cmd);
+
+ return err ? err : len;
+}
+
+static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+ const u_char *buf)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct device *dev = flash->qspi->dev;
+ struct stm32_qspi_cmd cmd;
+ int err;
+
+ dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
+ nor->program_opcode, buf, (u32)to, len);
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = nor->program_opcode;
+ cmd.addr_width = nor->addr_width;
+ cmd.addr = (u32)to;
+ cmd.tx_data = true;
+ cmd.len = len;
+ cmd.buf = (void *)buf;
+ cmd.qspimode = CCR_FMODE_INDW;
+
+ stm32_qspi_set_framemode(nor, &cmd, false);
+ err = stm32_qspi_send(flash, &cmd);
+
+ return err ? err : len;
+}
+
+static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct device *dev = flash->qspi->dev;
+ struct stm32_qspi_cmd cmd;
+
+ dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = nor->erase_opcode;
+ cmd.addr_width = nor->addr_width;
+ cmd.addr = (u32)offs;
+ cmd.qspimode = CCR_FMODE_INDW;
+
+ stm32_qspi_set_framemode(nor, &cmd, false);
+
+ return stm32_qspi_send(flash, &cmd);
+}
+
+static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
+{
+ struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
+ u32 cr, sr, fcr = 0;
+
+ cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+ sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
+
+ if ((cr & CR_TCIE) && (sr & SR_TCF)) {
+ /* tx complete */
+ fcr |= FCR_CTCF;
+ complete(&qspi->cmd_completion);
+ } else {
+ dev_info_ratelimited(qspi->dev, "spurious interrupt\n");
+ }
+
+ writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
+
+ return IRQ_HANDLED;
+}
+
+static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct stm32_qspi *qspi = flash->qspi;
+
+ mutex_lock(&qspi->lock);
+ return 0;
+}
+
+static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+ struct stm32_qspi_flash *flash = nor->priv;
+ struct stm32_qspi *qspi = flash->qspi;
+
+ mutex_unlock(&qspi->lock);
+}
+
+static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
+ struct device_node *np)
+{
+ u32 width, flash_read, presc, cs_num, max_rate = 0;
+ struct stm32_qspi_flash *flash;
+ struct mtd_info *mtd;
+ int ret;
+
+ of_property_read_u32(np, "reg", &cs_num);
+ if (cs_num >= STM32_MAX_NORCHIP)
+ return -EINVAL;
+
+ of_property_read_u32(np, "spi-max-frequency", &max_rate);
+ if (!max_rate)
+ return -EINVAL;
+
+ presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
+
+ if (of_property_read_u32(np, "spi-rx-bus-width", &width))
+ width = 1;
+
+ if (width == 4)
+ flash_read = SPI_NOR_QUAD;
+ else if (width == 2)
+ flash_read = SPI_NOR_DUAL;
+ else if (width == 1)
+ flash_read = SPI_NOR_NORMAL;
+ else
+ return -EINVAL;
+
+ flash = &qspi->flash[cs_num];
+ flash->qspi = qspi;
+ flash->cs = cs_num;
+ flash->presc = presc;
+
+ flash->nor.dev = qspi->dev;
+ spi_nor_set_flash_node(&flash->nor, np);
+ flash->nor.priv = flash;
+ mtd = &flash->nor.mtd;
+ mtd->priv = &flash->nor;
+
+ flash->nor.read = stm32_qspi_read;
+ flash->nor.write = stm32_qspi_write;
+ flash->nor.erase = stm32_qspi_erase;
+ flash->nor.read_reg = stm32_qspi_read_reg;
+ flash->nor.write_reg = stm32_qspi_write_reg;
+ flash->nor.prepare = stm32_qspi_prep;
+ flash->nor.unprepare = stm32_qspi_unprep;
+
+ writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
+
+ writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
+ | CR_EN, qspi->io_base + QUADSPI_CR);
+
+ /*
+ * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
+ * which define the size of nor flash.
+ * if fsize is NULL, the controller can't sent spi-nor command.
+ * set a temporary value just to discover the nor flash with
+ * "spi_nor_scan". After, the right value (mtd->size) can be set.
+ */
+ flash->fsize = FSIZE_VAL(SZ_1K);
+
+ ret = spi_nor_scan(&flash->nor, NULL, flash_read);
+ if (ret) {
+ dev_err(qspi->dev, "device scan failed\n");
+ return ret;
+ }
+
+ flash->fsize = FSIZE_VAL(mtd->size);
+
+ flash->read_mode = CCR_FMODE_MM;
+ if (mtd->size > qspi->mm_size)
+ flash->read_mode = CCR_FMODE_INDR;
+
+ writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
+
+ ret = mtd_device_register(mtd, NULL, 0);
+ if (ret) {
+ dev_err(qspi->dev, "mtd device parse failed\n");
+ return ret;
+ }
+
+ dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
+ flash->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
+
+ return 0;
+}
+
+static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
+{
+ int i;
+
+ for (i = 0; i < STM32_MAX_NORCHIP; i++) {
+ if (qspi->flash[i].qspi)
+ mtd_device_unregister(&qspi->flash[i].nor.mtd);
+ }
+}
+
+static int stm32_qspi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *flash_np;
+ struct reset_control *rstc;
+ struct stm32_qspi *qspi;
+ struct resource *res;
+ int ret, irq;
+
+ qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
+ if (!qspi)
+ return -ENOMEM;
+
+ qspi->nor_num = of_get_child_count(dev->of_node);
+ if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
+ return -ENODEV;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
+ qspi->io_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(qspi->io_base))
+ return PTR_ERR(qspi->io_base);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
+ qspi->mm_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(qspi->mm_base))
+ return PTR_ERR(qspi->mm_base);
+
+ qspi->mm_size = resource_size(res);
+
+ irq = platform_get_irq(pdev, 0);
+ ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
+ dev_name(dev), qspi);
+ if (ret) {
+ dev_err(dev, "failed to request irq\n");
+ return ret;
+ }
+
+ init_completion(&qspi->cmd_completion);
+
+ qspi->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(qspi->clk))
+ return PTR_ERR(qspi->clk);
+
+ qspi->clk_rate = clk_get_rate(qspi->clk);
+ if (!qspi->clk_rate)
+ return -EINVAL;
+
+ ret = clk_prepare_enable(qspi->clk);
+ if (ret) {
+ dev_err(dev, "can not enable the clock\n");
+ return ret;
+ }
+
+ rstc = devm_reset_control_get(dev, NULL);
+ if (!IS_ERR(rstc)) {
+ reset_control_assert(rstc);
+ udelay(2);
+ reset_control_deassert(rstc);
+ }
+
+ qspi->dev = dev;
+ platform_set_drvdata(pdev, qspi);
+ mutex_init(&qspi->lock);
+
+ for_each_available_child_of_node(dev->of_node, flash_np) {
+ ret = stm32_qspi_flash_setup(qspi, flash_np);
+ if (ret) {
+ dev_err(dev, "unable to setup flash chip\n");
+ goto err_flash;
+ }
+ }
+
+ return 0;
+
+err_flash:
+ mutex_destroy(&qspi->lock);
+ stm32_qspi_mtd_free(qspi);
+
+ clk_disable_unprepare(qspi->clk);
+ return ret;
+}
+
+static int stm32_qspi_remove(struct platform_device *pdev)
+{
+ struct stm32_qspi *qspi = platform_get_drvdata(pdev);
+
+ /* disable qspi */
+ writel_relaxed(0, qspi->io_base + QUADSPI_CR);
+
+ stm32_qspi_mtd_free(qspi);
+ mutex_destroy(&qspi->lock);
+
+ clk_disable_unprepare(qspi->clk);
+ return 0;
+}
+
+static const struct of_device_id stm32_qspi_match[] = {
+ {.compatible = "st,stm32f469-qspi"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, stm32_qspi_match);
+
+static struct platform_driver stm32_qspi_driver = {
+ .probe = stm32_qspi_probe,
+ .remove = stm32_qspi_remove,
+ .driver = {
+ .name = "stm32-quadspi",
+ .of_match_table = stm32_qspi_match,
+ },
+};
+module_platform_driver(stm32_qspi_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Thierry Reding @ 2017-04-12 17:11 UTC (permalink / raw)
To: Ralph Sennhauser
Cc: Andrew Lunn, Linus Walleij, Alexandre Courbot, Rob Herring,
Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170409180931.4884-2-ralph.sennhauser@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 17676 bytes --]
On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> From: Andrew Lunn <andrew@lunn.ch>
>
> Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> and off period. This can be modelled as a PWM.
>
> However, there are only two sets of PWM configuration registers for
> all the GPIO lines. This driver simply allows a single GPIO line per
> GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
>
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the GPIO driver. Thus the GPIO driver has been
> extended with a PWM driver.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> URL: https://patchwork.ozlabs.org/patch/427287/
> URL: https://patchwork.ozlabs.org/patch/427295/
> [Ralph Sennhauser:
> * Port forward
> * Merge PWM portion into gpio-mvebu.c
> * Switch to atomic PWM API
> * Add new compatible string marvell,armada-370-xp-gpio
> * Update and merge documentation patch
> * Update MAINTAINERS]
> Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> ---
> .../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
> MAINTAINERS | 2 +
> drivers/gpio/gpio-mvebu.c | 324 ++++++++++++++++++++-
> 3 files changed, 346 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> index a6f3bec..fe49e9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> @@ -38,6 +38,24 @@ Required properties:
> - #gpio-cells: Should be two. The first cell is the pin number. The
> second cell is reserved for flags, unused at the moment.
>
> +Optional properties:
> +
> +In order to use the gpio lines in PWM mode, some additional optional
> +properties are required. Only Armada 370 and XP support these properties.
> +
> +- compatible: Must contain "marvell,armada-370-xp-gpio"
> +
> +- reg: an additional register set is needed, for the GPIO Blink
> + Counter on/off registers.
> +
> +- reg-names: Must contain an entry "pwm" corresponding to the
> + additional register range needed for pwm operation.
> +
> +- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> + second cell is the period in nanoseconds.
> +
> +- clocks: Must be a phandle to the clock for the gpio controller.
> +
> Example:
>
> gpio0: gpio@d0018100 {
> @@ -51,3 +69,17 @@ Example:
> #interrupt-cells = <2>;
> interrupts = <16>, <17>, <18>, <19>;
> };
> +
> + gpio1: gpio@18140 {
> + compatible = "marvell,armada-370-xp-gpio";
> + reg = <0x18140 0x40>, <0x181c8 0x08>;
> + reg-names = "gpio", "pwm";
> + ngpios = <17>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #pwm-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <87>, <88>, <89>;
> + clocks = <&coreclk 0>;
> + };
This is going to need an Acked-by from one of the device tree
maintainers. Rob and devicetree@vger.kernel.org are on Cc, but I suspect
nobody might look for the binding change "hidden" in this patch.
Maybe best to split this off into a separate patch, or explicitly ping
Rob to look at this patch.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58b3a22..19382f5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10295,6 +10295,8 @@ F: include/linux/pwm.h
> F: drivers/pwm/
> F: drivers/video/backlight/pwm_bl.c
> F: include/linux/pwm_backlight.h
> +F: drivers/gpio/gpio-mvebu.c
> +F: Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
>
> PXA2xx/PXA3xx SUPPORT
> M: Daniel Mack <daniel@zonque.org>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index fae4db6..e310951 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -42,22 +42,34 @@
> #include <linux/io.h>
> #include <linux/of_irq.h>
> #include <linux/of_device.h>
> +#include <linux/pwm.h>
> #include <linux/clk.h>
> #include <linux/pinctrl/consumer.h>
> #include <linux/irqchip/chained_irq.h>
> +#include <linux/platform_device.h>
> #include <linux/bitops.h>
>
> +#include "gpiolib.h"
> +
> /*
> * GPIO unit register offsets.
> */
> -#define GPIO_OUT_OFF 0x0000
> -#define GPIO_IO_CONF_OFF 0x0004
> -#define GPIO_BLINK_EN_OFF 0x0008
> -#define GPIO_IN_POL_OFF 0x000c
> -#define GPIO_DATA_IN_OFF 0x0010
> -#define GPIO_EDGE_CAUSE_OFF 0x0014
> -#define GPIO_EDGE_MASK_OFF 0x0018
> -#define GPIO_LEVEL_MASK_OFF 0x001c
> +#define GPIO_OUT_OFF 0x0000
> +#define GPIO_IO_CONF_OFF 0x0004
> +#define GPIO_BLINK_EN_OFF 0x0008
> +#define GPIO_IN_POL_OFF 0x000c
> +#define GPIO_DATA_IN_OFF 0x0010
> +#define GPIO_EDGE_CAUSE_OFF 0x0014
> +#define GPIO_EDGE_MASK_OFF 0x0018
> +#define GPIO_LEVEL_MASK_OFF 0x001c
> +#define GPIO_BLINK_CNT_SELECT_OFF 0x0020
> +
> +/*
> + * PWM register offsets.
> + */
> +#define PWM_BLINK_ON_DURATION_OFF 0x0
> +#define PWM_BLINK_OFF_DURATION_OFF 0x4
> +
>
> /* The MV78200 has per-CPU registers for edge mask and level mask */
> #define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
> @@ -78,6 +90,20 @@
>
> #define MVEBU_MAX_GPIO_PER_BANK 32
>
> +struct mvebu_pwm {
> + void __iomem *membase;
> + unsigned long clk_rate;
> + bool used;
I think you could've probably made this a little simpler by making bool
used into struct gpio_desc *gpio; and reference that rather than having
to convert with gpio_to_desc(). More on why I think that's important
below.
> + struct pwm_chip chip;
> + spinlock_t lock;
> + struct mvebu_gpio_chip *mvchip;
> +
> + /* Used to preserve GPIO/PWM registers across suspend/resume */
> + u32 blink_select;
> + u32 blink_on_duration;
> + u32 blink_off_duration;
> +};
> +
> struct mvebu_gpio_chip {
> struct gpio_chip chip;
> spinlock_t lock;
> @@ -87,6 +113,10 @@ struct mvebu_gpio_chip {
> struct irq_domain *domain;
> int soc_variant;
>
> + /* Used for PWM support */
> + struct clk *clk;
> + struct mvebu_pwm *mvpwm;
> +
> /* Used to preserve GPIO registers across suspend/resume */
> u32 out_reg;
> u32 io_conf_reg;
> @@ -110,6 +140,12 @@ static void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
> return mvchip->membase + GPIO_BLINK_EN_OFF;
> }
>
> +static void __iomem *mvebu_gpioreg_blink_counter_select(struct mvebu_gpio_chip
> + *mvchip)
> +{
> + return mvchip->membase + GPIO_BLINK_CNT_SELECT_OFF;
> +}
> +
> static void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
> {
> return mvchip->membase + GPIO_IO_CONF_OFF;
> @@ -181,6 +217,20 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
> }
>
> /*
> + * Functions returning addresses of individual registers for a given
> + * PWM controller.
> + */
> +static void __iomem *mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)
> +{
> + return mvpwm->membase + PWM_BLINK_ON_DURATION_OFF;
> +}
> +
> +static void __iomem *mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
> +{
> + return mvpwm->membase + PWM_BLINK_OFF_DURATION_OFF;
> +}
> +
> +/*
> * Functions implementing the gpio_chip methods
> */
> static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
> @@ -484,6 +534,243 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> +/*
> + * Functions implementing the pwm_chip methods
> + */
> +static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
> +{
> + return container_of(chip, struct mvebu_pwm, chip);
> +}
> +
> +static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> + struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
This assumes that the GPIO chip base and the PWM chip base are the same.
You make sure of that by setting the PWM chip base to be the same as the
GPIO chip base, which is the easy way out. It's also somewhat brittle
because some other PWM chip may have occupied the region that you want
to use. It's fairly unlikely, but I think you can very easily side-step
any issues by simply doing:
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
desc = gpio_to_desc(mvchip->chip.base + pwm->hwpwm);
Now that's somewhat complicated, but you only have to do it once if you
store the desc in mvpwm->gpio as pointed out above.
> + unsigned long flags;
> + int ret = 0;
> +
> + spin_lock_irqsave(&mvpwm->lock, flags);
> + if (mvpwm->used) {
> + ret = -EBUSY;
Then you can also easily use mvpwm->gpio to check for -EBUSY here.
> + } else {
> + if (!desc) {
> + ret = -ENODEV;
> + goto out;
> + }
> + ret = gpiod_request(desc, "mvebu-pwm");
> + if (ret)
> + goto out;
> +
> + ret = gpiod_direction_output(desc, 0);
> + if (ret) {
> + gpiod_free(desc);
> + goto out;
> + }
> +
> + mvpwm->used = true;
> + }
> +
> +out:
> + spin_unlock_irqrestore(&mvpwm->lock, flags);
> + return ret;
> +}
> +
> +static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> + struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&mvpwm->lock, flags);
> + gpiod_free(desc);
> + mvpwm->used = false;
> + spin_unlock_irqrestore(&mvpwm->lock, flags);
> +}
> +
> +static void mvebu_pwm_get_state(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + struct pwm_state *state) {
> +
> + struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> + struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> + unsigned long long val;
> + unsigned long flags;
> + u32 u;
> +
> + spin_lock_irqsave(&mvpwm->lock, flags);
> +
> + val = (unsigned long long)
> + readl_relaxed(mvebu_pwmreg_blink_on_duration);
> + val *= NSEC_PER_SEC;
> + do_div(val, mvpwm->clk_rate);
> + if (val > UINT_MAX)
> + state->duty_cycle = UINT_MAX;
> + else if (val)
> + state->duty_cycle = val;
> + else
> + state->duty_cycle = 1;
> +
> + val = (unsigned long long)
> + readl_relaxed(mvebu_pwmreg_blink_off_duration);
> + val *= NSEC_PER_SEC;
> + do_div(val, mvpwm->clk_rate);
> + if (val < state->duty_cycle) {
> + state->period = 1;
> + } else {
> + val -= state->duty_cycle;
> + if (val > UINT_MAX)
> + state->period = UINT_MAX;
> + else if (val)
> + state->period = val;
> + else
> + state->period = 1;
> + }
> +
> + u = readl_relaxed(mvebu_gpioreg_blink(mvchip));
> + if (u)
> + state->enabled = true;
> + else
> + state->enabled = false;
> +
> + spin_unlock_irqrestore(&mvpwm->lock, flags);
> +}
> +
> +static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + struct pwm_state *state)
> +{
> + struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> + struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> + unsigned long long val;
> + unsigned long flags;
> + unsigned int on, off;
> +
> + val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
> + do_div(val, NSEC_PER_SEC);
> + if (val > UINT_MAX)
> + return -EINVAL;
> + if (val)
> + on = val;
> + else
> + on = 1;
> +
> + val = (unsigned long long) mvpwm->clk_rate *
> + (state->period - state->duty_cycle);
> + do_div(val, NSEC_PER_SEC);
> + if (val > UINT_MAX)
> + return -EINVAL;
> + if (val)
> + off = val;
> + else
> + off = 1;
> +
> + spin_lock_irqsave(&mvpwm->lock, flags);
> +
> + writel_relaxed(on, mvebu_pwmreg_blink_on_duration(mvpwm));
> + writel_relaxed(off, mvebu_pwmreg_blink_off_duration(mvpwm));
> + if (state->enabled)
> + mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 1);
> + else
> + mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 0);
> +
> + spin_unlock_irqrestore(&mvpwm->lock, flags);
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops mvebu_pwm_ops = {
> + .request = mvebu_pwm_request,
> + .free = mvebu_pwm_free,
> + .get_state = mvebu_pwm_get_state,
> + .apply = mvebu_pwm_apply,
> + .owner = THIS_MODULE,
> +};
> +
> +static void __maybe_unused mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
> +{
> + struct mvebu_pwm *mvpwm = mvchip->mvpwm;
> +
> + mvpwm->blink_select =
> + readl_relaxed(mvebu_gpioreg_blink_counter_select(mvchip));
> + mvpwm->blink_on_duration =
> + readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
> + mvpwm->blink_off_duration =
> + readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
> +}
> +
> +static void __maybe_unused mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
> +{
> + struct mvebu_pwm *mvpwm = mvchip->mvpwm;
> +
> + writel_relaxed(mvpwm->blink_select,
> + mvebu_gpioreg_blink_counter_select(mvchip));
> + writel_relaxed(mvpwm->blink_on_duration,
> + mvebu_pwmreg_blink_on_duration(mvpwm));
> + writel_relaxed(mvpwm->blink_off_duration,
> + mvebu_pwmreg_blink_off_duration(mvpwm));
> +}
> +
> +static int mvebu_pwm_probe(struct platform_device *pdev,
> + struct mvebu_gpio_chip *mvchip,
> + int id)
> +{
> + struct device *dev = &pdev->dev;
> + struct mvebu_pwm *mvpwm;
> + struct resource *res;
> +
> + if (!of_device_is_compatible(mvchip->chip.of_node,
> + "marvell,armada-370-xp-gpio"))
> + return 0;
> + /*
> + * There are only two sets of PWM configuration registers for
> + * all the GPIO lines on those SoCs which this driver reserves
> + * for the first two GPIO chips. So if the resource is missing
> + * we can't treat it as an error.
> + */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
> + if (!res)
> + return 0;
> +
> + /*
> + * Use set A for lines of GPIO chip with id 0, B for GPIO chip
> + * with id 1. Don't allow further GPIO chips to be used for PWM.
> + */
> + if (id == 0)
> + writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
> + else if (id == 1)
> + writel_relaxed(U32_MAX,
> + mvebu_gpioreg_blink_counter_select(mvchip));
You could've just set a variable and then call writel_relaxed() after
the return -EINVAL below.
> + else
> + return -EINVAL;
> +
> + mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
> + if (!mvpwm)
> + return -ENOMEM;
> + mvchip->mvpwm = mvpwm;
> + mvpwm->mvchip = mvchip;
> +
> + mvpwm->membase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mvpwm->membase))
> + return PTR_ERR(mvpwm->membase);
> +
> + if (IS_ERR(mvchip->clk))
> + return PTR_ERR(mvchip->clk);
Maybe do this much earlier to avoid all the unnecessary register
accesses, allocations and mappings?
> +
> + mvpwm->clk_rate = clk_get_rate(mvchip->clk);
> + if (!mvpwm->clk_rate) {
> + dev_err(dev, "failed to get clock rate\n");
> + return -EINVAL;
> + }
> +
> + mvpwm->chip.dev = dev;
> + mvpwm->chip.ops = &mvebu_pwm_ops;
> + mvpwm->chip.base = mvchip->chip.base;
> + mvpwm->chip.npwm = mvchip->chip.ngpio;
I still would've done this differently. If you use this with a PWM user
you have to hook it up via DT anyway, so it doesn't matter whether you
specify the PWM index or the GPIO via some other property. The _only_
use-case where this might actually be an advantage is if you request a
PWM via the sysfs interface.
> + spin_lock_init(&mvpwm->lock);
> +
> + return pwmchip_add(&mvpwm->chip);
> +}
> +
> #ifdef CONFIG_DEBUG_FS
> #include <linux/seq_file.h>
>
> @@ -555,6 +842,10 @@ static const struct of_device_id mvebu_gpio_of_match[] = {
> .data = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
> },
> {
> + .compatible = "marvell,armada-370-xp-gpio",
> + .data = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
> + },
> + {
> /* sentinel */
> },
> };
> @@ -600,6 +891,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
> BUG();
> }
>
> + if (IS_ENABLED(CONFIG_PWM))
> + mvebu_pwm_suspend(mvchip);
> +
> return 0;
> }
>
> @@ -643,6 +937,9 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
> BUG();
> }
>
> + if (IS_ENABLED(CONFIG_PWM))
> + mvebu_pwm_resume(mvchip);
> +
> return 0;
> }
>
> @@ -654,7 +951,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
> struct resource *res;
> struct irq_chip_generic *gc;
> struct irq_chip_type *ct;
> - struct clk *clk;
> unsigned int ngpios;
> bool have_irqs;
> int soc_variant;
> @@ -688,10 +984,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
> return id;
> }
>
> - clk = devm_clk_get(&pdev->dev, NULL);
> + mvchip->clk = devm_clk_get(&pdev->dev, NULL);
> /* Not all SoCs require a clock.*/
> - if (!IS_ERR(clk))
> - clk_prepare_enable(clk);
> + if (!IS_ERR(mvchip->clk))
> + clk_prepare_enable(mvchip->clk);
>
> mvchip->soc_variant = soc_variant;
> mvchip->chip.label = dev_name(&pdev->dev);
> @@ -822,6 +1118,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
> mvchip);
> }
>
> + /* Armada 370/XP has simple PWM support for GPIO lines */
> + if (IS_ENABLED(CONFIG_PWM))
> + return mvebu_pwm_probe(pdev, mvchip, id);
> +
> return 0;
>
> err_domain:
> --
> 2.10.2
All of my comments are effectively of a bikeshed nature, so from a PWM
perspective this is:
Acked-by: Thierry Reding <thierry.reding@gmail.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] dt-bindings: add vendor prefix for NLT Technologies, Ltd.
From: Lucas Stach @ 2017-04-12 17:15 UTC (permalink / raw)
To: Thierry Reding, Rob Herring
Cc: Mark Rutland, devicetree, kernel, dri-devel, patchwork-lst
NLT technologies is the former NEC display business, but changed its
name to NLT Technologies when forming a joint venture with
Shenzhen AVIC OPTOELECTRONICS, Ltd.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4686f4bdaca0..b9edc2f4cb94 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -212,6 +212,7 @@ nexbox Nexbox
newhaven Newhaven Display International
ni National Instruments
nintendo Nintendo
+nlt NLT Technologies, Ltd.
nokia Nokia
nuvoton Nuvoton Technology Corporation
nvd New Vision Display
--
2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH 2/2] drm/panel: simple: add support for NLT NL192108AC18-02D
From: Lucas Stach @ 2017-04-12 17:15 UTC (permalink / raw)
To: Thierry Reding, Rob Herring
Cc: Mark Rutland, devicetree, kernel, dri-devel, patchwork-lst
In-Reply-To: <20170412171526.32393-1-l.stach@pengutronix.de>
This adds support for the NLT Technologies NL192108AC18-02D
15.6" LVDS FullHD TFT LCD panel, which can be supported
by the simple panel driver.
Timings are taken from the preliminary datasheet, as a final
one is not yet available.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
.../display/panel/nlt,nl192108ac18-02d.txt | 7 ++++++
drivers/gpu/drm/panel/panel-simple.c | 29 ++++++++++++++++++++++
2 files changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/nlt,nl192108ac18-02d.txt
diff --git a/Documentation/devicetree/bindings/display/panel/nlt,nl192108ac18-02d.txt b/Documentation/devicetree/bindings/display/panel/nlt,nl192108ac18-02d.txt
new file mode 100644
index 000000000000..edc34fbd2131
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/nlt,nl192108ac18-02d.txt
@@ -0,0 +1,7 @@
+NLT Technologies, Ltd. 15.6" FHD (1920x1080) LVDS TFT LCD panel
+
+Required properties:
+- compatible: should be "nlt,nl192108ac18-02d"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 89eb0422821c..888c3a22dcd9 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1346,6 +1346,32 @@ static const struct panel_desc netron_dy_e231732 = {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
};
+static const struct display_timing nlt_nl192108ac18_02d_timing = {
+ .pixelclock = { 130000000, 148350000, 163000000 },
+ .hactive = { 1920, 1920, 1920 },
+ .hfront_porch = { 80, 100, 100 },
+ .hback_porch = { 100, 120, 120 },
+ .hsync_len = { 50, 60, 60 },
+ .vactive = { 1080, 1080, 1080 },
+ .vfront_porch = { 12, 30, 30 },
+ .vback_porch = { 4, 10, 10 },
+ .vsync_len = { 4, 5, 5 },
+};
+
+static const struct panel_desc nlt_nl192108ac18_02d = {
+ .timings = &nlt_nl192108ac18_02d_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 344,
+ .height = 194,
+ },
+ .delay = {
+ .unprepare = 500,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+};
+
static const struct drm_display_mode nvd_9128_mode = {
.clock = 29500,
.hdisplay = 800,
@@ -1925,6 +1951,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "netron-dy,e231732",
.data = &netron_dy_e231732,
}, {
+ .compatible = "nlt,nl192108ac18-02d",
+ .data = &nlt_nl192108ac18_02d,
+ }, {
.compatible = "nvd,9128",
.data = &nvd_9128,
}, {
--
2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: [PATCH v5 0/4] gpio: mvebu: Add PWM fan support
From: Thierry Reding @ 2017-04-12 17:16 UTC (permalink / raw)
To: Ralph Sennhauser
Cc: Linus Walleij, Alexandre Courbot, Rob Herring, Mark Rutland,
Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Russell King, linux-pwm, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]
On Sun, Apr 09, 2017 at 08:09:26PM +0200, Ralph Sennhauser wrote:
> Hi Therry,
>
> Resending this as v5 with some minor changes since v4. What is missing is
> an ACK from you so Linus can merge the driver and Gregory the dts
> changes. For this driver to make it into 4.12 it would be nice to have
> it in next soon. I hope you can make some room in your schedule to have
> another look at this series.
>
> Thanks
> Ralph
>
> ---
>
> Notes:
>
> About npwm = 1:
> The only way I can think of to achieve that requires reading the
> GPIO line from the device tree. This would prevent a user to
> dynamically choose a line. Which is fine for the fan found on Mamba
> but let's take some development board with freely accessible GPIOs
> and suddenly we limit the use of this driver. Given the above, npwm
> = ngpio with only one usable at a time is a more accurate
I think "accurate" is perhaps not the word I'd choose. "npwm" is defined
as "number of PWMs controlled by this chip", and that's effectively just
the one. It's implied that all PWMs exposed by a chip can be used
concurrently.
Anyway, I can see how npwm = ngpio might be more convenient, and if that
is what you want to do, I don't feel strongly enough to object.
> description of the situation. The only downside is some "wasted"
> space.
>
> About the new compatible string:
> Orion was chosen for the SoC variant for the same reason as in
> commit 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on
> Armada XP").
> The "pwm" property remains optional for the new compatible string so
> the compatiple string "marvell,armada-370-xp-gpio" can be used by
> all and not just the first two GPIO chips. A property to select "Set
> A" / "Set B" registers could be invented though.
>
> ---
>
> Pending:
> * Needs ACK from Thierry Reding to be merged via linux-gpio tree by Linus
> Walleij. (fine with the general approach, requested changes which
> should have been taken care of now)
As I said elsewhere, I haven't seen an Acked-by on the binding changes,
so that would be another pending item here.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH V3 0/4] pwm: tegra: Pin configuration in suspend/resume and cleanups
From: Thierry Reding @ 2017-04-12 17:18 UTC (permalink / raw)
To: Laxman Dewangan
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
mark.rutland-5wv7dgnIgG8, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491557642-15940-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]
On Fri, Apr 07, 2017 at 03:03:58PM +0530, Laxman Dewangan wrote:
> This patch series have following fixes:
> - Add more precession in PWM period register value calculation
> for lower pwm frequency.
> - Add support to configure PWM pins in different state in the
> suspend/resume.
>
> Changes from v1:
> - Use standard pinctrl names for sleep and active state.
> - Use API pinctrl_pm_select_*()
>
> Changes from V2:
> - Type fixes, rephrases commit message and use pinctrl_pm_state* return
> value.
>
> Laxman Dewangan (4):
> pwm: tegra: Use DIV_ROUND_CLOSEST_ULL() instead of local
> implementation
> pwm: tegra: Increase precision in pwm rate calculation
> pwm: tegra: Add DT binding details to configure pin in suspends/resume
> pwm: tegra: Add support to configure pin state in suspends/resume
>
> .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++
> drivers/pwm/pwm-tegra.c | 77 ++++++++++++++++++++--
> 2 files changed, 116 insertions(+), 4 deletions(-)
All four patches applied to for-4.12/drivers, thanks.
I've slightly modified the commit messages of some patches for "pwm" ->
"PWM".
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH V3 2/4] pwm: tegra: Increase precision in pwm rate calculation
From: Thierry Reding @ 2017-04-12 17:19 UTC (permalink / raw)
To: Laxman Dewangan
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
mark.rutland-5wv7dgnIgG8, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491557642-15940-3-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]
On Fri, Apr 07, 2017 at 03:04:00PM +0530, Laxman Dewangan wrote:
> The rate of the PWM calculated as follows:
> hz = NSEC_PER_SEC / period_ns;
> rate = (rate + (hz / 2)) / hz;
>
> This has the precision loss in lower PWM rate.
>
> Change this to have more precision as:
> hz = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC * 100, period_ns);
> rate = DIV_ROUND_CLOSEST(rate * 100, hz)
>
> Example:
> 1. period_ns = 16672000, PWM clock rate is 200KHz.
> Based on old formula
> hz = NSEC_PER_SEC / period_ns
> = 1000000000ul/16672000
> = 59 (59.98)
> rate = (200K + 59/2)/59 = 3390
>
> Based on new method:
> hz = 5998
> rate = DIV_ROUND_CLOSE(200000*100, 5998) = 3334
>
> If we measure the PWM signal rate, we will get more accurate period
> with rate value of 3334 instead of 3390.
>
> 2. period_ns = 16803898, PWM clock rate is 200KHz.
> Based on old formula:
> hz = 59, rate = 3390
> Based on new formula:
> hz = 5951, rate = 3360
>
> The PWM signal rate of 3360 is more near to requested period than 3333.
>
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> ---
> Changes from v1:
> - None
>
> Changes from V2:
> - Fix the commit message with exact formula used.
> ---
> drivers/pwm/pwm-tegra.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 0a688da..21518be 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -76,6 +76,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
> unsigned long long c = duty_ns;
> unsigned long rate, hz;
> + unsigned long long ns100 = NSEC_PER_SEC;
> u32 val = 0;
> int err;
>
> @@ -94,9 +95,11 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> * cycles at the PWM clock rate will take period_ns nanoseconds.
> */
> rate = clk_get_rate(pc->clk) >> PWM_DUTY_WIDTH;
> - hz = NSEC_PER_SEC / period_ns;
>
> - rate = (rate + (hz / 2)) / hz;
> + /* Consider precision in PWM_SCALE_WIDTH rate calculation */
> + ns100 *= 100;
> + hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
I think hz could overflow for small enough values of period_ns. I've
sent a patch that makes hz unsigned long long. While at it, the patch
also removes the ns100 variable which isn't really necessary here.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Thierry Reding @ 2017-04-12 17:21 UTC (permalink / raw)
To: Ralph Sennhauser
Cc: Andrew Lunn, Linus Walleij, Alexandre Courbot, Rob Herring,
Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170409180931.4884-2-ralph.sennhauser@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]
On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> From: Andrew Lunn <andrew@lunn.ch>
>
> Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> and off period. This can be modelled as a PWM.
>
> However, there are only two sets of PWM configuration registers for
> all the GPIO lines. This driver simply allows a single GPIO line per
> GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
>
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the GPIO driver. Thus the GPIO driver has been
> extended with a PWM driver.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> URL: https://patchwork.ozlabs.org/patch/427287/
> URL: https://patchwork.ozlabs.org/patch/427295/
> [Ralph Sennhauser:
> * Port forward
> * Merge PWM portion into gpio-mvebu.c
> * Switch to atomic PWM API
> * Add new compatible string marvell,armada-370-xp-gpio
> * Update and merge documentation patch
> * Update MAINTAINERS]
> Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> ---
> .../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
> MAINTAINERS | 2 +
> drivers/gpio/gpio-mvebu.c | 324 ++++++++++++++++++++-
> 3 files changed, 346 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> index a6f3bec..fe49e9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> @@ -38,6 +38,24 @@ Required properties:
> - #gpio-cells: Should be two. The first cell is the pin number. The
> second cell is reserved for flags, unused at the moment.
>
> +Optional properties:
> +
> +In order to use the gpio lines in PWM mode, some additional optional
> +properties are required. Only Armada 370 and XP support these properties.
> +
> +- compatible: Must contain "marvell,armada-370-xp-gpio"
> +
> +- reg: an additional register set is needed, for the GPIO Blink
> + Counter on/off registers.
> +
> +- reg-names: Must contain an entry "pwm" corresponding to the
> + additional register range needed for pwm operation.
> +
> +- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> + second cell is the period in nanoseconds.
> +
> +- clocks: Must be a phandle to the clock for the gpio controller.
One other thing: there's a mix of pwm/PWM and gpio/GPIO in this hunk. In
prose, always use the all-uppercase variants because they are
abbreviations.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v5 0/8] mfd: Add OF device table to I2C drivers that are missing it
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Wolfram Sang,
Mark Rutland, linux-omap, Russell King, linux-arm-kernel,
linux-i2c
Hello,
This series add OF device ID tables to mfd I2C drivers whose devices are
either used in Device Tree source files or are listed in binding docs as
a compatible string.
That's done because the plan is to change the I2C core to report proper OF
modaliases instead of always reporting a MODALIAS=i2c:<foo> regardless if
a device was registered via DT or using the legacy platform data mechanism.
So these patches will make sure that mfd I2C drivers modules will continue
to be autoloaded once the I2C core is changed to report proper OF modalias.
Users didn't have a vendor prefix in the used compatible strings, but since
there wasn't a DT binding document for these drivers, it can be said that
were working for mere luck and so this series fixes the users and add a DT
binding doc for the drivers.
Most patches can be applied independently, with the exception of patches
2 to 4 that should be applied in the same tree to keep bisect-ability. I
suggest these to go through the MFD subsystem tree.
Best regards,
Javier
Changes in v5:
- Add missing properties for interrupts to DT binding doc (Rob Herring).
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Drop -mfd suffix in compatible string (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
Javier Martinez Canillas (8):
dt-bindings: mfd: Add retu/tahvo ASIC chips bindings
mfd: retu: Drop -mfd suffix from I2C device ID name
mfd: retu: Add OF device ID table
ARM: dts: n8x0: Add vendor prefix to retu node
i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
dt-bindings: mfd: Add TI tps6105x chip bindings
mfd: tps6105x: Add OF device ID table
ARM: ux500: Add vendor prefix to tps61052 node
.../devicetree/bindings/i2c/i2c-cbus-gpio.txt | 4 ++--
Documentation/devicetree/bindings/mfd/retu.txt | 23 ++++++++++++++++++++++
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 ++++++++++++++++
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 4 ++--
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
arch/arm/mach-omap1/board-nokia770.c | 4 ++--
drivers/mfd/retu-mfd.c | 12 +++++++++--
drivers/mfd/tps6105x.c | 8 ++++++++
8 files changed, 65 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
--
2.9.3
^ permalink raw reply
* [PATCH v5 1/8] dt-bindings: mfd: Add retu/tahvo ASIC chips bindings
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
There are Device Tree source files defining a device node for the
retu/tahvo I2C chip, but there isn't a DT binding document for it.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v5:
- Add missing properties for interrupts to DT binding doc (Rob Herring).
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/retu.txt | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
diff --git a/Documentation/devicetree/bindings/mfd/retu.txt b/Documentation/devicetree/bindings/mfd/retu.txt
new file mode 100644
index 000000000000..e1ea3a36a038
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/retu.txt
@@ -0,0 +1,23 @@
+* Device tree bindings for Nokia Retu and Tahvo multi-function device
+
+Retu and Tahvo are a multi-function devices found on Nokia Internet
+Tablets (770, N800 and N810). The Retu chip provides watchdog timer
+and power button control functionalities while Tahvo chip provides
+USB transceiver functionality.
+
+Required properties:
+- compatible: "nokia,retu" or "nokia,tahvo"
+- reg: Specifies the I2C slave address of the ASIC chip
+- interrupts: The interrupt line the device is connected to
+- interrupt-parent: The parent interrupt controller
+
+Example:
+
+i2c0 {
+ retu: retu@1 {
+ compatible = "nokia,retu";
+ interrupt-parent = <&gpio4>;
+ interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+ reg = <0x1>;
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 2/8] mfd: retu: Drop -mfd suffix from I2C device ID name
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Mark Rutland,
linux-omap, Russell King, linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
It's not correct to encode the subsystem in the I2C device name, so
drop the -mfd suffix. To maintain bisect-ability, change driver and
platform code / DTS users in the same patch.
Suggested-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 4 ++--
arch/arm/mach-omap1/board-nokia770.c | 4 ++--
drivers/mfd/retu-mfd.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 7e5ffc583c90..1b06430c8013 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -15,8 +15,8 @@
>;
#address-cells = <1>;
#size-cells = <0>;
- retu_mfd: retu@1 {
- compatible = "retu-mfd";
+ retu: retu@1 {
+ compatible = "retu";
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
reg = <0x1>;
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index ee8d9f553db4..06243c0b12d2 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c
@@ -233,10 +233,10 @@ static struct platform_device nokia770_cbus_device = {
static struct i2c_board_info nokia770_i2c_board_info_2[] __initdata = {
{
- I2C_BOARD_INFO("retu-mfd", 0x01),
+ I2C_BOARD_INFO("retu", 0x01),
},
{
- I2C_BOARD_INFO("tahvo-mfd", 0x02),
+ I2C_BOARD_INFO("tahvo", 0x02),
},
};
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index d4c114abeb75..53e1d386d2c0 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -302,8 +302,8 @@ static int retu_remove(struct i2c_client *i2c)
}
static const struct i2c_device_id retu_id[] = {
- { "retu-mfd", 0 },
- { "tahvo-mfd", 0 },
+ { "retu", 0 },
+ { "tahvo", 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, retu_id);
--
2.9.3
^ permalink raw reply related
* [PATCH v5 3/8] mfd: retu: Add OF device ID table
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Aaro Koskinen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Tony Lindgren, Lee Jones, Javier Martinez Canillas
In-Reply-To: <20170412172800.23035-1-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>'s Acked-by tag.
- Add Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>'s Acked-by tag.
Changes in v4:
- Drop -mfd suffix in compatible string (Lee Jones).
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/retu-mfd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index 53e1d386d2c0..e7d27b7861c1 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -308,9 +308,17 @@ static const struct i2c_device_id retu_id[] = {
};
MODULE_DEVICE_TABLE(i2c, retu_id);
+static const struct of_device_id retu_of_match[] = {
+ { .compatible = "nokia,retu" },
+ { .compatible = "nokia,tahvo" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, retu_of_match);
+
static struct i2c_driver retu_driver = {
.driver = {
.name = "retu-mfd",
+ .of_match_table = retu_of_match,
},
.probe = retu_probe,
.remove = retu_remove,
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 4/8] ARM: dts: n8x0: Add vendor prefix to retu node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Mark Rutland,
linux-omap, Russell King, linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The retu device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 1b06430c8013..91886231e5a8 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -16,7 +16,7 @@
#address-cells = <1>;
#size-cells = <0>;
retu: retu@1 {
- compatible = "retu";
+ compatible = "nokia,retu";
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
reg = <0x1>;
--
2.9.3
^ permalink raw reply related
* [PATCH v5 5/8] i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Wolfram Sang, linux-i2c, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The example contains a device node for a retu device, but
its compatible string doesn't have a vendor prefix.
While being there, drop the -mfd suffix since isn't correct.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Avoid using MFD in Device Tree (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
index 8ce9cd2855b5..c143948b2a37 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
@@ -20,8 +20,8 @@ i2c@0 {
#address-cells = <1>;
#size-cells = <0>;
- retu-mfd: retu@1 {
- compatible = "retu-mfd";
+ retu: retu@1 {
+ compatible = "nokia,retu";
reg = <0x1>;
};
};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 6/8] dt-bindings: mfd: Add TI tps6105x chip bindings
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
There are Device Tree source files defining a device node for the
tps61050/61052 I2C chip but there isn't a binding document for it.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
new file mode 100644
index 000000000000..93602c7a19c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
@@ -0,0 +1,17 @@
+* Device tree bindings for TI TPS61050/61052 Boost Converters
+
+The TP61050/TPS61052 is a high-power "white LED driver". The
+device provides LED, GPIO and regulator functionalities.
+
+Required properties:
+- compatible: "ti,tps61050" or "ti,tps61052"
+- reg: Specifies the I2C slave address
+
+Example:
+
+i2c0 {
+ tps61052@33 {
+ compatible = "ti,tps61052";
+ reg = <0x33>;
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 7/8] mfd: tps6105x: Add OF device ID table
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
Changes in v4: None
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/tps6105x.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c
index baa12ea666fb..187848c93779 100644
--- a/drivers/mfd/tps6105x.c
+++ b/drivers/mfd/tps6105x.c
@@ -173,9 +173,17 @@ static const struct i2c_device_id tps6105x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, tps6105x_id);
+static const struct of_device_id tps6105x_of_match[] = {
+ { .compatible = "ti,tps61050" },
+ { .compatible = "ti,tps61052" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tps6105x_of_match);
+
static struct i2c_driver tps6105x_driver = {
.driver = {
.name = "tps6105x",
+ .of_match_table = tps6105x_of_match,
},
.probe = tps6105x_probe,
.remove = tps6105x_remove,
--
2.9.3
^ permalink raw reply related
* [PATCH v5 8/8] ARM: ux500: Add vendor prefix to tps61052 node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Aaro Koskinen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Tony Lindgren, Lee Jones, Javier Martinez Canillas, Mark Rutland,
Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170412172800.23035-1-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
The tps61052 device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/ste-hrefprev60.dtsi b/arch/arm/boot/dts/ste-hrefprev60.dtsi
index 5882a2606ac3..3f14b4df69b4 100644
--- a/arch/arm/boot/dts/ste-hrefprev60.dtsi
+++ b/arch/arm/boot/dts/ste-hrefprev60.dtsi
@@ -30,7 +30,7 @@
i2c@80004000 {
tps61052@33 {
- compatible = "tps61052";
+ compatible = "ti,tps61052";
reg = <0x33>;
};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 3/3] of: Make of_match_node() an inline stub for CONFIG_OF=n
From: kbuild test robot @ 2017-04-12 18:58 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew-g2DYL2Zd6BY,
vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Florian Fainelli,
Lee Jones, Nicolas Ferre, Rob Herring, Frank Rowand,
open list:NETWORKING DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-4-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4084 bytes --]
Hi Florian,
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.11-rc6 next-20170412]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/of-Make-of_match_node-an-inline-stub-for-CONFIG_OF-n/20170412-160916
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arm-s3c2410_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-s3c2410.c: In function 's3c24xx_get_device_quirks':
>> drivers/i2c/busses/i2c-s3c2410.c:174:25: error: 's3c24xx_i2c_match' undeclared (first use in this function)
match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-s3c2410.c:174:25: note: each undeclared identifier is reported only once for each function it appears in
--
drivers/watchdog/s3c2410_wdt.c: In function 's3c2410_get_wdt_drv_data':
>> drivers/watchdog/s3c2410_wdt.c:515:25: error: 's3c2410_wdt_match' undeclared (first use in this function)
match = of_match_node(s3c2410_wdt_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/watchdog/s3c2410_wdt.c:515:25: note: each undeclared identifier is reported only once for each function it appears in
vim +/s3c24xx_i2c_match +174 drivers/i2c/busses/i2c-s3c2410.c
faf93ff6 Giridhar Maruthy 2013-01-24 158 .data = (void *)(QUIRK_S3C2440 | QUIRK_NO_GPIO) },
117053f7 Vasanth Ananthan 2013-11-11 159 { .compatible = "samsung,exynos5-sata-phy-i2c",
117053f7 Vasanth Ananthan 2013-11-11 160 .data = (void *)(QUIRK_S3C2440 | QUIRK_POLL | QUIRK_NO_GPIO) },
27452498 Karol Lewandowski 2012-04-23 161 {},
27452498 Karol Lewandowski 2012-04-23 162 };
27452498 Karol Lewandowski 2012-04-23 163 MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
27452498 Karol Lewandowski 2012-04-23 164 #endif
^1da177e Linus Torvalds 2005-04-16 165
ec7c34a4 Krzysztof Kozlowski 2016-04-21 166 /*
27452498 Karol Lewandowski 2012-04-23 167 * Get controller type either from device tree or platform device variant.
^1da177e Linus Torvalds 2005-04-16 168 */
5f1b1155 Pankaj Dubey 2014-01-15 169 static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev)
^1da177e Linus Torvalds 2005-04-16 170 {
27452498 Karol Lewandowski 2012-04-23 171 if (pdev->dev.of_node) {
27452498 Karol Lewandowski 2012-04-23 172 const struct of_device_id *match;
0915833b Krzysztof Kozlowski 2016-04-21 173
b900ba4c Karol Lewandowski 2012-05-30 @174 match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
5f1b1155 Pankaj Dubey 2014-01-15 175 return (kernel_ulong_t)match->data;
27452498 Karol Lewandowski 2012-04-23 176 }
5a5f5080 Thomas Abraham 2011-09-13 177
27452498 Karol Lewandowski 2012-04-23 178 return platform_get_device_id(pdev)->driver_data;
^1da177e Linus Torvalds 2005-04-16 179 }
^1da177e Linus Torvalds 2005-04-16 180
ec7c34a4 Krzysztof Kozlowski 2016-04-21 181 /*
ec7c34a4 Krzysztof Kozlowski 2016-04-21 182 * Complete the message and wake up the caller, using the given return code,
:::::: The code at line 174 was first introduced by commit
:::::: b900ba4c1513a8c9a2fab8dca4cc6f50b17d6861 i2c: s3c2410: Fix pointer type passed to of_match_node()
:::::: TO: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
:::::: CC: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22646 bytes --]
^ permalink raw reply
* Re: [PATCH v5 04/10] drm/stm: Add STM32 LTDC driver
From: Benjamin Gaignard @ 2017-04-12 18:58 UTC (permalink / raw)
To: Eric Anholt
Cc: Mark Rutland, devicetree, Alexandre TORGUE, Arnd Bergmann,
linux-arm-kernel, Russell King, Rob Herring, Philippe Cornu,
Yannick Fertre, kernel, dri-devel@lists.freedesktop.org,
Maxime Coquelin, Mickael Reulier, Vincent Abriou, Fabien Dessenne,
Gabriel FERNANDEZ, Benjamin Gaignard
In-Reply-To: <87wpaqeks2.fsf@eliezer.anholt.net>
2017-04-11 22:45 GMT+02:00 Eric Anholt <eric@anholt.net>:
> Yannick Fertre <yannick.fertre@st.com> writes:
>
>> This controller provides output signals to interface directly a variety
>> of LCD and TFT panels. These output signals are: RGB signals
>> (up to 24bpp), vertical & horizontal synchronisations, data enable and
>> the pixel clock.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
>> ---
>> drivers/gpu/drm/Kconfig | 3 +-
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/stm/Kconfig | 16 +
>> drivers/gpu/drm/stm/Makefile | 7 +
>> drivers/gpu/drm/stm/drv.c | 221 ++++++++
>> drivers/gpu/drm/stm/ltdc.c | 1210 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/stm/ltdc.h | 40 ++
>> 7 files changed, 1497 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/stm/Kconfig
>> create mode 100644 drivers/gpu/drm/stm/Makefile
>> create mode 100644 drivers/gpu/drm/stm/drv.c
>> create mode 100644 drivers/gpu/drm/stm/ltdc.c
>> create mode 100644 drivers/gpu/drm/stm/ltdc.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 78d7fc0..dd5762a 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -203,7 +203,6 @@ config DRM_VGEM
>> as used by Mesa's software renderer for enhanced performance.
>> If M is selected the module will be called vgem.
>>
>> -
>
> Stray whitespace change.
>
> With this removed, the driver is:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
>
> Apologies for the delay in the second review offered. The remainder of
> my comments are little cleanups, all of which I think are optional and
> fine to do after the code lands.
>
> You should probably update MAINTAINERS for your new driver. If you'd
> like to maintain this driver in the drm-misc small drivers collection
> (https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html),
> send a follow-up patch to the list to add the MAINTAINERS entry, and I
> can get that and patches 1-4 merged. Once you have a few more patches
> in, we can add you to the drm-misc committers crew so you can merge
> directly after getting review.
Since we are moving sti driver to drm-misc it makes sense to move all
STMicroelectronic SoC display drivers to it.
>
> I'll also take this moment to plug something: Please feel welcome to
> review other people's driver patches on the list. You've built
> something nice here, and probably learned a lot of lessons along the way
> that you could share with others. (I just found out about
> of_reset_control in reviewing your code, and I wish I had known about it
> back when I was landing vc4!)
>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> new file mode 100644
>> index 0000000..922f021
>> --- /dev/null
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>
>> +static void ltdc_crtc_disable(struct drm_crtc *crtc)
>> +{
>> + struct ltdc_device *ldev = crtc_to_ltdc(crtc);
>> + struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + if (!crtc->enabled) {
>> + DRM_DEBUG_DRIVER("already disabled\n");
>> + return;
>> + }
>
> I think this crtc->enabled is a given for the disable() being called.
>
>> +
>> + drm_crtc_vblank_off(crtc);
>> +
>> + /* disable LTDC */
>> + reg_clear(ldev->regs, LTDC_GCR, GCR_LTDCEN);
>> +
>> + /* disable IRQ */
>> + reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> + /* immediately commit disable of layers before switching off LTDC */
>> + reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
>> +
>> + if (event) {
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irq(&crtc->dev->event_lock);
>> + if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irq(&crtc->dev->event_lock);
>> + }
>
> I believe that we're guaranteed that crtc->state->event is NULL in the
> disable call, since your atomic_flush() already armed or sent the event
> and NULLed out the pointer.
>
>> +struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev)
>> +{
>> + struct drm_connector *connector;
>> + int err;
>> +
>> + connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
>> + if (!connector) {
>> + DRM_ERROR("Failed to allocate connector\n");
>> + return NULL;
>> + }
>> +
>> + connector->polled = DRM_CONNECTOR_POLL_HPD;
>> +
>> + err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
>> + DRM_MODE_CONNECTOR_LVDS);
>
> I think DRM_MODE_CONNECTOR_DPI (and _ENCODER_DPI) are slightly more
> accurate descriptions, if I'm interpreting your pinmux setup right.
> It's cosmetic, though.
>
>> +static struct drm_panel *ltdc_get_panel(struct drm_device *ddev)
>> +{
>> + struct device *dev = ddev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *entity, *port = NULL;
>> + struct drm_panel *panel = NULL;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + /*
>> + * Parse ltdc node to get remote port and find RGB panel / HDMI slave
>> + * If a dsi or a bridge (hdmi, lvds...) is connected to ltdc,
>> + * a remote port & RGB panel will not be found.
>> + */
>> + for_each_endpoint_of_node(np, entity) {
>> + if (!of_device_is_available(entity))
>> + continue;
>> +
>> + port = of_graph_get_remote_port_parent(entity);
>> + if (port) {
>> + panel = of_drm_find_panel(port);
>> + of_node_put(port);
>> + if (panel) {
>> + DRM_DEBUG_DRIVER("remote panel %s\n",
>> + port->full_name);
>> + } else {
>> + DRM_DEBUG_DRIVER("panel missing\n");
>> + of_node_put(entity);
>> + }
>> + }
>> + }
>
> Future work: You may find the new drm_of_find_panel_or_bridge() useful
> to drop this loop.
>
>> +
>> + return panel;
>> +}
>> +
>> +int ltdc_load(struct drm_device *ddev)
>> +{
>> + struct platform_device *pdev = to_platform_device(ddev->dev);
>> + struct ltdc_device *ldev = ddev->dev_private;
>> + struct device *dev = ddev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct drm_encoder *encoder;
>> + struct drm_connector *connector = NULL;
>> + struct drm_crtc *crtc;
>> + struct reset_control *rstc;
>> + struct resource res;
>> + int irq, ret, i;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + ldev->panel = ltdc_get_panel(ddev);
>> + if (!ldev->panel)
>> + return -EPROBE_DEFER;
>> +
>> + rstc = of_reset_control_get(np, NULL);
>> +
>> + mutex_init(&ldev->err_lock);
>> +
>> + ldev->pixel_clk = devm_clk_get(dev, "lcd");
>> + if (IS_ERR(ldev->pixel_clk)) {
>> + DRM_ERROR("Unable to get lcd clock\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (clk_prepare_enable(ldev->pixel_clk)) {
>> + DRM_ERROR("Unable to prepare pixel clock\n");
>> + return -ENODEV;
>> + }
>
> Future work: You may want to move the pixel clock enable into the CRTC's
> .enable() and disable in .disable(). It sounded in previous versions
> like the HW uses that clock for all register accesses, so you'd need to
> protect a couple of other places, but that should save power when the
> device is off, right?
>
>> +
>> + if (of_address_to_resource(np, 0, &res)) {
>> + DRM_ERROR("Unable to get resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + ldev->regs = devm_ioremap_resource(dev, &res);
>> + if (IS_ERR(ldev->regs)) {
>> + DRM_ERROR("Unable to get ltdc registers\n");
>> + return PTR_ERR(ldev->regs);
>> + }
>> +
>> + for (i = 0; i < MAX_IRQ; i++) {
>> + irq = platform_get_irq(pdev, i);
>> + if (irq < 0)
>> + continue;
>> +
>> + ret = devm_request_threaded_irq(dev, irq, ltdc_irq,
>> + ltdc_irq_thread, IRQF_ONESHOT,
>> + dev_name(dev), ddev);
>> + if (ret) {
>> + DRM_ERROR("Failed to register LTDC interrupt\n");
>> + return ret;
>> + }
>> + }
>> +
>> + if (!IS_ERR(rstc))
>> + reset_control_deassert(rstc);
>> +
>> + /* Disable interrupts */
>> + reg_clear(ldev->regs, LTDC_IER,
>> + IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> + ret = ltdc_get_caps(ddev);
>> + if (ret) {
>> + DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
>> + ldev->caps.hw_version);
>> + return ret;
>> + }
>> +
>> + DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
>> +
>> + if (ltdc_create_encoders(ddev)) {
>> + DRM_ERROR("Failed to create encoders\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (ldev->panel) {
>> + encoder = ltdc_rgb_encoder_find(ddev);
>> + if (!encoder) {
>> + DRM_ERROR("Failed to find RGB encoder\n");
>> + ret = -EINVAL;
>> + goto err;
>> + }
>
> Given that ltdc_create_encoders() only does work if ldev->panel, its
> body could probably be moved in here and then ltdc_rgb_encoder_find
> could be dropped.
>
>> +
>> + connector = ltdc_rgb_connector_create(ddev);
>> + if (!connector) {
>> + DRM_ERROR("Failed to create RGB connector\n");
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + ret = drm_mode_connector_attach_encoder(connector, encoder);
>> + if (ret) {
>> + DRM_ERROR("Failed to attach connector to encoder\n");
>> + goto err;
>> + }
>> +
>> + drm_panel_attach(ldev->panel, connector);
>> + }
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Marcel Holtmann @ 2017-04-12 20:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <20170411140637.GA4388-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Hi Sebastian,
>>>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>>>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>>>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
>>>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
>>>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
>>>>>>>> for the bluetooth subsystem.
>>>>>>>
>>>>>>> Greg doesn't read cover letters generally and since the serdev patches
>>>>>>> are Cc rather than To him, he's probably not planning to pick them up.
>>>>>>
>>>>>> I wonder actually if we should merge all of these via bluetooth-next
>>>>>> tree with proper Ack from Greg. However it would be good to also get
>>>>>> buy in from Dave for merging this ultimately through net-next.
>>>>>
>>>>> I don't really care where it goes. I can take the whole thing in my
>>>>> tty/serial tree now if no one objects and I get an ack from the relevant
>>>>> maintainers {hint...}
>>>>
>>>> I think it is better if it goes thru BT tree. I have another driver
>>>> converted that is dependent on this series. There's a couple other
>>>> serdev changes on the list too, but this shouldn't depend on them.
>>>
>>> Is this waiting for something, or could it be queued to
>>> bluetooth-next then? It would be nice to finally have
>>> this in 4.12 :)
>>
>> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
>
> Sorry thought this was coming through mine:
> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
>
> Merge away!
so I have applied patches 1-8 to bluetooth-next tree.
The last 2 I left out since they do cause build issues on non-DT platforms. We need to be able to build the driver on all platforms so that sanity compile checks happen all the time.
CC drivers/bluetooth/hci_nokia.o
drivers/bluetooth/hci_nokia.c:802:34: error: array type has incomplete element type ‘struct of_device_id’
static const struct of_device_id nokia_bluetooth_of_match[] = {
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:803:4: error: field name not in record or union initializer
{ .compatible = "nokia,h4p-bluetooth", },
^
drivers/bluetooth/hci_nokia.c:803:4: note: (near initialization for ‘nokia_bluetooth_of_match’)
drivers/bluetooth/hci_nokia.c:815:21: error: implicit declaration of function ‘of_match_ptr’ [-Werror=implicit-function-declaration]
.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
^~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:802:34: warning: ‘nokia_bluetooth_of_match’ defined but not used [-Wunused-variable]
static const struct of_device_id nokia_bluetooth_of_match[] = {
^~~~~~~~~~~~~~~~~~~~~~~~
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/4] bluetooth: hci_uart: add LL protocol serdev driver support
From: Marcel Holtmann @ 2017-04-12 20:20 UTC (permalink / raw)
To: Rob Herring
Cc: linux-bluetooth, linux-arm-kernel, Gustavo F. Padovan,
Johan Hedberg, Mark Rutland, Wei Xu, Eyal Reizer, Satish Patel,
netdev, devicetree
In-Reply-To: <20170407143516.9945-1-robh@kernel.org>
Hi Rob,
> Turns out that the LL protocol and the TI-ST are the same thing AFAICT.
> The TI-ST adds firmware loading, GPIO control, and shared access for
> NFC, FM radio, etc. For now, we're only implementing what is needed for
> BT. This mirrors other drivers like BCM and Intel, but uses the new
> serdev bus.
>
> The firmware loading is greatly simplified by using existing
> infrastructure to send commands. It may be a bit slower than the
> original code using synchronous functions, but the real bottleneck is
> likely doing firmware load at 115.2kbps.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org
> ---
> v2:
> - Use IS_ENABLED() to fix module build
>
> drivers/bluetooth/hci_ll.c | 261 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 260 insertions(+), 1 deletion(-)
can you re-send any missing patch on top of today's bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v3 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Cyrille Pitchen @ 2017-04-12 20:28 UTC (permalink / raw)
To: Ludovic Barre, Cyrille Pitchen, Marek Vasut
Cc: Boris Brezillon, Alexandre Torgue,
devicetree-u79uwXL29TY76Z2rM5mHXA, Richard Weinberger,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
David Woodhouse
In-Reply-To: <1492016785-24976-3-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>
Hi Ludovic,
Globally I will say this is pretty good and, IMHO, almost ready to be
merged into the github/spi-nor tree.
I have few comments, all but one are mainly recommendations hence not
blocking on my side. However one should be fixed. Even if not critical
and very unlikely to happen, I think it is more a potential bug than a
simple cosmetic issue.
So if you have time to prepare a new series, I will try if possible to
add your patch into the PR, which should be sent ideally before the end
of the week. I can't give you any guarantee since the timing is really
short now. Besides Marek still needs to add his Acked-by / Reviewed-by
once satisfied, otherwise your patch will have to wait for the next release.
If something I say is wrong or if you disagree with me, do not hesitate:
we can discuss and I can change my mind.
So my comments to follow:
Le 12/04/2017 à 19:06, Ludovic Barre a écrit :
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
>
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
> registers
> -read memory-mapped mode: the external Flash memory is mapped to the
> microcontroller address space and is seen by the system as if it was
> an internal memory
>
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> ---
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/stm32-quadspi.c | 693 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 701 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 7252087..bfdfb1e 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
> To compile this driver as a module, choose M here: the module
> will be called intel-spi-platform.
>
> +config SPI_STM32_QUADSPI
> + tristate "STM32 Quad SPI controller"
> + depends on ARCH_STM32
> + help
> + This enables support for the STM32 Quad SPI controller.
> + We only connect the NOR to this controller.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 72238a7..285aab8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> obj-$(CONFIG_SPI_INTEL_SPI) += intel-spi.o
> obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM) += intel-spi-platform.o
> +obj-$(CONFIG_SPI_STM32_QUADSPI) += stm32-quadspi.o
> \ No newline at end of file
> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
> new file mode 100644
> index 0000000..9e90dee
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c
> @@ -0,0 +1,693 @@
> +/*
> + * stm32_quadspi.c
> + *
> + * Copyright (C) 2017, Ludovic Barre
> + *
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +#include <linux/clk.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#define QUADSPI_CR 0x00
> +#define CR_EN BIT(0)
> +#define CR_ABORT BIT(1)
> +#define CR_DMAEN BIT(2)
> +#define CR_TCEN BIT(3)
> +#define CR_SSHIFT BIT(4)
> +#define CR_DFM BIT(6)
> +#define CR_FSEL BIT(7)
> +#define CR_FTHRES_SHIFT 8
> +#define CR_FTHRES_MASK GENMASK(12, 8)
> +#define CR_FTHRES(n) (((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
> +#define CR_TEIE BIT(16)
> +#define CR_TCIE BIT(17)
> +#define CR_FTIE BIT(18)
> +#define CR_SMIE BIT(19)
> +#define CR_TOIE BIT(20)
> +#define CR_PRESC_SHIFT 24
> +#define CR_PRESC_MASK GENMASK(31, 24)
> +#define CR_PRESC(n) (((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
> +
> +#define QUADSPI_DCR 0x04
> +#define DCR_CSHT_SHIFT 8
> +#define DCR_CSHT_MASK GENMASK(10, 8)
> +#define DCR_CSHT(n) (((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
> +#define DCR_FSIZE_SHIFT 16
> +#define DCR_FSIZE_MASK GENMASK(20, 16)
> +#define DCR_FSIZE(n) (((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
> +
> +#define QUADSPI_SR 0x08
> +#define SR_TEF BIT(0)
> +#define SR_TCF BIT(1)
> +#define SR_FTF BIT(2)
> +#define SR_SMF BIT(3)
> +#define SR_TOF BIT(4)
> +#define SR_BUSY BIT(5)
> +#define SR_FLEVEL_SHIFT 8
> +#define SR_FLEVEL_MASK GENMASK(13, 8)
> +
> +#define QUADSPI_FCR 0x0c
> +#define FCR_CTCF BIT(1)
> +
> +#define QUADSPI_DLR 0x10
> +
> +#define QUADSPI_CCR 0x14
> +#define CCR_INST_SHIFT 0
> +#define CCR_INST_MASK GENMASK(7, 0)
> +#define CCR_INST(n) (((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
> +#define CCR_IMODE_NONE (0 << 8)
> +#define CCR_IMODE_1 (1 << 8)
> +#define CCR_IMODE_2 (2 << 8)
> +#define CCR_IMODE_4 (3 << 8)
> +#define CCR_ADMODE_NONE (0 << 10)
> +#define CCR_ADMODE_1 (1 << 10)
> +#define CCR_ADMODE_2 (2 << 10)
> +#define CCR_ADMODE_4 (3 << 10)
For bitfields, it would have been better to use (1u << 8) that is
unsigned int instead of signed int. It might avoid some warnings or even
bugs depending on how you use those macros, especially if you were using
BIT(31). The GENMASK() macro generates an unsigned int on its side.
However, I guess you have tested your driver so if you have noticed any
issue, I will say this is not blocking for me :)
You don't use BIT(31) and I don't see any use of the >> operator so I
think it should be safe. My comment is just a recommendation for the
next time!
To be fair, you can point out the atmel_qspi.c driver and I did the same
mistake but like you, I didn't use the >> operator with the bitmask I
have defined :p
> +#define CCR_ADSIZE_SHIFT 12
> +#define CCR_ADSIZE_MASK GENMASK(13, 12)
> +#define CCR_ADSIZE(n) (((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
> +#define CCR_ABMODE_NONE (0 << 14)
> +#define CCR_ABMODE_1 (1 << 14)
> +#define CCR_ABMODE_2 (2 << 14)
> +#define CCR_ABMODE_4 (3 << 14)
> +#define CCR_ABSIZE_8 (0 << 16)
> +#define CCR_ABSIZE_16 (1 << 16)
> +#define CCR_ABSIZE_24 (2 << 16)
> +#define CCR_ABSIZE_32 (3 << 16)
> +#define CCR_DCYC_SHIFT 18
> +#define CCR_DCYC_MASK GENMASK(22, 18)
> +#define CCR_DCYC(n) (((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
> +#define CCR_DMODE_NONE (0 << 24)
> +#define CCR_DMODE_1 (1 << 24)
> +#define CCR_DMODE_2 (2 << 24)
> +#define CCR_DMODE_4 (3 << 24)
> +#define CCR_FMODE_INDW (0 << 26)
> +#define CCR_FMODE_INDR (1 << 26)
> +#define CCR_FMODE_APM (2 << 26)
> +#define CCR_FMODE_MM (3 << 26)
> +
> +#define QUADSPI_AR 0x18
> +#define QUADSPI_ABR 0x1c
> +#define QUADSPI_DR 0x20
> +#define QUADSPI_PSMKR 0x24
> +#define QUADSPI_PSMAR 0x28
> +#define QUADSPI_PIR 0x2c
> +#define QUADSPI_LPTR 0x30
> +#define LPTR_DFT_TIMEOUT 0x10
> +
> +#define FSIZE_VAL(size) (__fls(size) - 1)
> +
> +#define STM32_MAX_MMAP_SZ SZ_256M
> +#define STM32_MAX_NORCHIP 2
> +
> +#define STM32_QSPI_FIFO_TIMEOUT_US 30000
> +#define STM32_QSPI_BUSY_TIMEOUT_US 100000
> +
> +struct stm32_qspi_flash {
> + struct spi_nor nor;
> + u32 cs;
> + u32 fsize;
> + u32 presc;
> + u32 read_mode;
> + struct stm32_qspi *qspi;
> +};
> +
> +struct stm32_qspi {
> + struct device *dev;
> + void __iomem *io_base;
> + void __iomem *mm_base;
> + resource_size_t mm_size;
> + u32 nor_num;
> + struct clk *clk;
> + u32 clk_rate;
> + struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
> + struct completion cmd_completion;
> +
> + /*
> + * to protect device configuration, could be different between
> + * 2 flash access (bk1, bk2)
> + */
> + struct mutex lock;
> +};
> +
> +struct stm32_qspi_cmd {
> + u8 addr_width;
> + u8 dummy;
> + bool tx_data;
> + u8 opcode;
> + u32 framemode;
> + u32 qspimode;
> + u32 addr;
> + size_t len;
> + void *buf;
> +};
> +
> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
> +{
> + u32 cr;
> + int err = 0;
> +
> + if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
> + return 0;
> +
> + reinit_completion(&qspi->cmd_completion);
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
> +
> + if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
> + msecs_to_jiffies(1000)))
> + err = -ETIMEDOUT;
> +
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> + return err;
> +}
> +
> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
> +{
> + u32 sr;
> +
> + return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
> + !(sr & SR_BUSY), 10,
> + STM32_QSPI_BUSY_TIMEOUT_US);
> +}
> +
> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
> + struct stm32_qspi_cmd *cmd, bool read)
> +{
> + u32 dmode = CCR_DMODE_1;
> +
> + cmd->framemode = CCR_IMODE_1;
> +
> + if (read) {
> + switch (nor->flash_read) {
> + case SPI_NOR_NORMAL:
> + case SPI_NOR_FAST:
> + dmode = CCR_DMODE_1;
> + break;
> + case SPI_NOR_DUAL:
> + dmode = CCR_DMODE_2;
> + break;
> + case SPI_NOR_QUAD:
> + dmode = CCR_DMODE_4;
> + break;
> + }
> + }
> +
> + cmd->framemode |= cmd->tx_data ? dmode : 0;
> + cmd->framemode |= cmd->addr_width ? CCR_ADMODE_1 : 0;
> +}
> +
> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
> +{
> + *val = readb_relaxed(addr);
> +}
> +
> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
> +{
> + writeb_relaxed(*val, addr);
> +}
> +
> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + void (*tx_fifo)(u8 *, void __iomem *);
> + u32 len = cmd->len, sr;
> + u8 *buf = cmd->buf;
> + int ret;
> +
> + if (cmd->qspimode == CCR_FMODE_INDW)
> + tx_fifo = stm32_qspi_write_fifo;
> + else
> + tx_fifo = stm32_qspi_read_fifo;
> +
> + while (len--) {
> + ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
> + sr, (sr & SR_FTF), 10,
> + STM32_QSPI_FIFO_TIMEOUT_US);
> + if (ret) {
> + dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
> + break;
> + }
> + tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
> + }
> +
> + return ret;
> +}
> +
> +static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
> + return 0;
> +}
> +
> +static int stm32_qspi_tx(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + if (!cmd->tx_data)
> + return 0;
> +
> + if (cmd->qspimode == CCR_FMODE_MM)
> + return stm32_qspi_tx_mm(qspi, cmd);
> +
> + return stm32_qspi_tx_poll(qspi, cmd);
> +}
> +
> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + struct stm32_qspi *qspi = flash->qspi;
> + u32 ccr, dcr, cr;
> + int err;
> +
> + err = stm32_qspi_wait_nobusy(qspi);
> + if (err)
> + goto abort;
> +
> + dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
> + dcr |= DCR_FSIZE(flash->fsize);
> + writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
> +
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + cr &= ~CR_PRESC_MASK & ~CR_FSEL;
> + cr |= CR_PRESC(flash->presc);
> + cr |= flash->cs ? CR_FSEL : 0;
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> + if (cmd->tx_data)
> + writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
> +
> + ccr = cmd->framemode | cmd->qspimode;
> +
> + if (cmd->dummy)
> + ccr |= CCR_DCYC(cmd->dummy);
> +
> + if (cmd->addr_width)
> + ccr |= CCR_ADSIZE(cmd->addr_width - 1);
> +
> + ccr |= CCR_INST(cmd->opcode);
> + writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
> +
> + if (cmd->addr_width && cmd->qspimode != CCR_FMODE_MM)
> + writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
> +
> + err = stm32_qspi_tx(qspi, cmd);
> + if (err)
> + goto abort;
> +
> + if (cmd->qspimode != CCR_FMODE_MM) {
> + err = stm32_qspi_wait_cmd(qspi);
> + if (err)
> + goto abort;
> + writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
> + }
> +
> + return err;
> +
> +abort:
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> + dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
> + return err;
> +}
> +
> +static int stm32_qspi_read_reg(struct spi_nor *nor,
> + u8 opcode, u8 *buf, int len)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "read_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = opcode;
> + cmd.tx_data = true;
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = CCR_FMODE_INDR;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "write_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = opcode;
> + cmd.tx_data = !!(buf && len > 0);
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
> + u_char *buf)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> + struct stm32_qspi_cmd cmd;
> + int err;
> +
> + dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
> + nor->read_opcode, buf, (u32)from, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->read_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)from;
> + cmd.tx_data = true;
> + cmd.dummy = nor->read_dummy;
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = flash->read_mode;
> +
> + stm32_qspi_set_framemode(nor, &cmd, true);
> + err = stm32_qspi_send(flash, &cmd);
> +
> + return err ? err : len;
> +}
> +
> +static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
> + const u_char *buf)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> + int err;
> +
> + dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
> + nor->program_opcode, buf, (u32)to, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->program_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)to;
> + cmd.tx_data = true;
> + cmd.len = len;
> + cmd.buf = (void *)buf;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> + err = stm32_qspi_send(flash, &cmd);
> +
> + return err ? err : len;
> +}
> +
> +static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->erase_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)offs;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
> +{
> + struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
> + u32 cr, sr, fcr = 0;
> +
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
> +
> + if ((cr & CR_TCIE) && (sr & SR_TCF)) {
> + /* tx complete */
> + fcr |= FCR_CTCF;
> + complete(&qspi->cmd_completion);
> + } else {
> + dev_info_ratelimited(qspi->dev, "spurious interrupt\n");
> + }
> +
> + writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> +
> + mutex_lock(&qspi->lock);
> + return 0;
> +}
> +
> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> +
> + mutex_unlock(&qspi->lock);
> +}
> +
> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
> + struct device_node *np)
> +{
> + u32 width, flash_read, presc, cs_num, max_rate = 0;
> + struct stm32_qspi_flash *flash;
> + struct mtd_info *mtd;
> + int ret;
> +
> + of_property_read_u32(np, "reg", &cs_num);
> + if (cs_num >= STM32_MAX_NORCHIP)
> + return -EINVAL;
> +
> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
> + if (!max_rate)
> + return -EINVAL;
> +
> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
> +
> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
> + width = 1;
> +
> + if (width == 4)
> + flash_read = SPI_NOR_QUAD;
> + else if (width == 2)
> + flash_read = SPI_NOR_DUAL;
> + else if (width == 1)
> + flash_read = SPI_NOR_NORMAL;
> + else
> + return -EINVAL;
> +
> + flash = &qspi->flash[cs_num];
> + flash->qspi = qspi;
> + flash->cs = cs_num;
> + flash->presc = presc;
> +
> + flash->nor.dev = qspi->dev;
> + spi_nor_set_flash_node(&flash->nor, np);
> + flash->nor.priv = flash;
> + mtd = &flash->nor.mtd;
> + mtd->priv = &flash->nor;
This line is not needed: spi_nor_scan() already initializes mtd->priv
with the nor pointer. Not blocking anyway.
> +
> + flash->nor.read = stm32_qspi_read;
> + flash->nor.write = stm32_qspi_write;
> + flash->nor.erase = stm32_qspi_erase;
> + flash->nor.read_reg = stm32_qspi_read_reg;
> + flash->nor.write_reg = stm32_qspi_write_reg;
> + flash->nor.prepare = stm32_qspi_prep;
> + flash->nor.unprepare = stm32_qspi_unprep;
A shared comment: your implementations of all those handlers seem good
to me. I didn't notice any particular mistake or issue :)
> +
> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> + | CR_EN, qspi->io_base + QUADSPI_CR);
> +
> + /*
> + * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> + * which define the size of nor flash.
> + * if fsize is NULL, the controller can't sent spi-nor command.
> + * set a temporary value just to discover the nor flash with
> + * "spi_nor_scan". After, the right value (mtd->size) can be set.
> + */
> + flash->fsize = FSIZE_VAL(SZ_1K);
> +
> + ret = spi_nor_scan(&flash->nor, NULL, flash_read);
> + if (ret) {
> + dev_err(qspi->dev, "device scan failed\n");
At this point, (flash->qspi != NULL)
Hence in the control flow, once you have returned from
stm32_qspi__flash_setup() back into stm32_qspi_probe(), you handle the
reported error calling stm32_qspi_mtd_free().
This latest function test flash->qspi, which is not NULL, before calling
mtd_device_unregister(). However you did not register flash->nor.mtd at all.
Though not critical, can you fix this anyway, please?
I think, it's more a potential bug than a costmetic issue.
> + return ret;
> + }
> +
> + flash->fsize = FSIZE_VAL(mtd->size);
> +
> + flash->read_mode = CCR_FMODE_MM;
> + if (mtd->size > qspi->mm_size)
> + flash->read_mode = CCR_FMODE_INDR;
> +
> + writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret) {
> + dev_err(qspi->dev, "mtd device parse failed\n");
Almost the same mistake here: you shouldn't call mtd_device_unregister()
later from stm32_qspi_mtd_free() if mtd_devire_register() fails here.
> + return ret;
> + }
> +
> + dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> + flash->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> + return 0;
> +}
> +
> +static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
> +{
> + int i;
> +
> + for (i = 0; i < STM32_MAX_NORCHIP; i++) {
> + if (qspi->flash[i].qspi)
> + mtd_device_unregister(&qspi->flash[i].nor.mtd);
> + }
IMHO, I guess braces {} in the for() loop are not needed here. So if
checkpatch doesn't complain it's ok for me, you can leave it as is if
you want. Otherwise, please fix!
> +}
> +
> +static int stm32_qspi_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *flash_np;
> + struct reset_control *rstc;
> + struct stm32_qspi *qspi;
> + struct resource *res;
> + int ret, irq;
> +
> + qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
> + if (!qspi)
> + return -ENOMEM;
> +
> + qspi->nor_num = of_get_child_count(dev->of_node);
> + if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
> + return -ENODEV;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
> + qspi->io_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(qspi->io_base))
> + return PTR_ERR(qspi->io_base);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
> + qspi->mm_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(qspi->mm_base))
> + return PTR_ERR(qspi->mm_base);
> +
> + qspi->mm_size = resource_size(res);
> +
> + irq = platform_get_irq(pdev, 0);
> + ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
> + dev_name(dev), qspi);
> + if (ret) {
> + dev_err(dev, "failed to request irq\n");
> + return ret;
> + }
> +
> + init_completion(&qspi->cmd_completion);
> +
> + qspi->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(qspi->clk))
> + return PTR_ERR(qspi->clk);
> +
> + qspi->clk_rate = clk_get_rate(qspi->clk);
> + if (!qspi->clk_rate)
> + return -EINVAL;
> +
> + ret = clk_prepare_enable(qspi->clk);
> + if (ret) {
> + dev_err(dev, "can not enable the clock\n");
> + return ret;
> + }
> +
> + rstc = devm_reset_control_get(dev, NULL);
> + if (!IS_ERR(rstc)) {
> + reset_control_assert(rstc);
> + udelay(2);
> + reset_control_deassert(rstc);
> + }
> +
> + qspi->dev = dev;
> + platform_set_drvdata(pdev, qspi);
> + mutex_init(&qspi->lock);
> +
> + for_each_available_child_of_node(dev->of_node, flash_np) {
> + ret = stm32_qspi_flash_setup(qspi, flash_np);
> + if (ret) {
> + dev_err(dev, "unable to setup flash chip\n");
> + goto err_flash;
> + }
> + }
> +
> + return 0;
> +
> +err_flash:
> + mutex_destroy(&qspi->lock);
> + stm32_qspi_mtd_free(qspi);
> +
> + clk_disable_unprepare(qspi->clk);
> + return ret;
> +}
> +
> +static int stm32_qspi_remove(struct platform_device *pdev)
> +{
> + struct stm32_qspi *qspi = platform_get_drvdata(pdev);
> +
> + /* disable qspi */
> + writel_relaxed(0, qspi->io_base + QUADSPI_CR);
> +
> + stm32_qspi_mtd_free(qspi);
> + mutex_destroy(&qspi->lock);
> +
> + clk_disable_unprepare(qspi->clk);
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_qspi_match[] = {
> + {.compatible = "st,stm32f469-qspi"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, stm32_qspi_match);
> +
> +static struct platform_driver stm32_qspi_driver = {
> + .probe = stm32_qspi_probe,
> + .remove = stm32_qspi_remove,
> + .driver = {
> + .name = "stm32-quadspi",
> + .of_match_table = stm32_qspi_match,
> + },
> +};
> +module_platform_driver(stm32_qspi_driver);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_AUTHOR("Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
> +MODULE_LICENSE("GPL v2");
>
Best regards,
Cyrille
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/3] net: macb: Remove CONFIG_OF around DT match table
From: Rob Herring @ 2017-04-12 20:51 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel@vger.kernel.org, Andrew Lunn, Vivien Didelot,
Lee Jones, Nicolas Ferre, Frank Rowand,
open list:NETWORKING DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-3-f.fainelli@gmail.com>
On Tue, Apr 11, 2017 at 11:41 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> A subsequent patch is going to make of_match_node() an inline stub when
> CONFIG_OF is disabled, which will let the compiler eliminate unused variables.
> In order not to clutter the code more, remove the CONFIG_OF #ifdef such that
> macb_dt_ids and what it references are always defined.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 30606b11b128..01016e9525ee 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2811,7 +2811,6 @@ static int macb_init(struct platform_device *pdev)
> return 0;
> }
>
> -#if defined(CONFIG_OF)
> /* 1518 rounded up */
> #define AT91ETHER_MAX_RBUFF_SZ 0x600
> /* max number of receive buffers */
> @@ -3215,7 +3214,6 @@ static const struct of_device_id macb_dt_ids[] = {
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);
Did you actually check that the compiler can eliminate things when
modules are enabled? Because it's not going to be able to eliminate
MODULE_DEVICE_TABLE entry AFAICT.
Rob
^ permalink raw reply
* Re: [PATCH v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Cyrille Pitchen @ 2017-04-12 20:57 UTC (permalink / raw)
To: Guochun Mao, Cyrille Pitchen
Cc: Mark Rutland, devicetree, Richard Weinberger, Russell King,
linux-kernel, Rob Herring, linux-mtd, Matthias Brugger,
linux-mediatek, David Woodhouse, linux-arm-kernel
In-Reply-To: <1491381462-21893-2-git-send-email-guochun.mao@mediatek.com>
Hi Guochun,
Le 05/04/2017 à 10:37, Guochun Mao a écrit :
> When nor's size larger than 16MByte, nor's address width maybe
> set to 3 or 4, and controller should change address width according
> to nor's setting.
>
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
> drivers/mtd/spi-nor/mtk-quadspi.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> index e661877..b637770 100644
> --- a/drivers/mtd/spi-nor/mtk-quadspi.c
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -104,6 +104,8 @@
> #define MTK_NOR_MAX_RX_TX_SHIFT 6
> /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
> #define MTK_NOR_MAX_SHIFT 7
> +/* nor controller 4-byte address mode enable bit */
> +#define MTK_NOR_4B_ADDR_EN BIT(4)
>
> /* Helpers for accessing the program data / shift data registers */
> #define MTK_NOR_PRG_REG(n) (MTK_NOR_PRGDATA0_REG + 4 * (n))
> @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
> 10000);
> }
>
> +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
> +{
> + u8 val;
> + struct spi_nor *nor = &mt8173_nor->nor;
> +
> + val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
> +
> + switch (nor->addr_width) {
> + case 3:
> + val &= ~MTK_NOR_4B_ADDR_EN;
> + break;
> + case 4:
> + val |= MTK_NOR_4B_ADDR_EN;
> + break;
> + default:
> + dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
> + nor->addr_width);
> + break;
> + }
> +
> + writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
> +}
> +
> static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
> {
> int i;
>
> + mt8173_nor_set_addr_width(mt8173_nor);
> +
> for (i = 0; i < 3; i++) {
Should it be 'i < nor->addr_width' instead of 'i < 3' ?
Does it work when accessing data after 128Mbit ?
Best regards,
Cyrille
> writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
> addr >>= 8;
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox