2015-04-08 18:02:45

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 0/5] Add MSPI support for Cygnus

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.

Changes from v1:
- Split previous patch 2 into two patches to make it easier to review. One
commit for the file rename, and one for the actual changes to rename
variables, functions, etc.
- Checked return value of clk_get_rate().
- ifdef indentation fix.

Jonathan Richardson (5):
ARM: dts: Add binding for Broadcom MSPI driver.
spi: bcm53xx: Refactor driver to make nonspecific to 53xx SoCs
spi: bcm-mspi: 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 | 36 ++
drivers/spi/Kconfig | 12 +-
drivers/spi/Makefile | 3 +-
drivers/spi/spi-bcm-mspi.c | 457 ++++++++++++++++++++
drivers/spi/spi-bcm-mspi.h | 84 ++++
drivers/spi/spi-bcm53xx.c | 299 -------------
drivers/spi/spi-bcm53xx.h | 72 ---
7 files changed, 587 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


2015-04-08 18:02:47

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 1/5] ARM: dts: Add binding for Broadcom MSPI driver.


Signed-off-by: Jonathan Richardson <[email protected]>
---
.../devicetree/bindings/spi/brcm,mspi-spi.txt | 36 ++++++++++++++++++++
1 file changed, 36 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..8c10a39
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
@@ -0,0 +1,36 @@
+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.
+
+- #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

2015-04-08 18:03:50

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 2/5] spi: bcm53xx: Refactor driver to make nonspecific to 53xx SoCs

The Broadcom MSPI controller is used on various SoCs. It is being
renamed so that it can be extended and reused on other chips. The
driver no longer depends on 53xx architecture.

Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/Kconfig | 7 +-
drivers/spi/Makefile | 2 +-
drivers/spi/spi-bcm-mspi.c | 301 ++++++++++++++++++++++++++++++++++++++++++++
drivers/spi/spi-bcm-mspi.h | 72 +++++++++++
drivers/spi/spi-bcm53xx.c | 299 -------------------------------------------
drivers/spi/spi-bcm53xx.h | 72 -----------
6 files changed, 377 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..db3d293
--- /dev/null
+++ b/drivers/spi/spi-bcm-mspi.c
@@ -0,0 +1,301 @@
+#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-bcm-mspi.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-bcm-mspi.h b/drivers/spi/spi-bcm-mspi.h
new file mode 100644
index 0000000..b44fdca
--- /dev/null
+++ b/drivers/spi/spi-bcm-mspi.h
@@ -0,0 +1,72 @@
+#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 */
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

2015-04-08 18:02:50

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 3/5] spi: bcm-mspi: Refactor to make driver nonspecific to 53xx SoCs

Rename variables, structures, etc so that the naming matches the new
driver name (bcm mspi) instead of the previous naming that was chip
specific (53xx).

Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/spi-bcm-mspi.c | 200 ++++++++++++++++++++++----------------------
drivers/spi/spi-bcm-mspi.h | 140 +++++++++++++++++--------------
2 files changed, 178 insertions(+), 162 deletions(-)

diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
index db3d293..502227d 100644
--- a/drivers/spi/spi-bcm-mspi.c
+++ b/drivers/spi/spi-bcm-mspi.c
@@ -1,5 +1,15 @@
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
+/*
+ * 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>
@@ -9,58 +19,58 @@

#include "spi-bcm-mspi.h"

-#define BCM53XXSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */
+#define BCM_MSPI_MAX_SPI_BAUD 13500000 /* 216 MHz? */

/* The longest observed required wait was 19 ms */
-#define BCM53XXSPI_SPE_TIMEOUT_MS 80
+#define BCM_MSPI_SPE_TIMEOUT_MS 80

-struct bcm53xxspi {
+struct bcm_mspi {
struct bcma_device *core;
struct spi_master *master;

size_t read_offset;
};

-static inline u32 bcm53xxspi_read(struct bcm53xxspi *b53spi, u16 offset)
+static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
{
- return bcma_read32(b53spi->core, offset);
+ return bcma_read32(mspi->core, offset);
}

-static inline void bcm53xxspi_write(struct bcm53xxspi *b53spi, u16 offset,
+static inline void bcm_mspi_write(struct bcm_mspi *mspi, u16 offset,
u32 value)
{
- bcma_write32(b53spi->core, offset, value);
+ bcma_write32(mspi->core, offset, value);
}

-static inline unsigned int bcm53xxspi_calc_timeout(size_t len)
+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 / BCM53XXSPI_MAX_SPI_BAUD * 110 / 100) + 1;
+ return (len * 9000 / BCM_MSPI_MAX_SPI_BAUD * 110 / 100) + 1;
}

-static int bcm53xxspi_wait(struct bcm53xxspi *b53spi, unsigned int timeout_ms)
+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 + BCM53XXSPI_SPE_TIMEOUT_MS * HZ / 1000;
+ deadline = jiffies + BCM_MSPI_SPE_TIMEOUT_MS * HZ / 1000;
do {
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- if (!(tmp & B53SPI_MSPI_SPCR2_SPE))
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ if (!(tmp & MSPI_SPCR2_SPE))
break;
udelay(5);
} while (!time_after_eq(jiffies, deadline));

- if (tmp & B53SPI_MSPI_SPCR2_SPE)
+ if (tmp & 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);
+ tmp = bcm_mspi_read(mspi, MSPI_STATUS);
+ if (tmp & MSPI_STATUS_SPIF) {
+ bcm_mspi_write(mspi, MSPI_STATUS, 0);
return 0;
}

@@ -69,14 +79,14 @@ static int bcm53xxspi_wait(struct bcm53xxspi *b53spi, unsigned int timeout_ms)
} while (!time_after_eq(jiffies, deadline));

spi_timeout:
- bcm53xxspi_write(b53spi, B53SPI_MSPI_MSPI_STATUS, 0);
+ bcm_mspi_write(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,
+static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
size_t len, bool cont)
{
u32 tmp;
@@ -84,96 +94,95 @@ static void bcm53xxspi_buf_write(struct bcm53xxspi *b53spi, u8 *w_buf,

for (i = 0; i < len; i++) {
/* Transmit Register File MSB */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_TXRAM + 4 * (i * 2),
+ bcm_mspi_write(mspi, 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;
+ tmp = MSPI_CDRAM_CONT | MSPI_CDRAM_PCS_DISABLE_ALL |
+ MSPI_CDRAM_PCS_DSCK;
if (!cont && i == len - 1)
- tmp &= ~B53SPI_CDRAM_CONT;
+ tmp &= ~MSPI_CDRAM_CONT;
tmp &= ~0x1;
/* Command Register File */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_CDRAM + 4 * i, tmp);
+ bcm_mspi_write(mspi, MSPI_CDRAM + 4 * i, tmp);
}

/* Set queue pointers */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
- bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP, len - 1);
+ bcm_mspi_write(mspi, MSPI_NEWQP, 0);
+ bcm_mspi_write(mspi, MSPI_ENDQP, len - 1);

if (cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);

/* Start SPI transfer */
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- tmp |= B53SPI_MSPI_SPCR2_SPE;
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp |= MSPI_SPCR2_SPE;
if (cont)
- tmp |= B53SPI_MSPI_SPCR2_CONT_AFTER_CMD;
- bcm53xxspi_write(b53spi, B53SPI_MSPI_SPCR2, tmp);
+ tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
+ bcm_mspi_write(mspi, MSPI_SPCR2, tmp);

/* Wait for SPI to finish */
- bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
+ bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));

if (!cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);

- b53spi->read_offset = len;
+ mspi->read_offset = len;
}

-static void bcm53xxspi_buf_read(struct bcm53xxspi *b53spi, u8 *r_buf,
+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 < 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;
+ for (i = 0; i < mspi->read_offset + len; i++) {
+ tmp = MSPI_CDRAM_CONT | MSPI_CDRAM_PCS_DISABLE_ALL |
+ MSPI_CDRAM_PCS_DSCK;
+ if (!cont && i == mspi->read_offset + len - 1)
+ tmp &= ~MSPI_CDRAM_CONT;
tmp &= ~0x1;
/* Command Register File */
- bcm53xxspi_write(b53spi, B53SPI_MSPI_CDRAM + 4 * i, tmp);
+ bcm_mspi_write(mspi, 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);
+ bcm_mspi_write(mspi, MSPI_NEWQP, 0);
+ bcm_mspi_write(mspi, MSPI_ENDQP, mspi->read_offset + len - 1);

if (cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 1);

/* Start SPI transfer */
- tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
- tmp |= B53SPI_MSPI_SPCR2_SPE;
+ tmp = bcm_mspi_read(mspi, MSPI_SPCR2);
+ tmp |= MSPI_SPCR2_SPE;
if (cont)
- tmp |= B53SPI_MSPI_SPCR2_CONT_AFTER_CMD;
- bcm53xxspi_write(b53spi, B53SPI_MSPI_SPCR2, tmp);
+ tmp |= MSPI_SPCR2_CONT_AFTER_CMD;
+ bcm_mspi_write(mspi, MSPI_SPCR2, tmp);

/* Wait for SPI to finish */
- bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
+ bcm_mspi_wait(mspi, bcm_mspi_calc_timeout(len));

if (!cont)
- bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 0);
+ bcm_mspi_write(mspi, MSPI_WRITE_LOCK, 0);

for (i = 0; i < len; ++i) {
- int offset = b53spi->read_offset + i;
+ int offset = mspi->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));
+ r_buf[i] = (u8)bcm_mspi_read(mspi,
+ MSPI_RXRAM + 4 * (1 + offset * 2));
}

- b53spi->read_offset = 0;
+ mspi->read_offset = 0;
}

-static int bcm53xxspi_transfer_one(struct spi_master *master,
+static int bcm_mspi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *t)
{
- struct bcm53xxspi *b53spi = spi_master_get_devdata(master);
+ struct bcm_mspi *mspi = spi_master_get_devdata(master);
u8 *buf;
size_t left;

@@ -184,7 +193,7 @@ static int bcm53xxspi_transfer_one(struct spi_master *master,
size_t to_write = min_t(size_t, 16, left);
bool cont = left - to_write > 0;

- bcm53xxspi_buf_write(b53spi, buf, to_write, cont);
+ bcm_mspi_buf_write(mspi, buf, to_write, cont);
left -= to_write;
buf += to_write;
}
@@ -194,11 +203,11 @@ static int bcm53xxspi_transfer_one(struct spi_master *master,
buf = (u8 *)t->rx_buf;
left = t->len;
while (left) {
- size_t to_read = min_t(size_t, 16 - b53spi->read_offset,
+ size_t to_read = min_t(size_t, 16 - mspi->read_offset,
left);
bool cont = left - to_read > 0;

- bcm53xxspi_buf_read(b53spi, buf, to_read, cont);
+ bcm_mspi_buf_read(mspi, buf, to_read, cont);
left -= to_read;
buf += to_read;
}
@@ -207,45 +216,43 @@ static int bcm53xxspi_transfer_one(struct spi_master *master,
return 0;
}

-/**************************************************
- * BCMA
- **************************************************/
-
-static struct spi_board_info bcm53xx_info = {
+static struct spi_board_info bcm_mspi_info = {
.modalias = "bcm53xxspiflash",
};

-static const struct bcma_device_id bcm53xxspi_bcma_tbl[] = {
+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, bcm53xxspi_bcma_tbl);
+MODULE_DEVICE_TABLE(bcma, bcm_mspi_bcma_tbl);

-static int bcm53xxspi_bcma_probe(struct bcma_device *core)
+static int bcm_mspi_bcma_probe(struct bcma_device *core)
{
- struct bcm53xxspi *b53spi;
+ 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(*b53spi));
+ master = spi_alloc_master(&core->dev, sizeof(*data));
if (!master)
return -ENOMEM;

- b53spi = spi_master_get_devdata(master);
- b53spi->master = master;
- b53spi->core = core;
+ data = spi_master_get_devdata(master);
+ data->master = master;
+ data->core = core;

- master->transfer_one = bcm53xxspi_transfer_one;
+ master->transfer_one = bcm_mspi_transfer_one;

- bcma_set_drvdata(core, b53spi);
+ bcma_set_drvdata(core, data);

- err = devm_spi_register_master(&core->dev, master);
+ err = devm_spi_register_master(&core->dev, data->master);
if (err) {
spi_master_put(master);
bcma_set_drvdata(core, NULL);
@@ -253,49 +260,46 @@ static int bcm53xxspi_bcma_probe(struct bcma_device *core)
}

/* Broadcom SoCs (at least with the CC rev 42) use SPI for flash only */
- spi_new_device(master, &bcm53xx_info);
+ spi_new_device(master, &bcm_mspi_info);

out:
return err;
}

-static void bcm53xxspi_bcma_remove(struct bcma_device *core)
+static void bcm_mspi_bcma_remove(struct bcma_device *core)
{
- struct bcm53xxspi *b53spi = bcma_get_drvdata(core);
+ struct bcm_mspi *mspi = bcma_get_drvdata(core);

- spi_unregister_master(b53spi->master);
+ spi_unregister_master(mspi->master);
}

-static struct bcma_driver bcm53xxspi_bcma_driver = {
+static struct bcma_driver bcm_mspi_bcma_driver = {
.name = KBUILD_MODNAME,
- .id_table = bcm53xxspi_bcma_tbl,
- .probe = bcm53xxspi_bcma_probe,
- .remove = bcm53xxspi_bcma_remove,
+ .id_table = bcm_mspi_bcma_tbl,
+ .probe = bcm_mspi_bcma_probe,
+ .remove = bcm_mspi_bcma_remove,
};

-/**************************************************
- * Init & exit
- **************************************************/
-
-static int __init bcm53xxspi_module_init(void)
+static int __init bcm_mspi_module_init(void)
{
int err = 0;

- err = bcma_driver_register(&bcm53xxspi_bcma_driver);
+ 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 bcm53xxspi_module_exit(void)
+static void __exit bcm_mspi_module_exit(void)
{
- bcma_driver_unregister(&bcm53xxspi_bcma_driver);
+ bcma_driver_unregister(&bcm_mspi_bcma_driver);
}

-module_init(bcm53xxspi_module_init);
-module_exit(bcm53xxspi_module_exit);
+module_init(bcm_mspi_module_init);
+module_exit(bcm_mspi_module_exit);

-MODULE_DESCRIPTION("Broadcom BCM53xx SPI Controller driver");
+MODULE_DESCRIPTION("Broadcom MSPI SPI Controller driver");
MODULE_AUTHOR("Rafał Miłecki <[email protected]>");
-MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Broadcom");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/spi/spi-bcm-mspi.h b/drivers/spi/spi-bcm-mspi.h
index b44fdca..18b24ff 100644
--- a/drivers/spi/spi-bcm-mspi.h
+++ b/drivers/spi/spi-bcm-mspi.h
@@ -1,72 +1,84 @@
-#ifndef SPI_BCM53XX_H
-#define SPI_BCM53XX_H
+/*
+ * 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 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
+#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 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
+#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 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
+#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_STATUS 0x220
+#define 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 MSPI_CDRAM_PCS_PCS0 0x00000001
+#define MSPI_CDRAM_PCS_PCS1 0x00000002
+#define MSPI_CDRAM_PCS_PCS2 0x00000004
+#define MSPI_CDRAM_PCS_PCS3 0x00000008
+#define MSPI_CDRAM_PCS_DISABLE_ALL 0x0000000f
+#define MSPI_CDRAM_PCS_DSCK 0x00000010
+#define MSPI_CDRAM_BITSE 0x00000040
+#define MSPI_CDRAM_CONT 0x00000080
+#define MSPI_WRITE_LOCK 0x380
+#define 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
+#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 /* SPI_BCM53XX_H */
+#endif
--
1.7.9.5

2015-04-08 18:03:47

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 4/5] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips

The Broadcom MSPI controller is used on various chips. The driver only
supported BCM53xx chips with BCMA (an AMBA bus variant). It now supports
both BCMA MSPI and non-BCMA MSPI. To do this the following changes were
made:

- A new config for non-BCMA chips has been added.
- Common code between the BCMA and non BCMA version are shared.
- Function pointers to set read/write functions to abstract bcma
and non-bcma versions are provided.
- DT is now mandatory. Hard coded SPI devices are removed and must be
set in DT.
- Remove function was unnecessary and removed.

Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/spi/Kconfig | 5 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-bcm-mspi.c | 228 ++++++++++++++++++++++++++++++++------------
3 files changed, 171 insertions(+), 63 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 502227d..32bb1f0 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_STATUS);
+ tmp = mspi->mspi_read(mspi, MSPI_STATUS);
if (tmp & MSPI_STATUS_SPIF) {
- bcm_mspi_write(mspi, MSPI_STATUS, 0);
+ mspi->mspi_write(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_STATUS, 0);
+ mspi->mspi_write(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]);
}

@@ -105,28 +102,28 @@ static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
tmp &= ~MSPI_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;
}
@@ -144,34 +141,35 @@ static void bcm_mspi_buf_read(struct bcm_mspi *mspi, u8 *r_buf,
tmp &= ~MSPI_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->read_offset + len - 1);
+ 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,10 +214,104 @@ static int bcm_mspi_transfer_one(struct spi_master *master,
return 0;
}

-static struct spi_board_info bcm_mspi_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,
BCMA_ANY_CLASS),
@@ -227,62 +319,70 @@ 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);
+}
+
+static inline void bcm_bcma_mspi_write(struct bcm_mspi *mspi, u16 offset,
+ u32 value)
+{
+ 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, &bcm_mspi_info);
-
-out:
- return err;
-}
-
-static void bcm_mspi_bcma_remove(struct bcma_device *core)
-{
- struct bcm_mspi *mspi = bcma_get_drvdata(core);
-
- spi_unregister_master(mspi->master);
+ 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,
- .remove = bcm_mspi_bcma_remove,
};

-static int __init bcm_mspi_module_init(void)
+static int __init bcm_mspi_bcma_module_init(void)
{
- int err = 0;
+ int err;

err = bcma_driver_register(&bcm_mspi_bcma_driver);
if (err)
@@ -291,13 +391,15 @@ static int __init bcm_mspi_module_init(void)
return err;
}

-static void __exit bcm_mspi_module_exit(void)
+static void __exit bcm_mspi_bcma_module_exit(void)
{
bcma_driver_unregister(&bcm_mspi_bcma_driver);
}

-module_init(bcm_mspi_module_init);
-module_exit(bcm_mspi_module_exit);
+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]>");
--
1.7.9.5

2015-04-08 18:03:26

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 5/5] spi: bcm-mspi: Add support to set serial baud clock rate

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 | 50 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)

diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
index 32bb1f0..9320de9 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;
@@ -222,6 +238,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) {
@@ -236,6 +253,33 @@ 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);
+
+ if (spbr > 0)
+ data->spbr = clamp_val(spbr, SPBR_MIN,
+ SPBR_MAX);
+ }
+ }
+
return data;
}

@@ -287,6 +331,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;
@@ -362,6 +409,9 @@ static int bcm_mspi_bcma_probe(struct bcma_device *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

2015-04-08 19:27:54

by Jonas Gorski

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips

Hi,

On Wed, Apr 8, 2015 at 8:04 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). It now supports
> both BCMA MSPI and non-BCMA MSPI. To do this the following changes were
> made:
>
> - A new config for non-BCMA chips has been added.
> - Common code between the BCMA and non BCMA version are shared.
> - Function pointers to set read/write functions to abstract bcma
> and non-bcma versions are provided.
> - DT is now mandatory. Hard coded SPI devices are removed and must be
> set in DT.
> - Remove function was unnecessary and removed.
>
> Signed-off-by: Jonathan Richardson <[email protected]>
> ---
> drivers/spi/Kconfig | 5 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-bcm-mspi.c | 228 ++++++++++++++++++++++++++++++++------------
> 3 files changed, 171 insertions(+), 63 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"

You say "DT is now mandatory", but I don't see you depending on OF.
Does it compile with OF disabled?

> + 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

What happens if SPI_BCMA_MSPI and SPI_BCM_MSPI are both set? What
happens if they disagree, i.e. one is y, the other m?

> 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 502227d..32bb1f0 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;

You could make this part a bit more readable by not reordering existing members.

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

Why not keep these and let them call mspi->mspi_read() resp.
mspi->mspi_write()? It would reduce the amount of changes quite a bit.

>
> 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_STATUS);
> + tmp = mspi->mspi_read(mspi, MSPI_STATUS);
> if (tmp & MSPI_STATUS_SPIF) {
> - bcm_mspi_write(mspi, MSPI_STATUS, 0);
> + mspi->mspi_write(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_STATUS, 0);
> + mspi->mspi_write(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]);
> }
>
> @@ -105,28 +102,28 @@ static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
> tmp &= ~MSPI_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;
> }
> @@ -144,34 +141,35 @@ static void bcm_mspi_buf_read(struct bcm_mspi *mspi, u8 *r_buf,
> tmp &= ~MSPI_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->read_offset + len - 1);
> + 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,10 +214,104 @@ static int bcm_mspi_transfer_one(struct spi_master *master,
> return 0;
> }
>
> -static struct spi_board_info bcm_mspi_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;

return NULL;

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

Won't this break when being build as a module? I think you need

#if IS_ENABLED(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");

devm_ioremap_resource will already complain for you.

> + 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

likewise.

> +
> 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),
> @@ -227,62 +319,70 @@ 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);
> +}
> +
> +static inline void bcm_bcma_mspi_write(struct bcm_mspi *mspi, u16 offset,
> + u32 value)
> +{
> + 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, &bcm_mspi_info);

Why are you removing the registration of the flash device? Won't this
break bcm53xx's flash registration?

> -
> -out:
> - return err;
> -}
> -
> -static void bcm_mspi_bcma_remove(struct bcma_device *core)
> -{
> - struct bcm_mspi *mspi = bcma_get_drvdata(core);
> -
> - spi_unregister_master(mspi->master);
> + 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,
> - .remove = bcm_mspi_bcma_remove,
> };
>
> -static int __init bcm_mspi_module_init(void)
> +static int __init bcm_mspi_bcma_module_init(void)
> {
> - int err = 0;
> + int err;

Unrelated change.

>
> err = bcma_driver_register(&bcm_mspi_bcma_driver);
> if (err)
> @@ -291,13 +391,15 @@ static int __init bcm_mspi_module_init(void)
> return err;
> }
>
> -static void __exit bcm_mspi_module_exit(void)
> +static void __exit bcm_mspi_bcma_module_exit(void)
> {
> bcma_driver_unregister(&bcm_mspi_bcma_driver);
> }
>
> -module_init(bcm_mspi_module_init);
> -module_exit(bcm_mspi_module_exit);
> +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]>");
> --


Regards
Jonas

2015-04-08 19:32:41

by Jonas Gorski

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] spi: bcm-mspi: Add support to set serial baud clock rate

Hi,

On Wed, Apr 8, 2015 at 8:04 PM, Jonathan Richardson
<[email protected]> wrote:
> 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 | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/spi/spi-bcm-mspi.c b/drivers/spi/spi-bcm-mspi.c
> index 32bb1f0..9320de9 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;
> @@ -222,6 +238,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) {
> @@ -236,6 +253,33 @@ 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);

You need to disable_unprepare the clock on removal, I don't see you doing that.

> +
> + 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);
> +
> + if (spbr > 0)
> + data->spbr = clamp_val(spbr, SPBR_MIN,
> + SPBR_MAX);
> + }
> + }
> +
> return data;
> }
>
> @@ -287,6 +331,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;
> @@ -362,6 +409,9 @@ static int bcm_mspi_bcma_probe(struct bcma_device *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);
> --

Regards
Jonas

2015-04-08 19:49:37

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] spi: bcm53xx: Refactor driver to make nonspecific to 53xx SoCs

On Wed, Apr 08, 2015 at 11:04:32AM -0700, Jonathan Richardson wrote:

> ---
> drivers/spi/Kconfig | 7 +-
> drivers/spi/Makefile | 2 +-
> drivers/spi/spi-bcm-mspi.c | 301 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/spi/spi-bcm-mspi.h | 72 +++++++++++
> drivers/spi/spi-bcm53xx.c | 299 -------------------------------------------
> drivers/spi/spi-bcm53xx.h | 72 -----------
> 6 files changed, 377 insertions(+), 376 deletions(-)

Please use git format-patch -M and split the moves from the renames,
this is basically unreviewable as is since it's just a remove/add.


Attachments:
(No filename) (610.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-08 19:53:48

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] spi: bcm-mspi: Add support to set serial baud clock rate

On Wed, Apr 08, 2015 at 11:04:35AM -0700, Jonathan Richardson wrote:

> + /*
> + * 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)) {

This is adding things to the DT binding, any new binding or binding
changes need to be documented. It should also be paying attention to
errors rather than silently ignoring them, at least -EPROBE_DEFER should
be handled properly.


Attachments:
(No filename) (700.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-08 20:03:25

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips

On Wed, Apr 08, 2015 at 11:04:34AM -0700, Jonathan Richardson wrote:

> - A new config for non-BCMA chips has been added.
> - Common code between the BCMA and non BCMA version are shared.
> - Function pointers to set read/write functions to abstract bcma
> and non-bcma versions are provided.
> - DT is now mandatory. Hard coded SPI devices are removed and must be
> set in DT.
> - Remove function was unnecessary and removed.

This looks like it should be a patch series in itself - for example, the
move to using function pointers as a read/write operation looks like
something that could easily be pulled out, as could the removal of
unused functions. Having things split out makes life a lot easier for
review since it makes it much easier to check if the change is doing the
things it's supposed to be doing.


Attachments:
(No filename) (833.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-08 22:26:59

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips

Jonas, thanks for the review. Comments below.

On 15-04-08 12:27 PM, Jonas Gorski wrote:
> Hi,
>
> On Wed, Apr 8, 2015 at 8:04 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). It now supports
>> both BCMA MSPI and non-BCMA MSPI. To do this the following changes were
>> made:
>>
>> - A new config for non-BCMA chips has been added.
>> - Common code between the BCMA and non BCMA version are shared.
>> - Function pointers to set read/write functions to abstract bcma
>> and non-bcma versions are provided.
>> - DT is now mandatory. Hard coded SPI devices are removed and must be
>> set in DT.
>> - Remove function was unnecessary and removed.
>>
>> Signed-off-by: Jonathan Richardson <[email protected]>
>> ---
>> drivers/spi/Kconfig | 5 +
>> drivers/spi/Makefile | 1 +
>> drivers/spi/spi-bcm-mspi.c | 228 ++++++++++++++++++++++++++++++++------------
>> 3 files changed, 171 insertions(+), 63 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"
>
> You say "DT is now mandatory", but I don't see you depending on OF.
> Does it compile with OF disabled?
>
>> + 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
>
> What happens if SPI_BCMA_MSPI and SPI_BCM_MSPI are both set? What
> happens if they disagree, i.e. one is y, the other m?

If they're both set then everything compiles fine. But if y/m then you
don't get a module. Only if you set both to m will you get a module. I
could possibly force both to m in the makefile or split into 3 files.

Any preference or another idea?

>
>> 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 502227d..32bb1f0 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;
>
> You could make this part a bit more readable by not reordering existing members.
>
>> 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);
>> +};
>
> Why not keep these and let them call mspi->mspi_read() resp.
> mspi->mspi_write()? It would reduce the amount of changes quite a bit.
>
>>
>> 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_STATUS);
>> + tmp = mspi->mspi_read(mspi, MSPI_STATUS);
>> if (tmp & MSPI_STATUS_SPIF) {
>> - bcm_mspi_write(mspi, MSPI_STATUS, 0);
>> + mspi->mspi_write(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_STATUS, 0);
>> + mspi->mspi_write(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]);
>> }
>>
>> @@ -105,28 +102,28 @@ static void bcm_mspi_buf_write(struct bcm_mspi *mspi, u8 *w_buf,
>> tmp &= ~MSPI_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;
>> }
>> @@ -144,34 +141,35 @@ static void bcm_mspi_buf_read(struct bcm_mspi *mspi, u8 *r_buf,
>> tmp &= ~MSPI_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->read_offset + len - 1);
>> + 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,10 +214,104 @@ static int bcm_mspi_transfer_one(struct spi_master *master,
>> return 0;
>> }
>>
>> -static struct spi_board_info bcm_mspi_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;
>
> return NULL;
>
>> + }
>> +
>> + 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
>
> Won't this break when being build as a module? I think you need
>
> #if IS_ENABLED(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");
>
> devm_ioremap_resource will already complain for you.
>
>> + 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
>
> likewise.
>
>> +
>> 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),
>> @@ -227,62 +319,70 @@ 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);
>> +}
>> +
>> +static inline void bcm_bcma_mspi_write(struct bcm_mspi *mspi, u16 offset,
>> + u32 value)
>> +{
>> + 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, &bcm_mspi_info);
>
> Why are you removing the registration of the flash device? Won't this
> break bcm53xx's flash registration?

I'm actually not sure which spi device is being used here. What device
is "bcm53xxspiflash".. maybe Rafal can help? Then intention was to
select it from DT. Maybe something more needs to happen here though to
enable this though.

>From bcm_mspi_init():
/* SPI master will always use the SPI device(s) from DT. */
master->dev.of_node = dev->of_node;

I think creating a spi device here would conflict with DT now. Here's
how I select m25p80 with non-bcma for example:

mspi@18047000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "brcm,mspi";
reg = <0x18047000 0x1000>;

flash: m25p80@0 {
compatible = "m25p80";
reg = <0>;
spi-max-frequency = <12500000>;
m25p,fast-read;
mode = <3>;
};
};


>
>> -
>> -out:
>> - return err;
>> -}
>> -
>> -static void bcm_mspi_bcma_remove(struct bcma_device *core)
>> -{
>> - struct bcm_mspi *mspi = bcma_get_drvdata(core);
>> -
>> - spi_unregister_master(mspi->master);
>> + 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,
>> - .remove = bcm_mspi_bcma_remove,
>> };
>>
>> -static int __init bcm_mspi_module_init(void)
>> +static int __init bcm_mspi_bcma_module_init(void)
>> {
>> - int err = 0;
>> + int err;
>
> Unrelated change.
>
>>
>> err = bcma_driver_register(&bcm_mspi_bcma_driver);
>> if (err)
>> @@ -291,13 +391,15 @@ static int __init bcm_mspi_module_init(void)
>> return err;
>> }
>>
>> -static void __exit bcm_mspi_module_exit(void)
>> +static void __exit bcm_mspi_bcma_module_exit(void)
>> {
>> bcma_driver_unregister(&bcm_mspi_bcma_driver);
>> }
>>
>> -module_init(bcm_mspi_module_init);
>> -module_exit(bcm_mspi_module_exit);
>> +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]>");
>> --
>
>
> Regards
> Jonas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2015-04-09 22:26:36

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips

On 15-04-08 01:03 PM, Mark Brown wrote:
> On Wed, Apr 08, 2015 at 11:04:34AM -0700, Jonathan Richardson wrote:
>
>> - A new config for non-BCMA chips has been added.
>> - Common code between the BCMA and non BCMA version are shared.
>> - Function pointers to set read/write functions to abstract bcma
>> and non-bcma versions are provided.
>> - DT is now mandatory. Hard coded SPI devices are removed and must be
>> set in DT.
>> - Remove function was unnecessary and removed.
>
> This looks like it should be a patch series in itself - for example, the
> move to using function pointers as a read/write operation looks like
> something that could easily be pulled out, as could the removal of
> unused functions. Having things split out makes life a lot easier for
> review since it makes it much easier to check if the change is doing the
> things it's supposed to be doing.
>

Mark, thanks for the comments. I think we need a new driver instead of
trying to re-use the existing driver that requires quite a few changes
and that I also can't test. Having both drivers in the same file doesn't
work well either. Common code isn't desirable because we're likely going
to make changes to it anyway. It doesn't use interrupts currently and I
don't like the polling for SPI transfer completion. Once I make the
changes I'll submit a standalone non-BCMA MSPI driver for review.

Jon