This patchset adds support for the MSPI controller on Cygnus. The existing MSPI
driver in the kernel was written for BCMA which is a Broadcom AMBA bus variant
used on certain chips such as the 53xx.
This patch makes BCMA support optional. The current config is being renamed to
make it chip nonspecific supporting BCMA, and a new config is added to support
non-BCMA chips. DT support is now mandatory to allow removal of a hardcoded SPI
device.
Support is also added to set the baud rate. The controller currently runs at the
slowest speed possible.
Jonathan Richardson (4):
ARM: dts: Add binding for Broadcom MSPI driver.
spi: bcm53xx: Refactor to make driver nonspecific to 53xx SoCs
spi: bcm-mspi: Make BCMA optional to support non-BCMA chips
spi: bcm-mspi: Add support to set serial baud clock rate
.../devicetree/bindings/spi/brcm,mspi-spi.txt | 38 ++
drivers/spi/Kconfig | 12 +-
drivers/spi/Makefile | 3 +-
drivers/spi/spi-bcm-mspi.c | 453 ++++++++++++++++++++
drivers/spi/spi-bcm-mspi.h | 84 ++++
drivers/spi/spi-bcm53xx.c | 299 -------------
drivers/spi/spi-bcm53xx.h | 72 ----
7 files changed, 585 insertions(+), 376 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
create mode 100644 drivers/spi/spi-bcm-mspi.c
create mode 100644 drivers/spi/spi-bcm-mspi.h
delete mode 100644 drivers/spi/spi-bcm53xx.c
delete mode 100644 drivers/spi/spi-bcm53xx.h
--
1.7.9.5
Signed-off-by: Jonathan Richardson <[email protected]>
---
.../devicetree/bindings/spi/brcm,mspi-spi.txt | 38 ++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
diff --git a/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
new file mode 100644
index 0000000..16164e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
@@ -0,0 +1,38 @@
+Broadcom MSPI controller
+
+Required properties:
+- compatible: Must be either "brcm,mspi" or "brcm,bcma-mspi". Use
+ "brcm,bcma-mspi" for controllers on a bcma bus and "brcm,mspi" otherwise.
+
+- reg: Physical base address and length of the controller's registers.
+
+- interrupts: The interrupt id for the controller.
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 0.
+
+Optional properties:
+- clocks: The MSPI reference clock. If not provided then it is assumed a clock
+ is enabled by default and no control of clock-frequency (see below) is
+ possible.
+
+- clock-names: The name of the reference clock.
+
+- clock-frequency: Desired frequency of the clock. This will set the serial
+ clock baud rate (SPBR) based on the reference clock frequency. The frequency
+ of the SPBR is mspi_clk / (2 * SPBR) where SPBR is a value between 1-255
+ determined by the desired 'clock-frequency'. If not provided then the default
+ baud rate of the controller is used.
+
+Example:
+
+mspi@18047000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,mspi";
+ reg = <0x18047000 0x1000>;
+ clocks = <&axi41_clk>;
+ clock-names = "mspi_clk";
+ clock-frequency = <12500000>;
+};
--
1.7.9.5
The Broadcom MSPI controller is used on various SoCs. It is being
renamed so that it can be extended and reused on other chips. It is
renamed to bcm-mspi.
Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/Kconfig | 7 +-
drivers/spi/Makefile | 2 +-
drivers/spi/spi-bcm-mspi.c | 307 ++++++++++++++++++++++++++++++++++++++++++++
drivers/spi/spi-bcm-mspi.h | 84 ++++++++++++
drivers/spi/spi-bcm53xx.c | 299 ------------------------------------------
drivers/spi/spi-bcm53xx.h | 72 -----------
6 files changed, 395 insertions(+), 376 deletions(-)
create mode 100644 drivers/spi/spi-bcm-mspi.c
create mode 100644 drivers/spi/spi-bcm-mspi.h
delete mode 100644 drivers/spi/spi-bcm53xx.c
delete mode 100644 drivers/spi/spi-bcm53xx.h
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index ab8dfbe..766e08d 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -113,13 +113,12 @@ config SPI_AU1550
If you say yes to this option, support will be included for the
PSC SPI controller found on Au1550, Au1200 and Au1300 series.
-config SPI_BCM53XX
- tristate "Broadcom BCM53xx SPI controller"
- depends on ARCH_BCM_5301X
+config SPI_BCMA_MSPI
+ tristate "Broadcom BCMA MSPI controller"
depends on BCMA_POSSIBLE
select BCMA
help
- Enable support for the SPI controller on Broadcom BCM53xx ARM SoCs.
+ Enable support for the Broadcom BCMA MSPI controller.
config SPI_BCM63XX
tristate "Broadcom BCM63xx SPI controller"
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d8cbf65..6b58100 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_SPI_ATMEL) += spi-atmel.o
obj-$(CONFIG_SPI_ATH79) += spi-ath79.o
obj-$(CONFIG_SPI_AU1550) += spi-au1550.o
obj-$(CONFIG_SPI_BCM2835) += spi-bcm2835.o
-obj-$(CONFIG_SPI_BCM53XX) += spi-bcm53xx.o
+obj-$(CONFIG_SPI_BCMA_MSPI) += spi-bcm-mspi.o
obj-$(CONFIG_SPI_BCM63XX) += spi-bcm63xx.o
obj-$(CONFIG_SPI_BCM63XX_HSSPI) += spi-bcm63xx-hsspi.o
obj-$(CONFIG_SPI_BFIN5XX) += spi-bfin5xx.o
diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
new file mode 100644
index 0000000..67ea246
--- /dev/null
+++ b/drivers/spi/spi-bcm-mspi.c
@@ -0,0 +1,307 @@
+/*
+ * Portions Copyright (C) 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/bcma/bcma.h>
+#include <linux/spi/spi.h>
+
+#include "spi-bcm-mspi.h"
+
+#define BCM_MSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */
+
+/* The longest observed required wait was 19 ms */
+#define BCM_MSPI_SPE_TIMEOUT_MS 80
+
+struct bcm_mspi {
+ struct bcma_device *core;
+ struct spi_master *master;
+
+ size_t read_offset;
+};
+
+static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
+{
+ return bcma_read32(mspi->core, offset);
+}
+
+static inline void bcm_mspi_write(struct bcm_mspi *mspi, u16 offset,
+ u32 value)
+{
+ bcma_write32(mspi->core, offset, value);
+}
+
+static inline unsigned int bcm_mspi_calc_timeout(size_t len)
+{
+ /* Do some magic calculation based on length and buad. Add 10% and 1. */
+ return (len * 9000 / BCM_MSPI_MAX_SPI_BAUD * 110 / 100) + 1;
+}
+
+static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms)
+{
+ unsigned long deadline;
+ u32 tmp;
+
+ /* SPE bit has to be 0 before we read MSPI STATUS */
+ deadline = jiffies + BCM_MSPI_SPE_TIMEOUT_MS * HZ / 1000;
+ do {
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ if (!(tmp & MSPI_SPCR2_SPE))
+ break;
+ udelay(5);
+ } while (!time_after_eq(jiffies, deadline));
+
+ if (tmp & MSPI_SPCR2_SPE)
+ goto spi_timeout;
+
+ /* Check status */
+ deadline = jiffies + timeout_ms * HZ / 1000;
+ do {
+ tmp = bcm_mspi_read(mspi, MSPI_MSPI_STATUS);
+ if (tmp & MSPI_MSPI_STATUS_SPIF) {
+ bcm_mspi_write(mspi, MSPI_MSPI_STATUS, 0);
+ return 0;
+ }
+
+ cpu_relax();
+ udelay(100);
+ } while (!time_after_eq(jiffies, deadline));
+
+spi_timeout:
+ bcm_mspi_write(mspi, MSPI_MSPI_STATUS, 0);
+
+ pr_err("Timeout waiting for SPI to be ready!\n");
+
+ return -EBUSY;
+}
+
+static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
+ size_t len, bool cont)
+{
+ u32 tmp;
+ int i;
+
+ for (i = 0; i < len; i++) {
+ /* Transmit Register File MSB */
+ bcm_mspi_write(mspi, MSPI_TXRAM + 4 * (i * 2),
+ (unsigned int)w_buf[i]);
+ }
+
+ for (i = 0; i < len; i++) {
+ tmp = CDRAM_CONT | CDRAM_PCS_DISABLE_ALL | CDRAM_PCS_DSCK;
+ if (!cont && i == len - 1)
+ tmp &= ~CDRAM_CONT;
+ tmp &= ~0x1;
+ /* Command Register File */
+ bcm_mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
+ }
+
+ /* Set queue pointers */
+ bcm_mspi_write(mspi, MSPI_NEWQP, 0);
+ bcm_mspi_write(mspi, MSPI_ENDQP, len - 1);
+
+ if (cont)
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);
+
+ /* Start SPI transfer */
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp |= MSPI_SPCR2_SPE;
+ if (cont)
+ tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
+ bcm_mspi_write(mspi, MSPI_SPCR2, tmp);
+
+ /* Wait for SPI to finish */
+ bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));
+
+ if (!cont)
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);
+
+ mspi->read_offset = len;
+}
+
+static void bcm_mspi_buf_read(struct bcm_mspi *mspi, u8 *r_buf,
+ size_t len, bool cont)
+{
+ u32 tmp;
+ int i;
+
+ for (i = 0; i < mspi->read_offset + len; i++) {
+ tmp = CDRAM_CONT | CDRAM_PCS_DISABLE_ALL |
+ CDRAM_PCS_DSCK;
+ if (!cont && i == mspi->read_offset + len - 1)
+ tmp &= ~CDRAM_CONT;
+ tmp &= ~0x1;
+ /* Command Register File */
+ bcm_mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
+ }
+
+ /* Set queue pointers */
+ bcm_mspi_write(mspi, MSPI_NEWQP, 0);
+ bcm_mspi_write(mspi, MSPI_ENDQP,
+ mspi->read_offset + len - 1);
+
+ if (cont)
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);
+
+ /* Start SPI transfer */
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp |= MSPI_SPCR2_SPE;
+ if (cont)
+ tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
+ bcm_mspi_write(mspi, MSPI_SPCR2, tmp);
+
+ /* Wait for SPI to finish */
+ bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));
+
+ if (!cont)
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);
+
+ for (i = 0; i < len; ++i) {
+ int offset = mspi->read_offset + i;
+
+ /* Data stored in the transmit register file LSB */
+ r_buf[i] = (u8)bcm_mspi_read(mspi,
+ MSPI_RXRAM + 4 * (1 + offset * 2));
+ }
+
+ mspi->read_offset = 0;
+}
+
+static int bcm_mspi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *t)
+{
+ struct bcm_mspi *mspi = spi_master_get_devdata(master);
+ u8 *buf;
+ size_t left;
+
+ if (t->tx_buf) {
+ buf = (u8 *)t->tx_buf;
+ left = t->len;
+ while (left) {
+ size_t to_write = min_t(size_t, 16, left);
+ bool cont = left - to_write > 0;
+
+ bcm_mspi_buf_write(mspi, buf, to_write, cont);
+ left -= to_write;
+ buf += to_write;
+ }
+ }
+
+ if (t->rx_buf) {
+ buf = (u8 *)t->rx_buf;
+ left = t->len;
+ while (left) {
+ size_t to_read = min_t(size_t, 16 - mspi->read_offset,
+ left);
+ bool cont = left - to_read > 0;
+
+ bcm_mspi_buf_read(mspi, buf, to_read, cont);
+ left -= to_read;
+ buf += to_read;
+ }
+ }
+
+ return 0;
+}
+
+static struct spi_board_info bcm53xx_info = {
+ .modalias = "bcm53xxspiflash",
+};
+
+static const struct bcma_device_id bcm_mspi_bcma_tbl[] = {
+ BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NS_QSPI, BCMA_ANY_REV,
+ BCMA_ANY_CLASS),
+ {},
+};
+MODULE_DEVICE_TABLE(bcma, bcm_mspi_bcma_tbl);
+
+static inline u32 bcm_bcma_mspi_read(struct bcm_mspi *mspi, u16 offset)
+{
+ return bcma_read32(mspi->core, offset);
+}
+
+static inline void bcm_bcma_mspi_write(struct bcm_mspi *mspi, u16 offset,
+ u32 value)
+{
+ bcma_write32(mspi->core, offset, value);
+}
+
+static int bcm_mspi_bcma_probe(struct bcma_device *core)
+{
+ struct bcm_mspi *data;
+ struct spi_master *master;
+ int err;
+
+ dev_info(&core->dev, "BCM MSPI BCMA probe\n");
+
+ if (core->bus->drv_cc.core->id.rev != 42) {
+ pr_err("SPI on SoC with unsupported ChipCommon rev\n");
+ return -ENOTSUPP;
+ }
+
+ master = spi_alloc_master(&core->dev, sizeof(*data));
+ if (!master)
+ return -ENOMEM;
+
+ data = spi_master_get_devdata(master);
+ data->master = master;
+ data->core = core;
+
+ master->transfer_one = bcm_mspi_transfer_one;
+ bcma_set_drvdata(core, data);
+
+ err = devm_spi_register_master(&core->dev, data->master);
+ if (err) {
+ spi_master_put(master);
+ bcma_set_drvdata(core, NULL);
+ goto out;
+ }
+
+ /* Broadcom SoCs (at least with the CC rev 42) use SPI for flash only */
+ spi_new_device(master, &bcm53xx_info);
+
+out:
+ return err;
+}
+
+static struct bcma_driver bcm_mspi_bcma_driver = {
+ .name = KBUILD_MODNAME,
+ .id_table = bcm_mspi_bcma_tbl,
+ .probe = bcm_mspi_bcma_probe,
+};
+
+static int __init bcm_mspi_bcma_module_init(void)
+{
+ int err = 0;
+
+ err = bcma_driver_register(&bcm_mspi_bcma_driver);
+ if (err)
+ pr_err("Failed to register bcma driver: %d\n", err);
+
+ return err;
+}
+
+static void __exit bcm_mspi_bcma_module_exit(void)
+{
+ bcma_driver_unregister(&bcm_mspi_bcma_driver);
+}
+
+module_init(bcm_mspi_bcma_module_init);
+module_exit(bcm_mspi_bcma_module_exit);
+
+MODULE_DESCRIPTION("Broadcom MSPI SPI Controller driver");
+MODULE_AUTHOR("Rafał Miłecki <[email protected]>");
+MODULE_AUTHOR("Broadcom");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/spi/spi-bcm-mspi.h b/drivers/spi/spi-bcm-mspi.h
new file mode 100644
index 0000000..d633d5b
--- /dev/null
+++ b/drivers/spi/spi-bcm-mspi.h
@@ -0,0 +1,84 @@
+/*
+ * Portions Copyright (C) 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef SPI_BCM_MSPI_H
+#define SPI_BCM_MSPI_H
+
+#define BSPI_REVISION_ID 0x000
+#define BSPI_SCRATCH 0x004
+#define BSPI_MAST_N_BOOT_CTRL 0x008
+#define BSPI_BUSY_STATUS 0x00c
+#define BSPI_INTR_STATUS 0x010
+#define BSPI_B0_STATUS 0x014
+#define BSPI_B0_CTRL 0x018
+#define BSPI_B1_STATUS 0x01c
+#define BSPI_B1_CTRL 0x020
+#define BSPI_STRAP_OVERRIDE_CTRL 0x024
+#define BSPI_FLEX_MODE_ENABLE 0x028
+#define BSPI_BITS_PER_CYCLE 0x02c
+#define BSPI_BITS_PER_PHASE 0x030
+#define BSPI_CMD_AND_MODE_BYTE 0x034
+#define BSPI_BSPI_FLASH_UPPER_ADDR_BYTE 0x038
+#define BSPI_BSPI_XOR_VALUE 0x03c
+#define BSPI_BSPI_XOR_ENABLE 0x040
+#define BSPI_BSPI_PIO_MODE_ENABLE 0x044
+#define BSPI_BSPI_PIO_IODIR 0x048
+#define BSPI_BSPI_PIO_DATA 0x04c
+
+/* RAF */
+#define RAF_START_ADDR 0x100
+#define RAF_NUM_WORDS 0x104
+#define RAF_CTRL 0x108
+#define RAF_FULLNESS 0x10c
+#define RAF_WATERMARK 0x110
+#define RAF_STATUS 0x114
+#define RAF_READ_DATA 0x118
+#define RAF_WORD_CNT 0x11c
+#define RAF_CURR_ADDR 0x120
+
+/* MSPI */
+#define MSPI_SPCR0_LSB 0x200
+#define MSPI_SPCR0_MSB 0x204
+#define MSPI_SPCR1_LSB 0x208
+#define MSPI_SPCR1_MSB 0x20c
+#define MSPI_NEWQP 0x210
+#define MSPI_ENDQP 0x214
+#define MSPI_SPCR2 0x218
+#define MSPI_SPCR2_SPE 0x00000040
+#define MSPI_SPCR2_CONT_AFTER_CMD 0x00000080
+#define MSPI_MSPI_STATUS 0x220
+#define MSPI_MSPI_STATUS_SPIF 0x00000001
+#define MSPI_CPTQP 0x224
+#define MSPI_TXRAM 0x240 /* 32 registers, up to 0x2b8 */
+#define MSPI_RXRAM 0x2c0 /* 32 registers, up to 0x33c */
+#define MSPI_CDRAM 0x340 /* 16 registers, up to 0x37c */
+#define CDRAM_PCS_PCS0 0x00000001
+#define CDRAM_PCS_PCS1 0x00000002
+#define CDRAM_PCS_PCS2 0x00000004
+#define CDRAM_PCS_PCS3 0x00000008
+#define CDRAM_PCS_DISABLE_ALL 0x0000000f
+#define CDRAM_PCS_DSCK 0x00000010
+#define CDRAM_BITSE 0x00000040
+#define CDRAM_CONT 0x00000080
+#define MSPI_WRITE_LOCK 0x380
+#define MSPI_DISABLE_FLUSH_GEN 0x384
+
+/* Interrupt */
+#define INTR_RAF_LR_FULLNESS_REACHED 0x3a0
+#define INTR_RAF_LR_TRUNCATED 0x3a4
+#define INTR_RAF_LR_IMPATIENT 0x3a8
+#define INTR_RAF_LR_SESSION_DONE 0x3ac
+#define INTR_RAF_LR_OVERREAD 0x3b0
+#define INTR_MSPI_DONE 0x3b4
+#define INTR_MSPI_HALT_SET_TRANSACTION_DONE 0x3b8
+
+#endif
diff --git a/drivers/spi/spi-bcm53xx.c b/drivers/spi/spi-bcm53xx.c
deleted file mode 100644
index 3fb91c8..0000000
--- a/drivers/spi/spi-bcm53xx.c
+++ /dev/null
@@ -1,299 +0,0 @@
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/delay.h>
-#include <linux/bcma/bcma.h>
-#include <linux/spi/spi.h>
-
-#include "spi-bcm53xx.h"
-
-#define BCM53XXSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */
-
-/* The longest observed required wait was 19 ms */
-#define BCM53XXSPI_SPE_TIMEOUT_MS 80
-
-struct bcm53xxspi {
- struct bcma_device *core;
- struct spi_master *master;
-
- size_t read_offset;
-};
-
-static inline u32 bcm53xxspi_read(struct bcm53xxspi *b53spi, u16 offset)
-{
- return bcma_read32(b53spi->core, offset);
-}
-
-static inline void bcm53xxspi_write(struct bcm53xxspi *b53spi, u16 offset,
- u32 value)
-{
- bcma_write32(b53spi->core, offset, value);
-}
-
-static inline unsigned int bcm53xxspi_calc_timeout(size_t len)
-{
- /* Do some magic calculation based on length and buad. Add 10% and 1. */
- return (len * 9000 / BCM53XXSPI_MAX_SPI_BAUD * 110 / 100) + 1;
-}
-
-static int bcm53xxspi_wait(struct bcm53xxspi *b53spi, unsigned int timeout_ms)
-{
- unsigned long deadline;
- u32 tmp;
-
- /* SPE bit has to be 0 before we read MSPI STATUS */
- deadline = jiffies + BCM53XXSPI_SPE_TIMEOUT_MS * HZ / 1000;
- do {
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- if (!(tmp & B53SPI_MSPI_SPCR2_SPE))
- break;
- udelay(5);
- } while (!time_after_eq(jiffies, deadline));
-
- if (tmp & B53SPI_MSPI_SPCR2_SPE)
- goto spi_timeout;
-
- /* Check status */
- deadline = jiffies + timeout_ms * HZ / 1000;
- do {
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_MSPI_STATUS);
- if (tmp & B53SPI_MSPI_MSPI_STATUS_SPIF) {
- bcm53xxspi_write(b53spi, B53SPI_MSPI_MSPI_STATUS, 0);
- return 0;
- }
-
- cpu_relax();
- udelay(100);
- } while (!time_after_eq(jiffies, deadline));
-
-spi_timeout:
- bcm53xxspi_write(b53spi, B53SPI_MSPI_MSPI_STATUS, 0);
-
- pr_err("Timeout waiting for SPI to be ready!\n");
-
- return -EBUSY;
-}
-
-static void bcm53xxspi_buf_write(struct bcm53xxspi *b53spi, u8 *w_buf,
- size_t len, bool cont)
-{
- u32 tmp;
- int i;
-
- for (i = 0; i < len; i++) {
- /* Transmit Register File MSB */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_TXRAM + 4 * (i * 2),
- (unsigned int)w_buf[i]);
- }
-
- for (i = 0; i < len; i++) {
- tmp = B53SPI_CDRAM_CONT | B53SPI_CDRAM_PCS_DISABLE_ALL |
- B53SPI_CDRAM_PCS_DSCK;
- if (!cont && i == len - 1)
- tmp &= ~B53SPI_CDRAM_CONT;
- tmp &= ~0x1;
- /* Command Register File */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_CDRAM + 4 * i, tmp);
- }
-
- /* Set queue pointers */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
- bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP, len - 1);
-
- if (cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
-
- /* Start SPI transfer */
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- tmp |= B53SPI_MSPI_SPCR2_SPE;
- if (cont)
- tmp |= B53SPI_MSPI_SPCR2_CONT_AFTER_CMD;
- bcm53xxspi_write(b53spi, B53SPI_MSPI_SPCR2, tmp);
-
- /* Wait for SPI to finish */
- bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
-
- if (!cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
-
- b53spi->read_offset = len;
-}
-
-static void bcm53xxspi_buf_read(struct bcm53xxspi *b53spi, u8 *r_buf,
- size_t len, bool cont)
-{
- u32 tmp;
- int i;
-
- for (i = 0; i < b53spi->read_offset + len; i++) {
- tmp = B53SPI_CDRAM_CONT | B53SPI_CDRAM_PCS_DISABLE_ALL |
- B53SPI_CDRAM_PCS_DSCK;
- if (!cont && i == b53spi->read_offset + len - 1)
- tmp &= ~B53SPI_CDRAM_CONT;
- tmp &= ~0x1;
- /* Command Register File */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_CDRAM + 4 * i, tmp);
- }
-
- /* Set queue pointers */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
- bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP,
- b53spi->read_offset + len - 1);
-
- if (cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
-
- /* Start SPI transfer */
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- tmp |= B53SPI_MSPI_SPCR2_SPE;
- if (cont)
- tmp |= B53SPI_MSPI_SPCR2_CONT_AFTER_CMD;
- bcm53xxspi_write(b53spi, B53SPI_MSPI_SPCR2, tmp);
-
- /* Wait for SPI to finish */
- bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
-
- if (!cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
-
- for (i = 0; i < len; ++i) {
- int offset = b53spi->read_offset + i;
-
- /* Data stored in the transmit register file LSB */
- r_buf[i] = (u8)bcm53xxspi_read(b53spi, B53SPI_MSPI_RXRAM + 4 * (1 + offset * 2));
- }
-
- b53spi->read_offset = 0;
-}
-
-static int bcm53xxspi_transfer_one(struct spi_master *master,
- struct spi_device *spi,
- struct spi_transfer *t)
-{
- struct bcm53xxspi *b53spi = spi_master_get_devdata(master);
- u8 *buf;
- size_t left;
-
- if (t->tx_buf) {
- buf = (u8 *)t->tx_buf;
- left = t->len;
- while (left) {
- size_t to_write = min_t(size_t, 16, left);
- bool cont = left - to_write > 0;
-
- bcm53xxspi_buf_write(b53spi, buf, to_write, cont);
- left -= to_write;
- buf += to_write;
- }
- }
-
- if (t->rx_buf) {
- buf = (u8 *)t->rx_buf;
- left = t->len;
- while (left) {
- size_t to_read = min_t(size_t, 16 - b53spi->read_offset,
- left);
- bool cont = left - to_read > 0;
-
- bcm53xxspi_buf_read(b53spi, buf, to_read, cont);
- left -= to_read;
- buf += to_read;
- }
- }
-
- return 0;
-}
-
-/**************************************************
- * BCMA
- **************************************************/
-
-static struct spi_board_info bcm53xx_info = {
- .modalias = "bcm53xxspiflash",
-};
-
-static const struct bcma_device_id bcm53xxspi_bcma_tbl[] = {
- BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NS_QSPI, BCMA_ANY_REV, BCMA_ANY_CLASS),
- {},
-};
-MODULE_DEVICE_TABLE(bcma, bcm53xxspi_bcma_tbl);
-
-static int bcm53xxspi_bcma_probe(struct bcma_device *core)
-{
- struct bcm53xxspi *b53spi;
- struct spi_master *master;
- int err;
-
- if (core->bus->drv_cc.core->id.rev != 42) {
- pr_err("SPI on SoC with unsupported ChipCommon rev\n");
- return -ENOTSUPP;
- }
-
- master = spi_alloc_master(&core->dev, sizeof(*b53spi));
- if (!master)
- return -ENOMEM;
-
- b53spi = spi_master_get_devdata(master);
- b53spi->master = master;
- b53spi->core = core;
-
- master->transfer_one = bcm53xxspi_transfer_one;
-
- bcma_set_drvdata(core, b53spi);
-
- err = devm_spi_register_master(&core->dev, master);
- if (err) {
- spi_master_put(master);
- bcma_set_drvdata(core, NULL);
- goto out;
- }
-
- /* Broadcom SoCs (at least with the CC rev 42) use SPI for flash only */
- spi_new_device(master, &bcm53xx_info);
-
-out:
- return err;
-}
-
-static void bcm53xxspi_bcma_remove(struct bcma_device *core)
-{
- struct bcm53xxspi *b53spi = bcma_get_drvdata(core);
-
- spi_unregister_master(b53spi->master);
-}
-
-static struct bcma_driver bcm53xxspi_bcma_driver = {
- .name = KBUILD_MODNAME,
- .id_table = bcm53xxspi_bcma_tbl,
- .probe = bcm53xxspi_bcma_probe,
- .remove = bcm53xxspi_bcma_remove,
-};
-
-/**************************************************
- * Init & exit
- **************************************************/
-
-static int __init bcm53xxspi_module_init(void)
-{
- int err = 0;
-
- err = bcma_driver_register(&bcm53xxspi_bcma_driver);
- if (err)
- pr_err("Failed to register bcma driver: %d\n", err);
-
- return err;
-}
-
-static void __exit bcm53xxspi_module_exit(void)
-{
- bcma_driver_unregister(&bcm53xxspi_bcma_driver);
-}
-
-module_init(bcm53xxspi_module_init);
-module_exit(bcm53xxspi_module_exit);
-
-MODULE_DESCRIPTION("Broadcom BCM53xx SPI Controller driver");
-MODULE_AUTHOR("Rafał Miłecki <[email protected]>");
-MODULE_LICENSE("GPL");
diff --git a/drivers/spi/spi-bcm53xx.h b/drivers/spi/spi-bcm53xx.h
deleted file mode 100644
index 73575df..0000000
--- a/drivers/spi/spi-bcm53xx.h
+++ /dev/null
@@ -1,72 +0,0 @@
-#ifndef SPI_BCM53XX_H
-#define SPI_BCM53XX_H
-
-#define B53SPI_BSPI_REVISION_ID 0x000
-#define B53SPI_BSPI_SCRATCH 0x004
-#define B53SPI_BSPI_MAST_N_BOOT_CTRL 0x008
-#define B53SPI_BSPI_BUSY_STATUS 0x00c
-#define B53SPI_BSPI_INTR_STATUS 0x010
-#define B53SPI_BSPI_B0_STATUS 0x014
-#define B53SPI_BSPI_B0_CTRL 0x018
-#define B53SPI_BSPI_B1_STATUS 0x01c
-#define B53SPI_BSPI_B1_CTRL 0x020
-#define B53SPI_BSPI_STRAP_OVERRIDE_CTRL 0x024
-#define B53SPI_BSPI_FLEX_MODE_ENABLE 0x028
-#define B53SPI_BSPI_BITS_PER_CYCLE 0x02c
-#define B53SPI_BSPI_BITS_PER_PHASE 0x030
-#define B53SPI_BSPI_CMD_AND_MODE_BYTE 0x034
-#define B53SPI_BSPI_BSPI_FLASH_UPPER_ADDR_BYTE 0x038
-#define B53SPI_BSPI_BSPI_XOR_VALUE 0x03c
-#define B53SPI_BSPI_BSPI_XOR_ENABLE 0x040
-#define B53SPI_BSPI_BSPI_PIO_MODE_ENABLE 0x044
-#define B53SPI_BSPI_BSPI_PIO_IODIR 0x048
-#define B53SPI_BSPI_BSPI_PIO_DATA 0x04c
-
-/* RAF */
-#define B53SPI_RAF_START_ADDR 0x100
-#define B53SPI_RAF_NUM_WORDS 0x104
-#define B53SPI_RAF_CTRL 0x108
-#define B53SPI_RAF_FULLNESS 0x10c
-#define B53SPI_RAF_WATERMARK 0x110
-#define B53SPI_RAF_STATUS 0x114
-#define B53SPI_RAF_READ_DATA 0x118
-#define B53SPI_RAF_WORD_CNT 0x11c
-#define B53SPI_RAF_CURR_ADDR 0x120
-
-/* MSPI */
-#define B53SPI_MSPI_SPCR0_LSB 0x200
-#define B53SPI_MSPI_SPCR0_MSB 0x204
-#define B53SPI_MSPI_SPCR1_LSB 0x208
-#define B53SPI_MSPI_SPCR1_MSB 0x20c
-#define B53SPI_MSPI_NEWQP 0x210
-#define B53SPI_MSPI_ENDQP 0x214
-#define B53SPI_MSPI_SPCR2 0x218
-#define B53SPI_MSPI_SPCR2_SPE 0x00000040
-#define B53SPI_MSPI_SPCR2_CONT_AFTER_CMD 0x00000080
-#define B53SPI_MSPI_MSPI_STATUS 0x220
-#define B53SPI_MSPI_MSPI_STATUS_SPIF 0x00000001
-#define B53SPI_MSPI_CPTQP 0x224
-#define B53SPI_MSPI_TXRAM 0x240 /* 32 registers, up to 0x2b8 */
-#define B53SPI_MSPI_RXRAM 0x2c0 /* 32 registers, up to 0x33c */
-#define B53SPI_MSPI_CDRAM 0x340 /* 16 registers, up to 0x37c */
-#define B53SPI_CDRAM_PCS_PCS0 0x00000001
-#define B53SPI_CDRAM_PCS_PCS1 0x00000002
-#define B53SPI_CDRAM_PCS_PCS2 0x00000004
-#define B53SPI_CDRAM_PCS_PCS3 0x00000008
-#define B53SPI_CDRAM_PCS_DISABLE_ALL 0x0000000f
-#define B53SPI_CDRAM_PCS_DSCK 0x00000010
-#define B53SPI_CDRAM_BITSE 0x00000040
-#define B53SPI_CDRAM_CONT 0x00000080
-#define B53SPI_MSPI_WRITE_LOCK 0x380
-#define B53SPI_MSPI_DISABLE_FLUSH_GEN 0x384
-
-/* Interrupt */
-#define B53SPI_INTR_RAF_LR_FULLNESS_REACHED 0x3a0
-#define B53SPI_INTR_RAF_LR_TRUNCATED 0x3a4
-#define B53SPI_INTR_RAF_LR_IMPATIENT 0x3a8
-#define B53SPI_INTR_RAF_LR_SESSION_DONE 0x3ac
-#define B53SPI_INTR_RAF_LR_OVERREAD 0x3b0
-#define B53SPI_INTR_MSPI_DONE 0x3b4
-#define B53SPI_INTR_MSPI_HALT_SET_TRANSACTION_DONE 0x3b8
-
-#endif /* SPI_BCM53XX_H */
--
1.7.9.5
The Broadcom MSPI controller is used on various chips. The driver only
supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
refactored to make BCMA optional and provides a new config for non BCMA
systems.
Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/Kconfig | 5 ++
drivers/spi/Makefile | 1 +
drivers/spi/spi-bcm-mspi.c | 200 +++++++++++++++++++++++++++++++++-----------
3 files changed, 155 insertions(+), 51 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 766e08d..23f2357 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -120,6 +120,11 @@ config SPI_BCMA_MSPI
help
Enable support for the Broadcom BCMA MSPI controller.
+config SPI_BCM_MSPI
+ tristate "Broadcom MSPI controller"
+ help
+ Enable support for the Broadcom MSPI controller.
+
config SPI_BCM63XX
tristate "Broadcom BCM63xx SPI controller"
depends on BCM63XX
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 6b58100..36872d2 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_SPI_ATH79) += spi-ath79.o
obj-$(CONFIG_SPI_AU1550) += spi-au1550.o
obj-$(CONFIG_SPI_BCM2835) += spi-bcm2835.o
obj-$(CONFIG_SPI_BCMA_MSPI) += spi-bcm-mspi.o
+obj-$(CONFIG_SPI_BCM_MSPI) += spi-bcm-mspi.o
obj-$(CONFIG_SPI_BCM63XX) += spi-bcm63xx.o
obj-$(CONFIG_SPI_BCM63XX_HSSPI) += spi-bcm63xx-hsspi.o
obj-$(CONFIG_SPI_BFIN5XX) += spi-bfin5xx.o
diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
index 67ea246..df27449 100644
--- a/drivers/spi/spi-bcm-mspi.c
+++ b/drivers/spi/spi-bcm-mspi.c
@@ -11,11 +11,13 @@
* GNU General Public License for more details.
*/
#include <linux/kernel.h>
+#include <linux/platform_device.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/bcma/bcma.h>
#include <linux/spi/spi.h>
+#include <linux/of.h>
#include "spi-bcm-mspi.h"
@@ -25,22 +27,17 @@
#define BCM_MSPI_SPE_TIMEOUT_MS 80
struct bcm_mspi {
+ #ifdef CONFIG_SPI_BCMA_MSPI
struct bcma_device *core;
- struct spi_master *master;
+ #endif
+ void __iomem *base;
+ struct spi_master *master;
size_t read_offset;
-};
-static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
-{
- return bcma_read32(mspi->core, offset);
-}
-
-static inline void bcm_mspi_write(struct bcm_mspi *mspi, u16 offset,
- u32 value)
-{
- bcma_write32(mspi->core, offset, value);
-}
+ void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
+ u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
+};
static inline unsigned int bcm_mspi_calc_timeout(size_t len)
{
@@ -56,7 +53,7 @@ static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms)
/* SPE bit has to be 0 before we read MSPI STATUS */
deadline = jiffies + BCM_MSPI_SPE_TIMEOUT_MS * HZ / 1000;
do {
- tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp = mspi->mspi_read(mspi, MSPI_SPCR2);
if (!(tmp & MSPI_SPCR2_SPE))
break;
udelay(5);
@@ -68,9 +65,9 @@ static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms)
/* Check status */
deadline = jiffies + timeout_ms * HZ / 1000;
do {
- tmp = bcm_mspi_read(mspi, MSPI_MSPI_STATUS);
+ tmp = mspi->mspi_read(mspi, MSPI_MSPI_STATUS);
if (tmp & MSPI_MSPI_STATUS_SPIF) {
- bcm_mspi_write(mspi, MSPI_MSPI_STATUS, 0);
+ mspi->mspi_write(mspi, MSPI_MSPI_STATUS, 0);
return 0;
}
@@ -79,7 +76,7 @@ static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms)
} while (!time_after_eq(jiffies, deadline));
spi_timeout:
- bcm_mspi_write(mspi, MSPI_MSPI_STATUS, 0);
+ mspi->mspi_write(mspi, MSPI_MSPI_STATUS, 0);
pr_err("Timeout waiting for SPI to be ready!\n");
@@ -94,7 +91,7 @@ static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
for (i = 0; i < len; i++) {
/* Transmit Register File MSB */
- bcm_mspi_write(mspi, MSPI_TXRAM + 4 * (i * 2),
+ mspi->mspi_write(mspi, MSPI_TXRAM + 4 * (i * 2),
(unsigned int)w_buf[i]);
}
@@ -104,28 +101,28 @@ static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
tmp &= ~CDRAM_CONT;
tmp &= ~0x1;
/* Command Register File */
- bcm_mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
+ mspi->mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
}
/* Set queue pointers */
- bcm_mspi_write(mspi, MSPI_NEWQP, 0);
- bcm_mspi_write(mspi, MSPI_ENDQP, len - 1);
+ mspi->mspi_write(mspi, MSPI_NEWQP, 0);
+ mspi->mspi_write(mspi, MSPI_ENDQP, len - 1);
if (cont)
- bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);
+ mspi->mspi_write(mspi, MSPI_WRITE_LOCK, 1);
/* Start SPI transfer */
- tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp = mspi->mspi_read(mspi, MSPI_SPCR2);
tmp |= MSPI_SPCR2_SPE;
if (cont)
tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
- bcm_mspi_write(mspi, MSPI_SPCR2, tmp);
+ mspi->mspi_write(mspi, MSPI_SPCR2, tmp);
/* Wait for SPI to finish */
bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));
if (!cont)
- bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);
+ mspi->mspi_write(mspi, MSPI_WRITE_LOCK, 0);
mspi->read_offset = len;
}
@@ -143,35 +140,35 @@ static void bcm_mspi_buf_read(struct bcm_mspi *mspi, u8 *r_buf,
tmp &= ~CDRAM_CONT;
tmp &= ~0x1;
/* Command Register File */
- bcm_mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
+ mspi->mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
}
/* Set queue pointers */
- bcm_mspi_write(mspi, MSPI_NEWQP, 0);
- bcm_mspi_write(mspi, MSPI_ENDQP,
+ mspi->mspi_write(mspi, MSPI_NEWQP, 0);
+ mspi->mspi_write(mspi, MSPI_ENDQP,
mspi->read_offset + len - 1);
if (cont)
- bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);
+ mspi->mspi_write(mspi, MSPI_WRITE_LOCK, 1);
/* Start SPI transfer */
- tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp = mspi->mspi_read(mspi, MSPI_SPCR2);
tmp |= MSPI_SPCR2_SPE;
if (cont)
tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
- bcm_mspi_write(mspi, MSPI_SPCR2, tmp);
+ mspi->mspi_write(mspi, MSPI_SPCR2, tmp);
/* Wait for SPI to finish */
bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));
if (!cont)
- bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);
+ mspi->mspi_write(mspi, MSPI_WRITE_LOCK, 0);
for (i = 0; i < len; ++i) {
int offset = mspi->read_offset + i;
/* Data stored in the transmit register file LSB */
- r_buf[i] = (u8)bcm_mspi_read(mspi,
+ r_buf[i] = (u8)mspi->mspi_read(mspi,
MSPI_RXRAM + 4 * (1 + offset * 2));
}
@@ -216,9 +213,103 @@ static int bcm_mspi_transfer_one(struct spi_master *master,
return 0;
}
-static struct spi_board_info bcm53xx_info = {
- .modalias = "bcm53xxspiflash",
+/*
+ * Allocate SPI master for both bcma and non bcma bus. The SPI device must be
+ * configured in DT.
+ */
+static struct bcm_mspi *bcm_mspi_init(struct device *dev)
+{
+ struct bcm_mspi *data;
+ struct spi_master *master;
+
+ master = spi_alloc_master(dev, sizeof(*data));
+ if (!master) {
+ dev_err(dev, "error allocating spi_master\n");
+ return 0;
+ }
+
+ data = spi_master_get_devdata(master);
+ data->master = master;
+
+ /* SPI master will always use the SPI device(s) from DT. */
+ master->dev.of_node = dev->of_node;
+ master->transfer_one = bcm_mspi_transfer_one;
+
+ return data;
+}
+
+#ifdef CONFIG_SPI_BCM_MSPI
+
+static const struct of_device_id bcm_mspi_dt[] = {
+ { .compatible = "brcm,mspi" },
+ { },
};
+MODULE_DEVICE_TABLE(of, bcm_mspi_dt);
+
+static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
+{
+ return readl(mspi->base + offset);
+}
+
+static inline void bcm_mspi_write(struct bcm_mspi *mspi, u16 offset,
+ u32 value)
+{
+ writel(value, mspi->base + offset);
+}
+
+/*
+ * Probe routine for non-bcma devices.
+ */
+static int bcm_mspi_probe(struct platform_device *pdev)
+{
+ struct bcm_mspi *data;
+ struct device *dev = &pdev->dev;
+ int err;
+ struct resource *res;
+
+ dev_info(dev, "BCM MSPI probe\n");
+
+ data = bcm_mspi_init(dev);
+ if (!data)
+ return -ENOMEM;
+
+ /* Map base memory address. */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data->base)) {
+ dev_err(&pdev->dev, "unable to map I/O memory\n");
+ err = PTR_ERR(data->base);
+ goto out;
+ }
+
+ data->mspi_read = bcm_mspi_read;
+ data->mspi_write = bcm_mspi_write;
+ platform_set_drvdata(pdev, data);
+
+ err = devm_spi_register_master(dev, data->master);
+ if (err)
+ goto out;
+
+ return 0;
+
+out:
+ spi_master_put(data->master);
+ return err;
+}
+
+static struct platform_driver bcm_mspi_driver = {
+ .driver = {
+ .name = "bcm-mspi",
+ .of_match_table = bcm_mspi_dt,
+ },
+ .probe = bcm_mspi_probe,
+};
+
+module_platform_driver(bcm_mspi_driver);
+
+#endif
+
+#ifdef CONFIG_SPI_BCMA_MSPI
static const struct bcma_device_id bcm_mspi_bcma_tbl[] = {
BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NS_QSPI, BCMA_ANY_REV,
@@ -227,6 +318,12 @@ static const struct bcma_device_id bcm_mspi_bcma_tbl[] = {
};
MODULE_DEVICE_TABLE(bcma, bcm_mspi_bcma_tbl);
+static const struct of_device_id bcm_bcma_mspi_dt[] = {
+ { .compatible = "brcm,bcma-mspi" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bcm_mspi_dt);
+
static inline u32 bcm_bcma_mspi_read(struct bcm_mspi *mspi, u16 offset)
{
return bcma_read32(mspi->core, offset);
@@ -238,53 +335,52 @@ static inline void bcm_bcma_mspi_write(struct bcm_mspi *mspi, u16 offset,
bcma_write32(mspi->core, offset, value);
}
+/*
+ * Probe routine for bcma devices.
+ */
static int bcm_mspi_bcma_probe(struct bcma_device *core)
{
struct bcm_mspi *data;
- struct spi_master *master;
int err;
dev_info(&core->dev, "BCM MSPI BCMA probe\n");
if (core->bus->drv_cc.core->id.rev != 42) {
- pr_err("SPI on SoC with unsupported ChipCommon rev\n");
+ dev_err(&core->dev,
+ "SPI on SoC with unsupported ChipCommon rev\n");
return -ENOTSUPP;
}
- master = spi_alloc_master(&core->dev, sizeof(*data));
- if (!master)
+ data = bcm_mspi_init(&core->dev);
+ if (!data)
return -ENOMEM;
- data = spi_master_get_devdata(master);
- data->master = master;
+ data->mspi_read = bcm_bcma_mspi_read;
+ data->mspi_write = bcm_bcma_mspi_write;
data->core = core;
-
- master->transfer_one = bcm_mspi_transfer_one;
bcma_set_drvdata(core, data);
err = devm_spi_register_master(&core->dev, data->master);
if (err) {
- spi_master_put(master);
- bcma_set_drvdata(core, NULL);
- goto out;
+ spi_master_put(data->master);
+ return err;
}
- /* Broadcom SoCs (at least with the CC rev 42) use SPI for flash only */
- spi_new_device(master, &bcm53xx_info);
-
-out:
- return err;
+ return 0;
}
static struct bcma_driver bcm_mspi_bcma_driver = {
.name = KBUILD_MODNAME,
+ .drv = {
+ .of_match_table = bcm_bcma_mspi_dt,
+ },
.id_table = bcm_mspi_bcma_tbl,
.probe = bcm_mspi_bcma_probe,
};
static int __init bcm_mspi_bcma_module_init(void)
{
- int err = 0;
+ int err;
err = bcma_driver_register(&bcm_mspi_bcma_driver);
if (err)
@@ -301,6 +397,8 @@ static void __exit bcm_mspi_bcma_module_exit(void)
module_init(bcm_mspi_bcma_module_init);
module_exit(bcm_mspi_bcma_module_exit);
+#endif
+
MODULE_DESCRIPTION("Broadcom MSPI SPI Controller driver");
MODULE_AUTHOR("Rafał Miłecki <[email protected]>");
MODULE_AUTHOR("Broadcom");
--
1.7.9.5
The driver wasn't setting the SPBR (serial clock baud rate) which caused
it to run at the slowest speed possible. The driver now calculates the
SPBR based on the reference clock frequency resulting in much faster
SPI transfers.
Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/spi-bcm-mspi.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
index df27449..5617b5b 100644
--- a/drivers/spi/spi-bcm-mspi.c
+++ b/drivers/spi/spi-bcm-mspi.c
@@ -18,10 +18,15 @@
#include <linux/bcma/bcma.h>
#include <linux/spi/spi.h>
#include <linux/of.h>
+#include <linux/clk.h>
#include "spi-bcm-mspi.h"
#define BCM_MSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */
+#define SPBR_MIN 8U
+#define SPBR_MAX 255U
+#define MSPI_SPCR0_LSB_OFFSET 0x200
+#define MSPI_SPCR0_LSB_SHIFT 0
/* The longest observed required wait was 19 ms */
#define BCM_MSPI_SPE_TIMEOUT_MS 80
@@ -33,7 +38,9 @@ struct bcm_mspi {
void __iomem *base;
struct spi_master *master;
+ struct clk *clk;
size_t read_offset;
+ u32 spbr;
void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
@@ -45,6 +52,15 @@ static inline unsigned int bcm_mspi_calc_timeout(size_t len)
return (len * 9000 / BCM_MSPI_MAX_SPI_BAUD * 110 / 100) + 1;
}
+static void bcm_mspi_hw_init(struct bcm_mspi *mspi)
+{
+ /* Set SPBR (serial clock baud rate). */
+ if (mspi->spbr) {
+ mspi->mspi_write(mspi, MSPI_SPCR0_LSB_OFFSET,
+ mspi->spbr << MSPI_SPCR0_LSB_SHIFT);
+ }
+}
+
static int bcm_mspi_wait(struct bcm_mspi *mspi, unsigned int timeout_ms)
{
unsigned long deadline;
@@ -221,6 +237,7 @@ static struct bcm_mspi *bcm_mspi_init(struct device *dev)
{
struct bcm_mspi *data;
struct spi_master *master;
+ u32 desired_rate;
master = spi_alloc_master(dev, sizeof(*data));
if (!master) {
@@ -235,6 +252,31 @@ static struct bcm_mspi *bcm_mspi_init(struct device *dev)
master->dev.of_node = dev->of_node;
master->transfer_one = bcm_mspi_transfer_one;
+ /*
+ * Enable clock if provided. The frequency can be changed by setting
+ * SPBR (serial clock baud rate) based on the desired 'clock-frequency'.
+ *
+ * Baud rate is calculated as: mspi_clk / (2 * SPBR) where SPBR is a
+ * value between 1-255. If not set then it is left at the h/w default.
+ */
+ data->clk = devm_clk_get(dev, "mspi_clk");
+ if (!IS_ERR(data->clk)) {
+ int ret = clk_prepare_enable(data->clk);
+
+ if (ret < 0) {
+ dev_err(dev, "failed to enable clock: %d\n", ret);
+ return 0;
+ }
+
+ /* Calculate SPBR if clock-frequency provided. */
+ if (of_property_read_u32(dev->of_node, "clock-frequency",
+ &desired_rate) >= 0) {
+ u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
+
+ data->spbr = clamp_val(spbr, SPBR_MIN, SPBR_MAX);
+ }
+ }
+
return data;
}
@@ -286,6 +328,9 @@ static int bcm_mspi_probe(struct platform_device *pdev)
data->mspi_write = bcm_mspi_write;
platform_set_drvdata(pdev, data);
+ /* Initialize SPI controller. */
+ bcm_mspi_hw_init(data);
+
err = devm_spi_register_master(dev, data->master);
if (err)
goto out;
@@ -360,6 +405,9 @@ static int bcm_mspi_bcma_probe(struct bcma_device *core)
data->core = core;
bcma_set_drvdata(core, data);
+ /* Initialize SPI controller. */
+ bcm_mspi_hw_init(data);
+
err = devm_spi_register_master(&core->dev, data->master);
if (err) {
spi_master_put(data->master);
--
1.7.9.5
On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
<[email protected]> wrote:
> The Broadcom MSPI controller is used on various SoCs. It is being
> renamed so that it can be extended and reused on other chips. It is
> renamed to bcm-mspi.
>
What if you resend this one with -M -C applied?
--
With Best Regards,
Andy Shevchenko
On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
<[email protected]> wrote:
> The Broadcom MSPI controller is used on various chips. The driver only
> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
> refactored to make BCMA optional and provides a new config for non BCMA
> systems.
> struct bcm_mspi {
> + #ifdef CONFIG_SPI_BCMA_MSPI
> struct bcma_device *core;
> - struct spi_master *master;
> + #endif
>
> + void __iomem *base;
> + struct spi_master *master;
> size_t read_offset;
> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
> +};
To avoid ugly ifdefs I think better to split driver to core part and
the actual driver part, at the end you will have something like
mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
--
With Best Regards,
Andy Shevchenko
On 03/04/15 06:38, Andy Shevchenko wrote:
> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
> <[email protected]> wrote:
>> The Broadcom MSPI controller is used on various chips. The driver only
>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
>> refactored to make BCMA optional and provides a new config for non BCMA
>> systems.
>
>> struct bcm_mspi {
>> + #ifdef CONFIG_SPI_BCMA_MSPI
>> struct bcma_device *core;
>> - struct spi_master *master;
>> + #endif
>>
>> + void __iomem *base;
>> + struct spi_master *master;
>> size_t read_offset;
>
>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
>> +};
>
> To avoid ugly ifdefs I think better to split driver to core part and
> the actual driver part, at the end you will have something like
> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
>
Actually, I am really curious whether we need the special BCMA I/O
accessors in the first place, cannot we just access the MSPI core on
BCM53xx chips using regular MMIO? That would probably solve the
"problem" entirely. Rafal, did you try this before?
As for splitting the driver into a "library" driver which is mostly
independent from the bus and a bus-specific wrapper, I think BCMA is
really the only special case here, which is why I suggested earlier to
Jonathan that we might just prefer ifdefing things out instead of
creating a separate layer just for BCMA.
--
Florian
Le 02/04/2015 12:23, Jonathan Richardson a écrit :
> The driver wasn't setting the SPBR (serial clock baud rate) which caused
> it to run at the slowest speed possible. The driver now calculates the
> SPBR based on the reference clock frequency resulting in much faster
> SPI transfers.
>
> Signed-off-by: Jonathan Richardson <[email protected]>
> ---
[snip]
> + data->clk = devm_clk_get(dev, "mspi_clk");
> + if (!IS_ERR(data->clk)) {
> + int ret = clk_prepare_enable(data->clk);
> +
> + if (ret < 0) {
> + dev_err(dev, "failed to enable clock: %d\n", ret);
> + return 0;
> + }
> +
> + /* Calculate SPBR if clock-frequency provided. */
> + if (of_property_read_u32(dev->of_node, "clock-frequency",
> + &desired_rate) >= 0) {
> + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
Usually, specifying a "clock-frequency" property is done when there is
no clock provider available, yet we take this code path only if we could
find a "mspi_clk" which sounds a litle weird.
Once there is a proper "mspi_clk" clock, I would make it mandatory for
the clock provider to be able to provide the rate as well?
--
Florian
Le 02/04/2015 12:23, Jonathan Richardson a écrit :
>
> Signed-off-by: Jonathan Richardson <[email protected]>
> ---
> .../devicetree/bindings/spi/brcm,mspi-spi.txt | 38 ++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
> new file mode 100644
> index 0000000..16164e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
> @@ -0,0 +1,38 @@
> +Broadcom MSPI controller
> +
> +Required properties:
> +- compatible: Must be either "brcm,mspi" or "brcm,bcma-mspi". Use
> + "brcm,bcma-mspi" for controllers on a bcma bus and "brcm,mspi" otherwise.
We need a more specific compatible property here since there are at
least 3 known SoCs families within Broadcom (Cygnus, BCM53xx, BCM7xxx)
that use this controller, also older versions of the core did not have a
revision register, yet they had an internal version numbering that we
might want to reflect here. This does not need to be fixed immediately
though, we can add compatible strings as we start adding support for
older cores.
> +
> +- reg: Physical base address and length of the controller's registers.
> +
> +- interrupts: The interrupt id for the controller.
I think this should be two cells, on BCM7xxx chips there is a MSPI_DONE
and a MSPI_ERROR interrupt bit, we typically only use the first one, but
since we are describing the hardware here, we need to be exhaustive.
> +
> +- #address-cells: should be 1.
> +
> +- #size-cells: should be 0.
> +
> +Optional properties:
> +- clocks: The MSPI reference clock. If not provided then it is assumed a clock
> + is enabled by default and no control of clock-frequency (see below) is
> + possible.
> +
> +- clock-names: The name of the reference clock.
> +
> +- clock-frequency: Desired frequency of the clock. This will set the serial
> + clock baud rate (SPBR) based on the reference clock frequency. The frequency
> + of the SPBR is mspi_clk / (2 * SPBR) where SPBR is a value between 1-255
> + determined by the desired 'clock-frequency'. If not provided then the default
> + baud rate of the controller is used.
See my reply to the patch 4, that does not seem to match the
"clock-frequency" vs. clock phandles practices in DT.
> +
> +Example:
> +
> +mspi@18047000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "brcm,mspi";
> + reg = <0x18047000 0x1000>;
> + clocks = <&axi41_clk>;
> + clock-names = "mspi_clk";
> + clock-frequency = <12500000>;
Since "interrupts" is a mandatory property you might want the example to
show it for consistency.
--
Florian
On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote:
> Le 02/04/2015 12:23, Jonathan Richardson a ?crit :
> > + /* Calculate SPBR if clock-frequency provided. */
> > + if (of_property_read_u32(dev->of_node, "clock-frequency",
> > + &desired_rate) >= 0) {
> > + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
> Usually, specifying a "clock-frequency" property is done when there is
> no clock provider available, yet we take this code path only if we could
> find a "mspi_clk" which sounds a litle weird.
> Once there is a proper "mspi_clk" clock, I would make it mandatory for
> the clock provider to be able to provide the rate as well?
As far as I can tell it's already mandatory if the property is present -
it's taking the clock presented to the block and then using an internal
divider to bring that down to the desired rate.
We are missing error handling though.
On 3 April 2015 at 15:35, Andy Shevchenko <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
> <[email protected]> wrote:
>> The Broadcom MSPI controller is used on various SoCs. It is being
>> renamed so that it can be extended and reused on other chips. It is
>> renamed to bcm-mspi.
>>
> What if you resend this one with -M -C applied?
Definitely, right now I can't really review this patch (and I want to).
--
Rafał
On 2 April 2015 at 21:23, Jonathan Richardson <[email protected]> wrote:
> The Broadcom MSPI controller is used on various chips. The driver only
> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
> refactored to make BCMA optional and provides a new config for non BCMA
> systems.
I think this patch provides 3 changes instead of just one described
above. You refactored R/W ops (pointers), made bcma optional and added
DT support. What about patch-per-change?
On 3 April 2015 at 15:38, Andy Shevchenko <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
> <[email protected]> wrote:
>> The Broadcom MSPI controller is used on various chips. The driver only
>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
>> refactored to make BCMA optional and provides a new config for non BCMA
>> systems.
>
>> struct bcm_mspi {
>> + #ifdef CONFIG_SPI_BCMA_MSPI
>> struct bcma_device *core;
>> - struct spi_master *master;
>> + #endif
>>
>> + void __iomem *base;
>> + struct spi_master *master;
>> size_t read_offset;
>
>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
>> +};
>
> To avoid ugly ifdefs I think better to split driver to core part and
> the actual driver part, at the end you will have something like
> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
I also believe we usually (always?) don't align any #if-s (no indent/tabs).
--
Rafał
On 3 April 2015 at 19:52, Florian Fainelli <[email protected]> wrote:
> On 03/04/15 06:38, Andy Shevchenko wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <[email protected]> wrote:
>>> The Broadcom MSPI controller is used on various chips. The driver only
>>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
>>> refactored to make BCMA optional and provides a new config for non BCMA
>>> systems.
>>
>>> struct bcm_mspi {
>>> + #ifdef CONFIG_SPI_BCMA_MSPI
>>> struct bcma_device *core;
>>> - struct spi_master *master;
>>> + #endif
>>>
>>> + void __iomem *base;
>>> + struct spi_master *master;
>>> size_t read_offset;
>>
>>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
>>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
>>> +};
>>
>> To avoid ugly ifdefs I think better to split driver to core part and
>> the actual driver part, at the end you will have something like
>> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
>
> Actually, I am really curious whether we need the special BCMA I/O
> accessors in the first place, cannot we just access the MSPI core on
> BCM53xx chips using regular MMIO? That would probably solve the
> "problem" entirely. Rafal, did you try this before?
It's a matter of choice between:
1) Using one design for all bcma users
2) Using one design for all bcm-mspi users
I believe no matter which one you choose, you'll break another one.
If you take a look at drivers/bcma/host_soc.c, you'll see we've there
core->io_addr. I guess you could use it as the base in bcm-mspi. That
of course will make you a bit less compatible with other bcma drivers
(skipping bcma R/W layer).
> As for splitting the driver into a "library" driver which is mostly
> independent from the bus and a bus-specific wrapper, I think BCMA is
> really the only special case here, which is why I suggested earlier to
> Jonathan that we might just prefer ifdefing things out instead of
> creating a separate layer just for BCMA.
I think you may be right, this #if for bcma shouldn't be that bad and
it shouldn't grow in the future.
Still, I'd like to get this patch split nicely to review independent changes.
--
Rafał
On 15-04-03 06:35 AM, Andy Shevchenko wrote:
> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
> <[email protected]> wrote:
>> The Broadcom MSPI controller is used on various SoCs. It is being
>> renamed so that it can be extended and reused on other chips. It is
>> renamed to bcm-mspi.
>>
> What if you resend this one with -M -C applied?
>
>
I'm not seeing any difference in the patches unfortunately. I'll keep
playing with it and re-send if I can find a way to improve it.
The changes are just renaming variables, structures, functions, file
name to get rid of 53xx and replace with mspi. The config is renamed
from SPI_BCM53XX to SPI_BCMA_MSPI.
On 15-04-03 10:52 AM, Florian Fainelli wrote:
> On 03/04/15 06:38, Andy Shevchenko wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <[email protected]> wrote:
>>> The Broadcom MSPI controller is used on various chips. The driver only
>>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
>>> refactored to make BCMA optional and provides a new config for non BCMA
>>> systems.
>>
>>> struct bcm_mspi {
>>> + #ifdef CONFIG_SPI_BCMA_MSPI
>>> struct bcma_device *core;
>>> - struct spi_master *master;
>>> + #endif
>>>
>>> + void __iomem *base;
>>> + struct spi_master *master;
>>> size_t read_offset;
>>
>>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
>>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
>>> +};
>>
>> To avoid ugly ifdefs I think better to split driver to core part and
>> the actual driver part, at the end you will have something like
>> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
>>
>
> Actually, I am really curious whether we need the special BCMA I/O
> accessors in the first place, cannot we just access the MSPI core on
> BCM53xx chips using regular MMIO? That would probably solve the
> "problem" entirely. Rafal, did you try this before?
>
> As for splitting the driver into a "library" driver which is mostly
> independent from the bus and a bus-specific wrapper, I think BCMA is
> really the only special case here, which is why I suggested earlier to
> Jonathan that we might just prefer ifdefing things out instead of
> creating a separate layer just for BCMA.
>
I cringed adding the ifdefs to the driver but didn't think the small
amount of code that wouldn't be used again warranted 3 files. I could
also handle the two different probe routines by doing some DT parsing in
init, but then BCMA would have to be compiled for the non-BCMA MSPI
driver and I didn't want to do that either.
On 15-04-04 12:17 PM, Florian Fainelli wrote:
> Le 02/04/2015 12:23, Jonathan Richardson a écrit :
>>
>> Signed-off-by: Jonathan Richardson <[email protected]>
>> ---
>> .../devicetree/bindings/spi/brcm,mspi-spi.txt | 38 ++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>> new file mode 100644
>> index 0000000..16164e3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>> @@ -0,0 +1,38 @@
>> +Broadcom MSPI controller
>> +
>> +Required properties:
>> +- compatible: Must be either "brcm,mspi" or "brcm,bcma-mspi". Use
>> + "brcm,bcma-mspi" for controllers on a bcma bus and "brcm,mspi" otherwise.
>
> We need a more specific compatible property here since there are at
> least 3 known SoCs families within Broadcom (Cygnus, BCM53xx, BCM7xxx)
> that use this controller, also older versions of the core did not have a
> revision register, yet they had an internal version numbering that we
> might want to reflect here. This does not need to be fixed immediately
> though, we can add compatible strings as we start adding support for
> older cores.
>
>> +
>> +- reg: Physical base address and length of the controller's registers.
>> +
>> +- interrupts: The interrupt id for the controller.
>
> I think this should be two cells, on BCM7xxx chips there is a MSPI_DONE
> and a MSPI_ERROR interrupt bit, we typically only use the first one, but
> since we are describing the hardware here, we need to be exhaustive.
My mistake. Interrupts are not supported by this driver yet. I intend on
adding this later. I will remove this from the docs.
>
>> +
>> +- #address-cells: should be 1.
>> +
>> +- #size-cells: should be 0.
>> +
>> +Optional properties:
>> +- clocks: The MSPI reference clock. If not provided then it is assumed a clock
>> + is enabled by default and no control of clock-frequency (see below) is
>> + possible.
>> +
>> +- clock-names: The name of the reference clock.
>> +
>> +- clock-frequency: Desired frequency of the clock. This will set the serial
>> + clock baud rate (SPBR) based on the reference clock frequency. The frequency
>> + of the SPBR is mspi_clk / (2 * SPBR) where SPBR is a value between 1-255
>> + determined by the desired 'clock-frequency'. If not provided then the default
>> + baud rate of the controller is used.
>
> See my reply to the patch 4, that does not seem to match the
> "clock-frequency" vs. clock phandles practices in DT.
I could use a different property for it. I agree it's a bit weird since
we're not changing the frequency of the reference clock but the clock
derived from it for the baud rate of the controller. Does this warrant
adding a different property though?
>
>> +
>> +Example:
>> +
>> +mspi@18047000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "brcm,mspi";
>> + reg = <0x18047000 0x1000>;
>> + clocks = <&axi41_clk>;
>> + clock-names = "mspi_clk";
>> + clock-frequency = <12500000>;
>
> Since "interrupts" is a mandatory property you might want the example to
> show it for consistency.
> --
> Florian
>
On 15-04-06 02:46 AM, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote:
>> Le 02/04/2015 12:23, Jonathan Richardson a ?crit :
>
>>> + /* Calculate SPBR if clock-frequency provided. */
>>> + if (of_property_read_u32(dev->of_node, "clock-frequency",
>>> + &desired_rate) >= 0) {
>>> + u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
>
>> Usually, specifying a "clock-frequency" property is done when there is
>> no clock provider available, yet we take this code path only if we could
>> find a "mspi_clk" which sounds a litle weird.
>
>> Once there is a proper "mspi_clk" clock, I would make it mandatory for
>> the clock provider to be able to provide the rate as well?
>
> As far as I can tell it's already mandatory if the property is present -
> it's taking the clock presented to the block and then using an internal
> divider to bring that down to the desired rate.
>
> We are missing error handling though.
>
Thanks for the review. Yes that's correct. I also tried to make it
backwards compatible with the current version of the driver where you
can ignore configuring the frequency. The result being ref clock
frequency / 2 * 255. If you provide the clock then it will
enable/prepare it. If you also provide clock-frequency then it will
configure the SPBR. If you don't provide anything then it works as
before - it assumes the clock is already enabled and uses the h/w
default SPBR.
On 15-04-06 03:18 AM, Rafał Miłecki wrote:
> On 3 April 2015 at 15:35, Andy Shevchenko <[email protected]> wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <[email protected]> wrote:
>>> The Broadcom MSPI controller is used on various SoCs. It is being
>>> renamed so that it can be extended and reused on other chips. It is
>>> renamed to bcm-mspi.
>>>
>> What if you resend this one with -M -C applied?
>
> Definitely, right now I can't really review this patch (and I want to).
>
Thanks Rafal for the review. Please see previous reply to Andy and let
me know if I'm missing something.
On 15-04-06 03:36 AM, Rafał Miłecki wrote:
> On 3 April 2015 at 19:52, Florian Fainelli <[email protected]> wrote:
>> On 03/04/15 06:38, Andy Shevchenko wrote:
>>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>>> <[email protected]> wrote:
>>>> The Broadcom MSPI controller is used on various chips. The driver only
>>>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is
>>>> refactored to make BCMA optional and provides a new config for non BCMA
>>>> systems.
>>>
>>>> struct bcm_mspi {
>>>> + #ifdef CONFIG_SPI_BCMA_MSPI
>>>> struct bcma_device *core;
>>>> - struct spi_master *master;
>>>> + #endif
>>>>
>>>> + void __iomem *base;
>>>> + struct spi_master *master;
>>>> size_t read_offset;
>>>
>>>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value);
>>>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset);
>>>> +};
>>>
>>> To avoid ugly ifdefs I think better to split driver to core part and
>>> the actual driver part, at the end you will have something like
>>> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c
>>
>> Actually, I am really curious whether we need the special BCMA I/O
>> accessors in the first place, cannot we just access the MSPI core on
>> BCM53xx chips using regular MMIO? That would probably solve the
>> "problem" entirely. Rafal, did you try this before?
>
> It's a matter of choice between:
> 1) Using one design for all bcma users
> 2) Using one design for all bcm-mspi users
> I believe no matter which one you choose, you'll break another one.
>
> If you take a look at drivers/bcma/host_soc.c, you'll see we've there
> core->io_addr. I guess you could use it as the base in bcm-mspi. That
> of course will make you a bit less compatible with other bcma drivers
> (skipping bcma R/W layer).
That would require compiling in BCMA for a driver/chip that doesn't use
BCMA but then I could do DT parsing in init only anyway. I don't think
that's really an option so I'm going to leave as is unless there is
further opinion on it.
>
>
>> As for splitting the driver into a "library" driver which is mostly
>> independent from the bus and a bus-specific wrapper, I think BCMA is
>> really the only special case here, which is why I suggested earlier to
>> Jonathan that we might just prefer ifdefing things out instead of
>> creating a separate layer just for BCMA.
>
> I think you may be right, this #if for bcma shouldn't be that bad and
> it shouldn't grow in the future.
> Still, I'd like to get this patch split nicely to review independent changes.
>
Making BCMA optional was made possible by using DT. I'm not sure I could
split it into two commits. I would have to add a hard coded SPI device
for non-BMCA as well. I thought the driver was a bit odd in that this
was hard coded. Normally this should be in a separate driver. How would
you use it if you wanted to use m25p80 for example?
On Mon, Apr 6, 2015 at 9:30 PM, Jonathan Richardson
<[email protected]> wrote:
> On 15-04-03 06:35 AM, Andy Shevchenko wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <[email protected]> wrote:
>>> The Broadcom MSPI controller is used on various SoCs. It is being
>>> renamed so that it can be extended and reused on other chips. It is
>>> renamed to bcm-mspi.
>>>
>> What if you resend this one with -M -C applied?
>>
>>
>
> I'm not seeing any difference in the patches unfortunately. I'll keep
> playing with it and re-send if I can find a way to improve it.
>
> The changes are just renaming variables, structures, functions, file
> name to get rid of 53xx and replace with mspi. The config is renamed
> from SPI_BCM53XX to SPI_BCMA_MSPI.
>
Better to split: a) moving, b) renaming something inside.
--
With Best Regards,
Andy Shevchenko