Add MSPI support for Cygnus

---
 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(-)

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 <jonathar-dY08KVG/lbpWk0Htik3J/[email protected]>

---
 drivers/spi/Kconfig        |    5 +
 drivers/spi/Makefile       |    1 +
 drivers/spi/spi-bcm-mspi.c |  228 ++++++++++++++++++++++++++++++++------------
 3 files changed, 171 insertions(+), 63 deletions(-)

drivers/spi/Kconfig +0 -0

index 766e08d..23f2357 100644

drivers/spi/Kconfig +3 -0

+       help
+         Enable support for the Broadcom MSPI controller.
+
 config SPI_BCM63XX
        tristate "Broadcom BCM63xx SPI controller"
        depends on BCM63XX

drivers/spi/Makefile +0 -0

index 6b58100..36872d2 100644

drivers/spi/Makefile +0 -0

 obj-$(CONFIG_SPI_BCM63XX)              += spi-bcm63xx.o
 obj-$(CONFIG_SPI_BCM63XX_HSSPI)                += spi-bcm63xx-hsspi.o
 obj-$(CONFIG_SPI_BFIN5XX)              += spi-bfin5xx.o

drivers/spi/spi-bcm-mspi.c +2 -0

index 502227d..32bb1f0 100644
@@ -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"
 

drivers/spi/spi-bcm-mspi.c +0 -5

        size_t read_offset;
-};
-
-static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
-{
-       return bcma_read32(mspi->core, offset);

drivers/spi/spi-bcm-mspi.c +24 -22

 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.

drivers/spi/spi-bcm-mspi.c +29 -0

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

drivers/spi/spi-bcm-mspi.c +22 -0

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

drivers/spi/spi-bcm-mspi.c +27 -8

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

drivers/spi/spi-bcm-mspi.c +4 -10

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

drivers/spi/spi-bcm-mspi.c +1 -1

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

Hi,

Quoted 25 lines · Jonathan Richardson

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 <jonathar-dY08KVG/lbpWk0Htik3J/[email protected]>

---
 drivers/spi/Kconfig        |    5 +
 drivers/spi/Makefile       |    1 +
 drivers/spi/spi-bcm-mspi.c |  228 ++++++++++++++++++++++++++++++++------------
 3 files changed, 171 insertions(+), 63 deletions(-)

drivers/spi/Kconfig +0 -0

index 766e08d..23f2357 100644

drivers/spi/Kconfig config SPI_BCMA_MSPI · line 124

Quoted 5 lines

drivers/spi/Kconfig +2 -0

@@ -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?

Quoted 10 lines

drivers/spi/Kconfig +3 -0

+       help
+         Enable support for the Broadcom MSPI controller.
+
 config SPI_BCM63XX
        tristate "Broadcom BCM63xx SPI controller"
        depends on BCM63XX

drivers/spi/Makefile +0 -0

index 6b58100..36872d2 100644

drivers/spi/Makefile obj-$(CONFIG_SPI_ATH79) += spi-ath79.o · line 19

Quoted 5 lines

drivers/spi/Makefile +1 -0

@@ -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?

Quoted 23 lines

drivers/spi/Makefile +0 -0

 obj-$(CONFIG_SPI_BCM63XX)              += spi-bcm63xx.o
 obj-$(CONFIG_SPI_BCM63XX_HSSPI)                += spi-bcm63xx-hsspi.o
 obj-$(CONFIG_SPI_BFIN5XX)              += spi-bfin5xx.o

drivers/spi/spi-bcm-mspi.c +2 -0

index 502227d..32bb1f0 100644
@@ -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"
 

drivers/spi/spi-bcm-mspi.c line 35

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +4 -1

@@ -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.

Quoted 6 lines

drivers/spi/spi-bcm-mspi.c +0 -5

        size_t read_offset;
-};
-
-static inline u32 bcm_mspi_read(struct bcm_mspi *mspi, u16 offset)
-{
-       return bcma_read32(mspi->core, offset);

drivers/spi/spi-bcm-mspi.c line 40

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +3 -6

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

Quoted 131 lines

drivers/spi/spi-bcm-mspi.c +24 -22

 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.

drivers/spi/spi-bcm-mspi.c static int bcm_mspi_transfer_one(struct spi_master *master, · line 229

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +10 -0

+ */
+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;

drivers/spi/spi-bcm-mspi.c static int bcm_mspi_transfer_one(struct spi_master *master, · line 242

Quoted 13 lines

drivers/spi/spi-bcm-mspi.c +13 -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

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

#if IS_ENABLED(CONFIG_SPI_BCM_MSPI)

Quoted 29 lines

drivers/spi/spi-bcm-mspi.c +29 -0

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

drivers/spi/spi-bcm-mspi.c static int bcm_mspi_transfer_one(struct spi_master *master, · line 281

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +10 -0

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

Quoted 22 lines

drivers/spi/spi-bcm-mspi.c +22 -0

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

drivers/spi/spi-bcm-mspi.c static int bcm_mspi_transfer_one(struct spi_master *master, · line 313

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +8 -0

+               .of_match_table = bcm_mspi_dt,
+       },
+       .probe = bcm_mspi_probe,
 };
 
+module_platform_driver(bcm_mspi_driver);
+
+#endif
+
+#ifdef CONFIG_SPI_BCMA_MSPI

likewise.

Quoted 68 lines

drivers/spi/spi-bcm-mspi.c +29 -11

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

drivers/spi/spi-bcm-mspi.c static const struct bcma_device_id bcm_mspi_bcma_tbl[] = { · line 263

Quoted 2 lines

drivers/spi/spi-bcm-mspi.c +0 -2

-       /* 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?

Quoted 19 lines

drivers/spi/spi-bcm-mspi.c +4 -10

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

drivers/spi/spi-bcm-mspi.c static const struct bcma_device_id bcm_mspi_bcma_tbl[] = { · line 385

Quoted 10 lines

drivers/spi/spi-bcm-mspi.c +2 -3

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

Quoted 20 lines

drivers/spi/spi-bcm-mspi.c +5 -3

        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
 

drivers/spi/spi-bcm-mspi.c static int __init bcm_mspi_module_init(void)

Quoted 3 lines

drivers/spi/spi-bcm-mspi.c +0 -1

 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/

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

Quoted 7 lines · Jonathan Richardson

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

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 <jonathar-dY08KVG/lbpWk0Htik3J/[email protected]>

---
 drivers/spi/spi-bcm-mspi.c |   50 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

drivers/spi/spi-bcm-mspi.c +17 -0

index 32bb1f0..9320de9 100644
@@ -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;
 

drivers/spi/spi-bcm-mspi.c +20 -0

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