Message ID | 20191203034519.5640-5-chris.brandt@renesas.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Series | spi: Add Renesas SPIBSC controller | expand |
On Mon, Dec 02, 2019 at 10:45:17PM -0500, Chris Brandt wrote: > +config SPI_SPIBSC > + tristate "Renesas SPI Multi I/O Bus Controller" > + depends on ARCH_R7S72100 || ARCH_R7S9210 I'm not seeing any build dependency here, please add an || COMPILE_TEST for build coverage. > +++ b/drivers/spi/spi-spibsc.c > @@ -0,0 +1,609 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SPI Bus Space Controller (SPIBSC) bus driver Please make the entire comment block here a C++ one so things look more intentional. > +static void spibsc_write(struct spibsc_priv *sbsc, int reg, u32 val) > +{ > + iowrite32(val, sbsc->base + reg); > +} > +static void spibsc_write8(struct spibsc_priv *sbsc, int reg, u8 val) Blank likes between functions, please see coding-style.rst. Looking at a bunch of the stuff here it looks like you could benefit from regmap, it's got lots of debug infrastructure. > + if (tx) > + pr_debug("spibsc: send data: "); > + else > + pr_debug("spibsc: recv data: "); dev_dbg() if you're going to do tis. > + > + for (i = 0; i < len; ) { > + sprintf(line_buffer + line_index, " %02X", buf[i]); snprintf()! > +static int spibsc_transfer_one_message(struct spi_controller *master, > + struct spi_message *msg) > +{ > + struct spibsc_priv *sbsc = spi_controller_get_devdata(master); > + struct spi_transfer *t, *t_last; > + u8 tx_data[MAX_CMD_LEN]; > + int tx_only; > + u8 tx_len; > + int ret; > + > + t_last = list_last_entry(&msg->transfers, struct spi_transfer, > + transfer_list); > + /* defaults */ > + ret = 0; > + sbsc->last_xfer = 0; > + tx_only = 1; > + > + /* Analyze the messages */ > + t = list_first_entry(&msg->transfers, struct spi_transfer, > + transfer_list); > + if (t->rx_buf) { > + dev_dbg(sbsc->dev, "Cannot Rx without Tx first!\n"); > + return -EIO; These errors should probably be -EINVAL, you're failing on validation here. > + } > + list_for_each_entry(t, &msg->transfers, transfer_list) { Blank line here please as well. > + if (spi->bits_per_word != 8) { > + dev_err(sbsc->dev, "bits_per_word must be 8\n"); > + return -EIO; > + } The core will validate this for you. > + master->num_chipselect = 1; > + master->mode_bits = SPI_CPOL | SPI_CPHA; > + master->setup = spibsc_setup; > + master->transfer_one_message = spibsc_transfer_one_message; Set bits_per_word_mask here. > + dev_info(&pdev->dev, "probed\n"); > + Remove this, it's just noise. > +static int spibsc_remove(struct platform_device *pdev) > +{ > + struct spibsc_priv *sbsc = dev_get_drvdata(&pdev->dev); > + > + pm_runtime_put(&pdev->dev); > + pm_runtime_disable(&pdev->dev); There seems to be no purpose in the runtime PM code in this driver, there's no PM operations of any kind and the driver holds a runtime PM reference for the entire lifetime of the device. > + spi_unregister_controller(sbsc->master); You registered the controller with devm_, there's no need to unregister it and if you do you need to use a matching devm_ unregiser.
Hello Mark, On Tue, Dec 3, 2019, Mark Brown wrote: > On Mon, Dec 02, 2019 at 10:45:17PM -0500, Chris Brandt wrote: > > > +config SPI_SPIBSC > > + tristate "Renesas SPI Multi I/O Bus Controller" > > + depends on ARCH_R7S72100 || ARCH_R7S9210 > > I'm not seeing any build dependency here, please add an || COMPILE_TEST for > build coverage. (snip) Thank you for your review! I have no complaints about your comments so I will make the changes and send out a v2. Chris
Hi Chris, On Tue, Dec 3, 2019 at 4:46 AM Chris Brandt <chris.brandt@renesas.com> wrote: > Add a driver for the SPIBSC controller in Renesas SoC devices. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Thanks for your patch! > --- /dev/null > +++ b/drivers/spi/spi-spibsc.c > @@ -0,0 +1,609 @@ > +// SPDX-License-Identifier: GPL-2.0 GPL-2.0-only > +static void spibsc_print_data(struct spibsc_priv *sbsc, u8 tx, const u8 *buf, > + int len) > +{ > +#if defined(DEBUG_PRINT_DATA) > + char line_buffer[3*16+1]; > + int i, line_index = 0; > + > + if (tx) > + pr_debug("spibsc: send data: "); > + else > + pr_debug("spibsc: recv data: "); > + > + for (i = 0; i < len; ) { > + sprintf(line_buffer + line_index, " %02X", buf[i]); > + line_index += 3; > + i++; > + if (i % 16 == 0) { > + pr_debug(line_buffer); > + line_index = 0; > + } > + } > + if (i % 16 != 0) > + pr_debug(line_buffer); > + else > + pr_debug("\n"); > +#endif print_hex_dump_debug()? > +} > + > +static int spibsc_wait_trans_completion(struct spibsc_priv *sbsc) > +{ > + int t = 256 * 100000; > + > + while (t--) { > + if (spibsc_read(sbsc, CMNSR) & CMNSR_TEND) > + return 0; > + ndelay(1); > + } So this may busy loop for up to 25.6 ms? Oops... Can you use the interrupt (SPIHF) instead, signalling a completion? > + > + dev_err(sbsc->dev, "Timeout waiting for TEND\n"); > + return -ETIMEDOUT; > +} > + /* Use 'Option Data' for 3rd-6th bytes */ > + switch (tx_len) { > + case 6: > + dropr |= DROPR_OPD0(tx_data[5]); > + opde |= (1 << 0); Both checkpatch and gcc tell you to add fallthrough coments. > + case 5: > + dropr |= DROPR_OPD1(tx_data[4]); > + opde |= (1 << 1); > + case 4: > + dropr |= DROPR_OPD2(tx_data[3]); > + opde |= (1 << 2); > + case 3: > + dropr |= DROPR_OPD3(tx_data[2]); > + opde |= (1 << 3); > + drenr |= DRENR_OPDE(opde); > + } > +static int spibsc_transfer_one_message(struct spi_controller *master, > + struct spi_message *msg) > +{ > + struct spibsc_priv *sbsc = spi_controller_get_devdata(master); > + struct spi_transfer *t, *t_last; > + u8 tx_data[MAX_CMD_LEN]; > + int tx_only; > + u8 tx_len; > + int ret; > + > + t_last = list_last_entry(&msg->transfers, struct spi_transfer, > + transfer_list); > + /* defaults */ > + ret = 0; > + sbsc->last_xfer = 0; > + tx_only = 1; > + > + /* Analyze the messages */ > + t = list_first_entry(&msg->transfers, struct spi_transfer, > + transfer_list); > + if (t->rx_buf) { > + dev_dbg(sbsc->dev, "Cannot Rx without Tx first!\n"); > + return -EIO; > + } > + list_for_each_entry(t, &msg->transfers, transfer_list) { > + if (t->rx_buf) { > + tx_only = 0; > + if (t != t_last) { > + dev_dbg(sbsc->dev, "RX transaction is not the last transaction!\n"); > + return -EIO; > + } > + } > + if (t->cs_change) { > + dev_err(sbsc->dev, "cs_change not supported"); > + return -EIO; > + } > + } If you would do the checks above in .prepare_message() (like e.g. rspi does)... > + > + /* Tx Only (SPI Mode is used) */ > + if (tx_only == 1) { > + > + dev_dbg(sbsc->dev, "%s: TX only\n", __func__); > + > + /* Initialize for SPI Mode */ > + spibsc_write(sbsc, CMNCR, CMNCR_INIT); > + > + /* Send messages */ > + list_for_each_entry(t, &msg->transfers, transfer_list) { > + > + if (t == t_last) > + sbsc->last_xfer = 1; > + > + ret = spibsc_send_data(sbsc, t->tx_buf, t->len); > + if (ret) > + break; > + > + msg->actual_length += t->len; > + } > + > + /* Done */ > + msg->status = ret; > + spi_finalize_current_message(master); > + return ret; > + } > + > + /* Tx, then RX (Data Read Mode is used) */ > + dev_dbg(sbsc->dev, "%s: Tx then Rx\n", __func__); > + > + /* Buffer up the transmit portion (cmd + addr) so we can send it all at > + * once > + */ > + tx_len = 0; > + list_for_each_entry(t, &msg->transfers, transfer_list) { > + if (t->tx_buf) { > + if ((tx_len + t->len) > sizeof(tx_data)) { > + dev_dbg(sbsc->dev, "Command too big (%d)\n", > + tx_len + t->len); > + return -EIO; > + } > + memcpy(tx_data + tx_len, t->tx_buf, t->len); > + tx_len += t->len; > + } > + > + if (t->rx_buf) > + ret = spibsc_send_recv_data(sbsc, tx_data, tx_len, > + t->rx_buf, t->len); > + > + msg->actual_length += t->len; > + } > + > + msg->status = ret; > + spi_finalize_current_message(master); ... can't you just use .transfer_one(), instead of duplicating the logic from spi_transfer_one_message()? Or is there some special detail that's not obvious to the casual reviewer? > +static const struct of_device_id of_spibsc_match[] = { > + { .compatible = "renesas,spibsc"}, > + { .compatible = "renesas,r7s72100-spibsc", .data = (void *)HAS_SPBCR}, > + { .compatible = "renesas,r7s9210-spibsc"}, Do you need to match against all 3 in the driver? Does SPIBSC work on RZ/A1 when not setting HAS_SPBCR? If not, the fallback to "renesas,spibsc" is not valid for RZ/A1. > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, of_spibsc_match); > + > +static struct platform_driver spibsc_driver = { > + .probe = spibsc_probe, > + .remove = spibsc_remove, > + .driver = { > + .name = "spibsc", > + .owner = THIS_MODULE, > + .of_match_table = of_spibsc_match, > + }, > +}; > +module_platform_driver(spibsc_driver); > + > +MODULE_DESCRIPTION("Renesas SPIBSC SPI Flash driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Chris Brandt"); > -- > 2.23.0 > -- Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Mark, On Tue, Dec 3, 2019 at 3:19 PM Mark Brown <broonie@kernel.org> wrote: > On Mon, Dec 02, 2019 at 10:45:17PM -0500, Chris Brandt wrote: > > +static int spibsc_remove(struct platform_device *pdev) > > +{ > > + struct spibsc_priv *sbsc = dev_get_drvdata(&pdev->dev); > > + > > + pm_runtime_put(&pdev->dev); > > + pm_runtime_disable(&pdev->dev); > > There seems to be no purpose in the runtime PM code in this > driver, there's no PM operations of any kind and the driver holds > a runtime PM reference for the entire lifetime of the device. It matters for the clock domain (assumed the module clock is not always marked as a critical clock). Gr{oetje,eeting}s, Geert
On Tue, Dec 03, 2019 at 07:28:18PM +0100, Geert Uytterhoeven wrote: > On Tue, Dec 3, 2019 at 4:46 AM Chris Brandt <chris.brandt@renesas.com> wrote: > > +static const struct of_device_id of_spibsc_match[] = { > > + { .compatible = "renesas,spibsc"}, > > + { .compatible = "renesas,r7s72100-spibsc", .data = (void *)HAS_SPBCR}, > > + { .compatible = "renesas,r7s9210-spibsc"}, > Do you need to match against all 3 in the driver? > Does SPIBSC work on RZ/A1 when not setting HAS_SPBCR? > If not, the fallback to "renesas,spibsc" is not valid for RZ/A1. I do think it's useful to explicitly list all the compatibles in the driver, it documents the handling needed for each of the variants and it provides some robustness against DTs that neglect to list the fallback compatibles.
On Tue, Dec 03, 2019 at 07:29:45PM +0100, Geert Uytterhoeven wrote: > On Tue, Dec 3, 2019 at 3:19 PM Mark Brown <broonie@kernel.org> wrote: > > > + pm_runtime_put(&pdev->dev); > > > + pm_runtime_disable(&pdev->dev); > > There seems to be no purpose in the runtime PM code in this > > driver, there's no PM operations of any kind and the driver holds > > a runtime PM reference for the entire lifetime of the device. > It matters for the clock domain (assumed the module clock is not always > marked as a critical clock). That seems like a problem with what the clock domains are doing surely? The default is supposed to be that if runtime PM isn't enabled we get the behaviour the driver is manually implementing here. Besides, if this is actually impacting power management I'd expect us to be letting the IP idle rather than holding it powered up all the time.
Hi Mark, On Wed, Dec 4, 2019 at 12:25 PM Mark Brown <broonie@kernel.org> wrote: > On Tue, Dec 03, 2019 at 07:29:45PM +0100, Geert Uytterhoeven wrote: > > On Tue, Dec 3, 2019 at 3:19 PM Mark Brown <broonie@kernel.org> wrote: > > > > + pm_runtime_put(&pdev->dev); > > > > + pm_runtime_disable(&pdev->dev); > > > > There seems to be no purpose in the runtime PM code in this > > > driver, there's no PM operations of any kind and the driver holds > > > a runtime PM reference for the entire lifetime of the device. > > > It matters for the clock domain (assumed the module clock is not always > > marked as a critical clock). > > That seems like a problem with what the clock domains are doing > surely? The default is supposed to be that if runtime PM isn't > enabled we get the behaviour the driver is manually implementing Unfortunately not: if the driver doesn't implement Runtime PM, the default is to not do anything. Later, unused clocks will be disabled, and the device will stop functioning. > here. Besides, if this is actually impacting power management > I'd expect us to be letting the IP idle rather than holding it > powered up all the time. That would be better, definitely. However, that's just an optimization over holding it powered up all the time. Gr{oetje,eeting}s, Geert
Hello Mark, On Tue, Dec 3, 2019, Mark Brown wrote: > > +static void spibsc_write(struct spibsc_priv *sbsc, int reg, u32 val) > > +{ > > + iowrite32(val, sbsc->base + reg); > > +} > > +static void spibsc_write8(struct spibsc_priv *sbsc, int reg, u8 val) > > Looking at a bunch of the stuff here it looks like you could benefit from > regmap, it's got lots of debug infrastructure. Thank you for the suggestion, but I looked into using regmap, and there are a lot of drivers that use it, but I don't think it's going to work well for me. Regmap assumes that all the registers will be the same size. I have to have functions that write with different widths (8/16/32) for a reason. Chris
On Wed, Dec 04, 2019 at 03:51:48PM +0000, Chris Brandt wrote: > On Tue, Dec 3, 2019, Mark Brown wrote: > > Looking at a bunch of the stuff here it looks like you could benefit from > > regmap, it's got lots of debug infrastructure. > Thank you for the suggestion, but I looked into using regmap, and there > are a lot of drivers that use it, but I don't think it's going to work > well for me. > Regmap assumes that all the registers will be the same size. I have to > have functions that write with different widths (8/16/32) for a reason. You *can* have more than one regmap for a device, or if it really only is one or two registers open code accesses to just those registers.
Hi Geert, Thank you for your review and suggestions! On Tue, Dec 3, 2019, Geert Uytterhoeven wrote: > > +static int spibsc_wait_trans_completion(struct spibsc_priv *sbsc) { > > + int t = 256 * 100000; > > + > > + while (t--) { > > + if (spibsc_read(sbsc, CMNSR) & CMNSR_TEND) > > + return 0; > > + ndelay(1); > > + } > > So this may busy loop for up to 25.6 ms? Oops... > > Can you use the interrupt (SPIHF) instead, signalling a completion? RZ/A1 doesn't have any interrupts. And I think the interrupts in RZ/A2 only work for HyperFlash (not QSPI). But, 25.6ms is way too long! I'll assume the slowest clock to be 1MHz and the max FIFO transfer of 4 bytes. That's 32us, so that's what I'll use. > > + if (t->cs_change) { > > + dev_err(sbsc->dev, "cs_change not supported"); > > + return -EIO; > > + } > > + } > > If you would do the checks above in .prepare_message() (like e.g. rspi > does)... OK, Thanks. :) > > + case 6: > > + dropr |= DROPR_OPD0(tx_data[5]); > > + opde |= (1 << 0); > > Both checkpatch and gcc tell you to add fallthrough coments. OK, fixed. > ... can't you just use .transfer_one(), instead of duplicating the logic from > spi_transfer_one_message()? > Or is there some special detail that's not obvious to the casual reviewer? I guess .transfer_one() could be used if I kept shoving more stuff into .prepare_message(). But, the screwy thing about this controller is that messages that are 'Tx only' are processed differently then 'Tx then Rx' messages and not like a traditional SPI controller. By implementing both conditions in .spi_transfer_one_message(), it makes that more clear for the next person in my opinion because you can see that sometimes we have to work with the entire message as a whole, not just individual pieces. > > +static const struct of_device_id of_spibsc_match[] = { > > + { .compatible = "renesas,spibsc"}, > > + { .compatible = "renesas,r7s72100-spibsc", .data = (void > *)HAS_SPBCR}, > > + { .compatible = "renesas,r7s9210-spibsc"}, > > Do you need to match against all 3 in the driver? > Does SPIBSC work on RZ/A1 when not setting HAS_SPBCR? > If not, the fallback to "renesas,spibsc" is not valid for RZ/A1. OK, I dropped "renesas,spibsc". I like having individual SoC names anyway. Chris
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 870f7797b56b..405d7e73963e 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -575,6 +575,14 @@ config SPI_RSPI help SPI driver for Renesas RSPI and QSPI blocks. +config SPI_SPIBSC + tristate "Renesas SPI Multi I/O Bus Controller" + depends on ARCH_R7S72100 || ARCH_R7S9210 + help + Also referred to as the SPI Bus Space Controller (SPIBSC). + This controller was designed specifically for accessing QSPI flash + devices. + config SPI_QCOM_QSPI tristate "QTI QSPI controller" depends on ARCH_QCOM diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index bb49c9e6d0a0..9525256c4d51 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -99,6 +99,7 @@ obj-$(CONFIG_SPI_SH_SCI) += spi-sh-sci.o obj-$(CONFIG_SPI_SIFIVE) += spi-sifive.o obj-$(CONFIG_SPI_SIRF) += spi-sirf.o obj-$(CONFIG_SPI_SLAVE_MT27XX) += spi-slave-mt27xx.o +obj-$(CONFIG_SPI_SPIBSC) += spi-spibsc.o obj-$(CONFIG_SPI_SPRD) += spi-sprd.o obj-$(CONFIG_SPI_SPRD_ADI) += spi-sprd-adi.o obj-$(CONFIG_SPI_STM32) += spi-stm32.o diff --git a/drivers/spi/spi-spibsc.c b/drivers/spi/spi-spibsc.c new file mode 100644 index 000000000000..ac7e6c84417c --- /dev/null +++ b/drivers/spi/spi-spibsc.c @@ -0,0 +1,609 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SPI Bus Space Controller (SPIBSC) bus driver + * Otherwise known as a SPI Multi I/O Bus Controller + * + * Copyright (C) 2019 Renesas Electronics + * + */ +#include <linux/platform_device.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/clk.h> +#include <linux/pm_runtime.h> +#include <linux/io.h> +#include <linux/delay.h> +#include <linux/spi/spi.h> + +/* SPIBSC registers */ +#define CMNCR 0x00 +#define SSLDR 0x04 +#define SPBCR 0x08 +#define DRCR 0x0c +#define DRCMR 0x10 +#define DREAR 0x14 +#define DROPR 0x18 +#define DRENR 0x1c +#define SMCR 0x20 +#define SMCMR 0x24 +#define SMADR 0x28 +#define SMOPR 0x2c +#define SMENR 0x30 +#define SMRDR0 0x38 +#define SMRDR1 0x3c +#define SMWDR0 0x40 +#define SMWDR1 0x44 +#define CMNSR 0x48 +#define SMDMCR 0x60 +#define SMDRENR 0x64 + +/* CMNCR */ +#define CMNCR_MD BIT(31) +#define CMNCR_SFDE BIT(24) +#define CMNCR_MOIIO3(x) (((u32)(x) & 0x3) << 22) +#define CMNCR_MOIIO2(x) (((u32)(x) & 0x3) << 20) +#define CMNCR_MOIIO1(x) (((u32)(x) & 0x3) << 18) +#define CMNCR_MOIIO0(x) (((u32)(x) & 0x3) << 16) +#define CMNCR_IO3FV(x) (((u32)(x) & 0x3) << 14) +#define CMNCR_IO2FV(x) (((u32)(x) & 0x3) << 12) +#define CMNCR_IO0FV(x) (((u32)(x) & 0x3) << 8) +#define CMNCR_CPHAT BIT(6) +#define CMNCR_CPHAR BIT(5) +#define CMNCR_SSLP BIT(4) +#define CMNCR_CPOL BIT(3) +#define CMNCR_BSZ(n) (((u32)(n) & 0x3) << 0) +#define OUT_0 (0u) +#define OUT_1 (1u) +#define OUT_REV (2u) +#define OUT_HIZ (3u) +#define BSZ_SINGLE (0) +#define BSZ_DUAL (1) +#define CMNCR_INIT (CMNCR_MD | \ + CMNCR_SFDE | \ + CMNCR_MOIIO3(OUT_HIZ) | \ + CMNCR_MOIIO2(OUT_HIZ) | \ + CMNCR_MOIIO1(OUT_HIZ) | \ + CMNCR_MOIIO0(OUT_HIZ) | \ + CMNCR_IO3FV(OUT_HIZ) | \ + CMNCR_IO2FV(OUT_HIZ) | \ + CMNCR_IO0FV(OUT_HIZ) | \ + CMNCR_CPHAR | \ + CMNCR_BSZ(BSZ_SINGLE)) + +/* SSLDR */ +#define SSLDR_SPNDL(x) (((u32)(x) & 0x7) << 16) +#define SSLDR_SLNDL(x) (((u32)(x) & 0x7) << 8) +#define SSLDR_SCKDL(x) (((u32)(x) & 0x7) << 0) +#define SSLDR_INIT (SSLDR_SPNDL(3) | SSLDR_SLNDL(3) | SSLDR_SCKDL(3)) + +/* SPBCR */ +#define SPBCR_SPBR(x) (((u32)(x) & 0xff) << 8) +#define SPBCR_BRDV(x) (((u32)(x) & 0x3) << 0) + +/* DRCR (read mode) */ +#define DRCR_SSLN BIT(24) +#define DRCR_RBURST(x) (((u32)(x) & 0xf) << 16) +#define DRCR_RCF BIT(9) +#define DRCR_RBE BIT(8) +#define DRCR_SSLE BIT(0) + +/* DROPR (read mode) */ +#define DROPR_OPD3(o) (((u32)(o) & 0xff) << 24) +#define DROPR_OPD2(o) (((u32)(o) & 0xff) << 16) +#define DROPR_OPD1(o) (((u32)(o) & 0xff) << 8) +#define DROPR_OPD0(o) (((u32)(o) & 0xff) << 0) + +/* DRENR (read mode) */ +#define DRENR_CDE BIT(14) +#define DRENR_OCDE BIT(12) +#define DRENR_OPDE(o) (((u32)(o) & 0xf) << 4) + +/* SMCR (spi mode) */ +#define SMCR_SSLKP BIT(8) +#define SMCR_SPIRE BIT(2) +#define SMCR_SPIWE BIT(1) +#define SMCR_SPIE BIT(0) + +/* SMCMR (spi mode) */ +#define SMCMR_CMD(c) (((u32)(c) & 0xff) << 16) +#define SMCMR_OCMD(o) (((u32)(o) & 0xff) << 0) + +/* SMENR (spi mode) */ +#define SMENR_SPIDE(b) (((u32)(b) & 0xf) << 0) +#define SPIDE_8BITS (0x8) +#define SPIDE_16BITS (0xc) +#define SPIDE_32BITS (0xf) + +/* CMNSR (spi mode) */ +#define CMNSR_SSLF BIT(1) +#define CMNSR_TEND BIT(0) + +#define MAX_CMD_LEN 6 + +/* HW Option Flags */ +#define HAS_SPBCR BIT(0) + +struct spibsc_priv { + void __iomem *base; /* register base */ + void __iomem *mmio; /* memory mapped io space of SPI Flash */ + int mmio_size; + struct spi_controller *master; + struct device *dev; + struct clk *clk; + u8 last_xfer; + u32 flags; +}; + +static void spibsc_write(struct spibsc_priv *sbsc, int reg, u32 val) +{ + iowrite32(val, sbsc->base + reg); +} +static void spibsc_write8(struct spibsc_priv *sbsc, int reg, u8 val) +{ + iowrite8(val, sbsc->base + reg); +} +static void spibsc_write16(struct spibsc_priv *sbsc, int reg, u16 val) +{ + iowrite16(val, sbsc->base + reg); +} +static u32 spibsc_read(struct spibsc_priv *sbsc, int reg) +{ + return ioread32(sbsc->base + reg); +} + +static void spibsc_print_data(struct spibsc_priv *sbsc, u8 tx, const u8 *buf, + int len) +{ +#if defined(DEBUG_PRINT_DATA) + char line_buffer[3*16+1]; + int i, line_index = 0; + + if (tx) + pr_debug("spibsc: send data: "); + else + pr_debug("spibsc: recv data: "); + + for (i = 0; i < len; ) { + sprintf(line_buffer + line_index, " %02X", buf[i]); + line_index += 3; + i++; + if (i % 16 == 0) { + pr_debug(line_buffer); + line_index = 0; + } + } + if (i % 16 != 0) + pr_debug(line_buffer); + else + pr_debug("\n"); +#endif +} + +static int spibsc_wait_trans_completion(struct spibsc_priv *sbsc) +{ + int t = 256 * 100000; + + while (t--) { + if (spibsc_read(sbsc, CMNSR) & CMNSR_TEND) + return 0; + ndelay(1); + } + + dev_err(sbsc->dev, "Timeout waiting for TEND\n"); + return -ETIMEDOUT; +} + +/* + * This function sends data using 'SPI operation mode'. It is intended to be + * used only with SPI Flash commands that do not require any reading back from + * the SPI flash. + */ +static int spibsc_send_data(struct spibsc_priv *sbsc, const u8 *data, int len) +{ + u32 smcr, smenr, smwdr0; + int ret, unit, sslkp; + + /* print data (for debugging) */ + spibsc_print_data(sbsc, 1, data, len); + + while (len > 0) { + if (len >= 4) { + unit = 4; + smenr = SMENR_SPIDE(SPIDE_32BITS); + } else { + unit = len; + if (unit == 3) + unit = 2; + + if (unit >= 2) + smenr = SMENR_SPIDE(SPIDE_16BITS); + else + smenr = SMENR_SPIDE(SPIDE_8BITS); + } + + /* set 4bytes data, bit stream */ + smwdr0 = *data++; + if (unit >= 2) + smwdr0 |= (u32)(*data++ << 8); + if (unit >= 3) + smwdr0 |= (u32)(*data++ << 16); + if (unit >= 4) + smwdr0 |= (u32)(*data++ << 24); + + /* mask unwrite area */ + if (unit == 3) + smwdr0 |= 0xFF000000; + else if (unit == 2) + smwdr0 |= 0xFFFF0000; + else if (unit == 1) + smwdr0 |= 0xFFFFFF00; + + /* write send data. */ + if (unit == 2) + spibsc_write16(sbsc, SMWDR0, (u16)smwdr0); + else if (unit == 1) + spibsc_write8(sbsc, SMWDR0, (u8)smwdr0); + else + spibsc_write(sbsc, SMWDR0, smwdr0); + + len -= unit; + if ((len <= 0) && sbsc->last_xfer) + sslkp = 0; + else + sslkp = 1; + + /* set params */ + spibsc_write(sbsc, SMCMR, 0); + spibsc_write(sbsc, SMADR, 0); + spibsc_write(sbsc, SMOPR, 0); + spibsc_write(sbsc, SMENR, smenr); + + /* start spi transfer */ + smcr = SMCR_SPIE|SMCR_SPIWE; + if (sslkp) + smcr |= SMCR_SSLKP; + spibsc_write(sbsc, SMCR, smcr); + + ret = spibsc_wait_trans_completion(sbsc); + if (ret) + return ret; + } + return 0; +} + +/* + * This function uses 'Data Read' mode to send the command (and address) then + * read data back out. + * The HW was designed such that you program the registers with the command, + * base address, additional command data, etc... But, that makes things too + * difficult because it means this driver has to pick out those parameters from + * the data stream that was passed. + * Instead, we will ignore how the HW was 'supposed' to be used and just + * blindly put the Tx data (commands) to send in the registers in the order + * in which we know they will be transmitted: + * + * [DRCMR.CMD]+[DRCMR.OCMD]+[DROPR.OPD3]+[DROPR.OPD2]+[DROPR.OPD1]+[DROPR.OPD0] + * + * We can send up to 6 bytes this way. + * We will tell the HW to skip sending the 'address' because we are secretly + * including it in the "command" and that way we can leave the address registers + * blank. + * + * Note that when reading data, the HW will read in 8-byte units which usually + * is not an issue for SPI Flash devices. + */ +static int spibsc_send_recv_data(struct spibsc_priv *sbsc, u8 *tx_data, + int tx_len, u8 *rx_data, int rx_len) +{ + u32 drcmr, drenr, dropr; + u8 opde; + + dev_dbg(sbsc->dev, "%s (tx=%d, rx=%d)\n", __func__, tx_len, rx_len); + + if (tx_len > MAX_CMD_LEN) { + dev_err(sbsc->dev, "Command length too long (%d)", tx_len); + return -EIO; + } + + if (rx_len > sbsc->mmio_size) { + dev_err(sbsc->dev, "Receive length too long (%d)", rx_len); + return -EIO; + } + + /* Setup Data Read mode + * Flush read cache and enable burst mode. Burst mode is required + * in order to keep chip select low between read transfers, but it + * also means data is read in 8-byte intervals. + */ + spibsc_write(sbsc, DRCR, DRCR_SSLN | DRCR_RCF | DRCR_RBE | DRCR_SSLE); + spibsc_read(sbsc, DRCR); + + /* Enter Data Read mode */ + spibsc_write(sbsc, CMNCR, 0x01FFF300); + drcmr = 0; + drenr = 0; + dropr = 0; + opde = 0; + + if (tx_len >= 1) { + /* Use 'Command' register for the 1st byte */ + drcmr |= SMCMR_CMD(tx_data[0]); + drenr |= DRENR_CDE; + } + + if (tx_len >= 2) { + /* Use 'Optional Command' register for the 2nd byte */ + drcmr |= SMCMR_OCMD(tx_data[1]); + drenr |= DRENR_OCDE; + } + + /* Use 'Option Data' for 3rd-6th bytes */ + switch (tx_len) { + case 6: + dropr |= DROPR_OPD0(tx_data[5]); + opde |= (1 << 0); + case 5: + dropr |= DROPR_OPD1(tx_data[4]); + opde |= (1 << 1); + case 4: + dropr |= DROPR_OPD2(tx_data[3]); + opde |= (1 << 2); + case 3: + dropr |= DROPR_OPD3(tx_data[2]); + opde |= (1 << 3); + drenr |= DRENR_OPDE(opde); + } + + spibsc_write(sbsc, DRCMR, drcmr); + spibsc_write(sbsc, DROPR, dropr); + spibsc_write(sbsc, DRENR, drenr); + + /* This internal bus access is what will trigger the actual Tx/Rx + * operation. Remember, we don't care about the address. + */ + memcpy(rx_data, sbsc->mmio, rx_len); + + /* Deactivate chip select */ + spibsc_write(sbsc, DRCR, DRCR_SSLN); + + /* Print data (for debugging) */ + spibsc_print_data(sbsc, 1, tx_data, tx_len); + spibsc_print_data(sbsc, 0, rx_data, rx_len); + + return 0; +} + +static int spibsc_transfer_one_message(struct spi_controller *master, + struct spi_message *msg) +{ + struct spibsc_priv *sbsc = spi_controller_get_devdata(master); + struct spi_transfer *t, *t_last; + u8 tx_data[MAX_CMD_LEN]; + int tx_only; + u8 tx_len; + int ret; + + t_last = list_last_entry(&msg->transfers, struct spi_transfer, + transfer_list); + /* defaults */ + ret = 0; + sbsc->last_xfer = 0; + tx_only = 1; + + /* Analyze the messages */ + t = list_first_entry(&msg->transfers, struct spi_transfer, + transfer_list); + if (t->rx_buf) { + dev_dbg(sbsc->dev, "Cannot Rx without Tx first!\n"); + return -EIO; + } + list_for_each_entry(t, &msg->transfers, transfer_list) { + if (t->rx_buf) { + tx_only = 0; + if (t != t_last) { + dev_dbg(sbsc->dev, "RX transaction is not the last transaction!\n"); + return -EIO; + } + } + if (t->cs_change) { + dev_err(sbsc->dev, "cs_change not supported"); + return -EIO; + } + } + + /* Tx Only (SPI Mode is used) */ + if (tx_only == 1) { + + dev_dbg(sbsc->dev, "%s: TX only\n", __func__); + + /* Initialize for SPI Mode */ + spibsc_write(sbsc, CMNCR, CMNCR_INIT); + + /* Send messages */ + list_for_each_entry(t, &msg->transfers, transfer_list) { + + if (t == t_last) + sbsc->last_xfer = 1; + + ret = spibsc_send_data(sbsc, t->tx_buf, t->len); + if (ret) + break; + + msg->actual_length += t->len; + } + + /* Done */ + msg->status = ret; + spi_finalize_current_message(master); + return ret; + } + + /* Tx, then RX (Data Read Mode is used) */ + dev_dbg(sbsc->dev, "%s: Tx then Rx\n", __func__); + + /* Buffer up the transmit portion (cmd + addr) so we can send it all at + * once + */ + tx_len = 0; + list_for_each_entry(t, &msg->transfers, transfer_list) { + if (t->tx_buf) { + if ((tx_len + t->len) > sizeof(tx_data)) { + dev_dbg(sbsc->dev, "Command too big (%d)\n", + tx_len + t->len); + return -EIO; + } + memcpy(tx_data + tx_len, t->tx_buf, t->len); + tx_len += t->len; + } + + if (t->rx_buf) + ret = spibsc_send_recv_data(sbsc, tx_data, tx_len, + t->rx_buf, t->len); + + msg->actual_length += t->len; + } + + msg->status = ret; + spi_finalize_current_message(master); + + return ret; +} + +static int spibsc_setup(struct spi_device *spi) +{ + struct spibsc_priv *sbsc = spi_controller_get_devdata(spi->master); + u32 max_speed_hz; + unsigned long rate; + u8 spbr; + u8 div_ratio; + + if (spi->bits_per_word != 8) { + dev_err(sbsc->dev, "bits_per_word must be 8\n"); + return -EIO; + } + + if (sbsc->flags & HAS_SPBCR) { + max_speed_hz = spi->max_speed_hz; + rate = clk_get_rate(sbsc->clk); + + div_ratio = 2; + spbr = 1; + while ((rate / div_ratio) > max_speed_hz) { + spbr++; + div_ratio += 2; + if (spbr == 255) + break; + } + spibsc_write(sbsc, SPBCR, SPBCR_SPBR(spbr) | SPBCR_BRDV(0)); + dev_dbg(sbsc->dev, "Clock set to %ld Hz\n", rate/div_ratio); + } + + return 0; +} + +static int spibsc_probe(struct platform_device *pdev) +{ + struct resource *res; + struct spi_controller *master; + struct spibsc_priv *sbsc; + int ret; + + master = spi_alloc_master(&pdev->dev, sizeof(*sbsc)); + if (!master) + return -ENOMEM; + + sbsc = spi_controller_get_devdata(master); + dev_set_drvdata(&pdev->dev, sbsc); + sbsc->master = master; + sbsc->dev = &pdev->dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sbsc->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(sbsc->base)) { + ret = PTR_ERR(sbsc->base); + goto error; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + sbsc->mmio_size = resource_size(res); + sbsc->mmio = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(sbsc->mmio)) { + ret = PTR_ERR(sbsc->mmio); + goto error; + } + + sbsc->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(sbsc->clk)) { + dev_err(&pdev->dev, "cannot get clock\n"); + ret = PTR_ERR(sbsc->clk); + goto error; + } + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); + + sbsc->flags = (u32) of_device_get_match_data(&pdev->dev); + + master->bus_num = pdev->id; + master->num_chipselect = 1; + master->mode_bits = SPI_CPOL | SPI_CPHA; + master->setup = spibsc_setup; + master->transfer_one_message = spibsc_transfer_one_message; + master->dev.of_node = pdev->dev.of_node; + + /* Initialize HW */ + spibsc_write(sbsc, CMNCR, CMNCR_INIT); + spibsc_write(sbsc, DRCR, DRCR_RCF); + spibsc_write(sbsc, SSLDR, SSLDR_INIT); + + ret = devm_spi_register_controller(&pdev->dev, master); + if (ret < 0) { + dev_err(&pdev->dev, "spi_register_controller error.\n"); + goto error; + } + + dev_info(&pdev->dev, "probed\n"); + + return 0; + +error: + pm_runtime_put(&pdev->dev); + pm_runtime_disable(&pdev->dev); + + spi_controller_put(master); + + return ret; +} + +static int spibsc_remove(struct platform_device *pdev) +{ + struct spibsc_priv *sbsc = dev_get_drvdata(&pdev->dev); + + pm_runtime_put(&pdev->dev); + pm_runtime_disable(&pdev->dev); + spi_unregister_controller(sbsc->master); + + return 0; +} + +static const struct of_device_id of_spibsc_match[] = { + { .compatible = "renesas,spibsc"}, + { .compatible = "renesas,r7s72100-spibsc", .data = (void *)HAS_SPBCR}, + { .compatible = "renesas,r7s9210-spibsc"}, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, of_spibsc_match); + +static struct platform_driver spibsc_driver = { + .probe = spibsc_probe, + .remove = spibsc_remove, + .driver = { + .name = "spibsc", + .owner = THIS_MODULE, + .of_match_table = of_spibsc_match, + }, +}; +module_platform_driver(spibsc_driver); + +MODULE_DESCRIPTION("Renesas SPIBSC SPI Flash driver"); +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Chris Brandt");
Add a driver for the SPIBSC controller in Renesas SoC devices. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- drivers/spi/Kconfig | 8 + drivers/spi/Makefile | 1 + drivers/spi/spi-spibsc.c | 609 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 618 insertions(+) create mode 100644 drivers/spi/spi-spibsc.c