| Message ID | 20201119120253.390883-2-damien.lemoal@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] spi: dw: Add support for 32-bits max xfer size | expand |
Damien, Thanks for sending the patch. My comments are below. On Thu, Nov 19, 2020 at 09:02:51PM +0900, Damien Le Moal wrote: > Synopsis DesignWare DW_apb_ssi version 3.23 onward defines a 32-bits > maximum transfer size synthesis parameter (SSI_MAX_XFER_SIZE=32) in > addition to the legacy 16-bits configuration (SSI_MAX_XFER_SIZE=16) for > SPI controllers. When SSI_MAX_XFER_SIZE=32, the layout of the ctrlr0 > register changes and RX/TX FIFO words can be up to 32-bits. > > The layout of ctrlr0 change for the 32-bits configuration moves the > data frame format field to bits 16..20 instead of bits 3..0 > when the controller is configured with SSI_MAX_XFER_SIZE=16. > > Introduce the DW SPI capability flag DW_SPI_CAP_DWC_APB_XFER32 to > indicate in that the controller is configured with SSI_MAX_XFER_SIZE=32. > Since the SSI_MAX_XFER_SIZE parameter is a controller synthesis > parameter not accessible through a register, the detection of this > parameter value is done in spi_hw_init() by writing and reading the > ctrlr0 register and testing the value of bits 20..16. > > The DW_SPI_CAP_DWC_APB_XFER32 flag is used in dw_spi_update_config() to > set the data frame size field at the correct position and in > dw_spi_add_host() to extend bits_per_word_mask to 32-bits. > dw_reader() and dw_writer() are also modified to handle 32-bits FIFO > words. > > Suggested-by: Sean Anderson <seanga2@gmail.com> > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > drivers/spi/spi-dw-core.c | 40 ++++++++++++++++++++++++++++++++------- > drivers/spi/spi-dw.h | 8 ++++++++ > 2 files changed, 41 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index 2e50cc0a9291..4c16832b16fd 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -137,14 +137,16 @@ static inline u32 rx_max(struct dw_spi *dws) > static void dw_writer(struct dw_spi *dws) > { > u32 max = tx_max(dws); > - u16 txw = 0; > + u32 txw = 0; > > while (max--) { > if (dws->tx) { > if (dws->n_bytes == 1) > txw = *(u8 *)(dws->tx); > - else > + else if (dws->n_bytes == 2) > txw = *(u16 *)(dws->tx); > + else > + txw = *(u32 *)(dws->tx); > > dws->tx += dws->n_bytes; > } > @@ -156,15 +158,17 @@ static void dw_writer(struct dw_spi *dws) > static void dw_reader(struct dw_spi *dws) > { > u32 max = rx_max(dws); > - u16 rxw; > + u32 rxw; > > while (max--) { > rxw = dw_read_io_reg(dws, DW_SPI_DR); > if (dws->rx) { > if (dws->n_bytes == 1) > *(u8 *)(dws->rx) = rxw; > - else > + else if (dws->n_bytes == 2) > *(u16 *)(dws->rx) = rxw; > + else > + *(u32 *)(dws->rx) = rxw; > > dws->rx += dws->n_bytes; > } > @@ -311,8 +315,12 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, > u32 speed_hz; > u16 clk_div; > > - /* CTRLR0[ 4/3: 0] Data Frame Size */ > - cr0 |= (cfg->dfs - 1); > + if (!(dws->caps & DW_SPI_CAP_DWC_APB_XFER32)) > + /* CTRLR0[ 4/3: 0] Data Frame Size */ > + cr0 |= (cfg->dfs - 1); > + else > + /* CTRLR0[20: 16] Data Frame Size */ > + cr0 |= FIELD_PREP(DWC_APB_CTRLR0_DFS32_MASK, cfg->dfs - 1); Instead of constantly checking the capability flag here I'd suggest to add a new dfs_off member to the private data. Like this: --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -148,6 +148,7 @@ struct dw_spi { unsigned long paddr; int irq; u32 fifo_len; /* depth of the FIFO buffer */ + int dfs_off; /* DFS field offset */ u32 max_mem_freq; /* max mem-ops bus freq */ u32 max_freq; /* max bus freq supported */ Then the only thing you'd need to add here is: - /* CTRLR0[ 4/3: 0] Data Frame Size */ - cr0 |= (cfg->dfs - 1); + /* CTRLR0[ 4/3: 0]/CTRLR0[ 20: 16] Data Frame Size */ + cr0 |= (cfg->dfs - 1) << dws->dfs_off; * Note SPI core guarantees that bits_per_word is within the bits_per_word_mask * initialized in the SPI controller descriptor. > > if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) > /* CTRLR0[ 9:8] Transfer Mode */ > @@ -828,6 +836,21 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws) > dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len); > } > > + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { > + u32 cr0; > + > + /* > + * Detect APB SSI CTRLR0 size by looking at the data > + * frame size field. > + */ > + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); This isn't going to work because you need to disable the SSI controller first. Then please revert the change or cleanup the register and disable the controller back after finishing the detection. > + cr0 = dw_readl(dws, DW_SPI_CTRLR0); > + if (FIELD_GET(DWC_APB_CTRLR0_DFS32_MASK, cr0)) { Instead of reading the DFS_32 field state I'd suggest to read the normal DFS bits as Sean does in the patch: https://patchwork.ozlabs.org/project/uboot/patch/20201016225755.302659-10-seanga2@gmail.com/ That'd be better because we can't guarantee what some older versions of the DW APB SSI controller preserve zeros in the upper part of CTRLR0, but we are sure the DFS field will always have zeros if DW APB SSI is synthesized with SSI_MAX_XFER_SIZE == 32. So instead of having a new DWC_APB_CTRLR0_DFS32_MASK mask declared you'll need to add the macros as following: --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -41,6 +41,7 @@ /* Bit fields in CTRLR0 */ #define SPI_DFS_OFFSET 0 +#define SPI_DFS_MASK GENMASK(3, 0) +#define SPI_DFS32_OFFSET 16 #define SPI_FRF_OFFSET 4 #define SPI_FRF_SPI 0x0 The SPI_DFS32_OFFSET macro will be needed to initialize the dfs_off member if DFS32 capability is detected. > + dev_dbg(dev, "Detected 32-bits max data frame size\n"); > + dws->caps |= DW_SPI_CAP_DWC_APB_XFER32; I suggest to rename the flag to DW_SPI_CAP_DFS32, because it will be set for both DW APB SSI v4 (in fact for v3.23a and newer (see the Sean' patch log), but never mind) and DW SSI controllers. See the next comment for details of implementation. > + } > + } To sum up what I said above the detection procedure would look like this (note it's untested...): + /* + * Detect CTRLR0.DFS field size and offset by testing the lowest bits + * writability. Note DW SSI controller also has the extended DFS, but + * with zero offset. + */ + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { + u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0); + + spi_enable_chip(dws, 0); + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); + cr0 = dw_readl(dws, DW_SPI_CTRLR0); + dw_writel(dws, DW_SPI_CTRLR0, tmp); + spi_enable_chip(dws, 1); + if (!(cr0 & SPI_DFS_MASK)) { + dev_dbg(dev, "Detected 32-bits max data frame size\n"); + dws->caps |= DW_SPI_CAP_DFS32; * dws->dfs_off = SPI_DFS32_OFFSET; + } + } else { + dws->caps |= DW_SPI_CAP_DFS32; + } Note I've added the DW SSI fix here as it was suggested in a comment to your previous patchset revision... > + > /* enable HW fixup for explicit CS deselect for Amazon's alpine chip */ > if (dws->caps & DW_SPI_CAP_CS_OVERRIDE) > dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF); > @@ -864,7 +887,10 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) > > master->use_gpio_descriptors = true; > master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP; > - master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); > + if (dws->caps & DW_SPI_CAP_DWC_APB_XFER32) Please use the DW_SPI_CAP_DFS32 capability flag here. > + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32); > + else > + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); So this part of your fix will work for all types of controllers: - DW APB SSI v3.22a and older - DW APB SSI v3.23a and newer with SSI_MAX_XFER_SIZE == 16 - DW APB SSI v3.23a and newer with SSI_MAX_XFER_SIZE == 32 - DW SSI v1.* > master->bus_num = dws->bus_num; > master->num_chipselect = dws->num_cs; > master->setup = dw_spi_setup; > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index faf40cb66498..cba5c1f8c456 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -9,6 +9,7 @@ > #include <linux/io.h> > #include <linux/scatterlist.h> > #include <linux/spi/spi-mem.h> > +#include <linux/bitfield.h> > > /* Register offsets */ > #define DW_SPI_CTRLR0 0x00 > @@ -72,6 +73,12 @@ > #define DWC_SSI_CTRLR0_FRF_OFFSET 6 > #define DWC_SSI_CTRLR0_DFS_OFFSET 0 > > +/* > + * Data frame size bits mask in CTRLR0 for DWC_apb_ssi v4 when the > + * controller capability supports DW_SPI_CAP_DWC_APB_XFER32. > + */ > +#define DWC_APB_CTRLR0_DFS32_MASK GENMASK(20, 16) This won't be needed if you implement the fix as I suggest. > + > /* > * For Keem Bay, CTRLR0[31] is used to select controller mode. > * 0: SSI is slave > @@ -121,6 +128,7 @@ enum dw_ssi_type { > #define DW_SPI_CAP_CS_OVERRIDE BIT(0) > #define DW_SPI_CAP_KEEMBAY_MST BIT(1) > #define DW_SPI_CAP_DWC_SSI BIT(2) > +#define DW_SPI_CAP_DWC_APB_XFER32 BIT(3) Please replace it with DW_SPI_CAP_DFS32, since the capability flag will be used for the DW APB SSI v3.23a and newer and DW SSI controllers. -Sergey > > /* Slave spi_transfer/spi_mem_op related */ > struct dw_spi_cfg { > -- > 2.28.0 >
On 11/25/20 2:52 PM, Serge Semin wrote: > Damien, > > Thanks for sending the patch. My comments are below. > > On Thu, Nov 19, 2020 at 09:02:51PM +0900, Damien Le Moal wrote: >> Synopsis DesignWare DW_apb_ssi version 3.23 onward defines a 32-bits >> maximum transfer size synthesis parameter (SSI_MAX_XFER_SIZE=32) in >> addition to the legacy 16-bits configuration (SSI_MAX_XFER_SIZE=16) for >> SPI controllers. When SSI_MAX_XFER_SIZE=32, the layout of the ctrlr0 >> register changes and RX/TX FIFO words can be up to 32-bits. >> >> The layout of ctrlr0 change for the 32-bits configuration moves the >> data frame format field to bits 16..20 instead of bits 3..0 >> when the controller is configured with SSI_MAX_XFER_SIZE=16. >> >> Introduce the DW SPI capability flag DW_SPI_CAP_DWC_APB_XFER32 to >> indicate in that the controller is configured with SSI_MAX_XFER_SIZE=32. >> Since the SSI_MAX_XFER_SIZE parameter is a controller synthesis >> parameter not accessible through a register, the detection of this >> parameter value is done in spi_hw_init() by writing and reading the >> ctrlr0 register and testing the value of bits 20..16. >> >> The DW_SPI_CAP_DWC_APB_XFER32 flag is used in dw_spi_update_config() to >> set the data frame size field at the correct position and in >> dw_spi_add_host() to extend bits_per_word_mask to 32-bits. >> dw_reader() and dw_writer() are also modified to handle 32-bits FIFO >> words. >> >> Suggested-by: Sean Anderson <seanga2@gmail.com> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> >> --- >> drivers/spi/spi-dw-core.c | 40 ++++++++++++++++++++++++++++++++------- >> drivers/spi/spi-dw.h | 8 ++++++++ >> 2 files changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c >> index 2e50cc0a9291..4c16832b16fd 100644 >> --- a/drivers/spi/spi-dw-core.c >> +++ b/drivers/spi/spi-dw-core.c >> @@ -137,14 +137,16 @@ static inline u32 rx_max(struct dw_spi *dws) >> static void dw_writer(struct dw_spi *dws) >> { >> u32 max = tx_max(dws); >> - u16 txw = 0; >> + u32 txw = 0; >> >> while (max--) { >> if (dws->tx) { >> if (dws->n_bytes == 1) >> txw = *(u8 *)(dws->tx); >> - else >> + else if (dws->n_bytes == 2) >> txw = *(u16 *)(dws->tx); >> + else >> + txw = *(u32 *)(dws->tx); >> >> dws->tx += dws->n_bytes; >> } >> @@ -156,15 +158,17 @@ static void dw_writer(struct dw_spi *dws) >> static void dw_reader(struct dw_spi *dws) >> { >> u32 max = rx_max(dws); >> - u16 rxw; >> + u32 rxw; >> >> while (max--) { >> rxw = dw_read_io_reg(dws, DW_SPI_DR); >> if (dws->rx) { >> if (dws->n_bytes == 1) >> *(u8 *)(dws->rx) = rxw; >> - else >> + else if (dws->n_bytes == 2) >> *(u16 *)(dws->rx) = rxw; >> + else >> + *(u32 *)(dws->rx) = rxw; >> >> dws->rx += dws->n_bytes; >> } >> @@ -311,8 +315,12 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, >> u32 speed_hz; >> u16 clk_div; >> > >> - /* CTRLR0[ 4/3: 0] Data Frame Size */ >> - cr0 |= (cfg->dfs - 1); >> + if (!(dws->caps & DW_SPI_CAP_DWC_APB_XFER32)) >> + /* CTRLR0[ 4/3: 0] Data Frame Size */ >> + cr0 |= (cfg->dfs - 1); >> + else >> + /* CTRLR0[20: 16] Data Frame Size */ >> + cr0 |= FIELD_PREP(DWC_APB_CTRLR0_DFS32_MASK, cfg->dfs - 1); > > Instead of constantly checking the capability flag here I'd suggest to add > a new dfs_off member to the private data. Like this: > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -148,6 +148,7 @@ struct dw_spi { > unsigned long paddr; > int irq; > u32 fifo_len; /* depth of the FIFO buffer */ > + int dfs_off; /* DFS field offset */ > u32 max_mem_freq; /* max mem-ops bus freq */ > u32 max_freq; /* max bus freq supported */ > > Then the only thing you'd need to add here is: > - /* CTRLR0[ 4/3: 0] Data Frame Size */ > - cr0 |= (cfg->dfs - 1); > + /* CTRLR0[ 4/3: 0]/CTRLR0[ 20: 16] Data Frame Size */ > + cr0 |= (cfg->dfs - 1) << dws->dfs_off; > > * Note SPI core guarantees that bits_per_word is within the bits_per_word_mask > * initialized in the SPI controller descriptor. > >> >> if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) >> /* CTRLR0[ 9:8] Transfer Mode */ >> @@ -828,6 +836,21 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws) >> dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len); >> } >> >> + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { >> + u32 cr0; >> + >> + /* >> + * Detect APB SSI CTRLR0 size by looking at the data >> + * frame size field. >> + */ > >> + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); > > This isn't going to work because you need to disable the SSI controller > first. Then please revert the change or cleanup the register and disable > the controller back after finishing the detection. In U-Boot at least, ctrlr0 is written before every transfer, so there is no need to restore it afterward. Same for enr. I don't know whether this holds true for Linux. --Sean > >> + cr0 = dw_readl(dws, DW_SPI_CTRLR0); > >> + if (FIELD_GET(DWC_APB_CTRLR0_DFS32_MASK, cr0)) { > > Instead of reading the DFS_32 field state I'd suggest to read the normal > DFS bits as Sean does in the patch: > https://patchwork.ozlabs.org/project/uboot/patch/20201016225755.302659-10-seanga2@gmail.com/ > > That'd be better because we can't guarantee what some older versions of > the DW APB SSI controller preserve zeros in the upper part of CTRLR0, but > we are sure the DFS field will always have zeros if DW APB SSI is > synthesized with SSI_MAX_XFER_SIZE == 32. > > So instead of having a new DWC_APB_CTRLR0_DFS32_MASK mask declared you'll > need to add the macros as following: > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -41,6 +41,7 @@ > > /* Bit fields in CTRLR0 */ > #define SPI_DFS_OFFSET 0 > +#define SPI_DFS_MASK GENMASK(3, 0) > +#define SPI_DFS32_OFFSET 16 > > #define SPI_FRF_OFFSET 4 > #define SPI_FRF_SPI 0x0 > > The SPI_DFS32_OFFSET macro will be needed to initialize the dfs_off member > if DFS32 capability is detected. > >> + dev_dbg(dev, "Detected 32-bits max data frame size\n"); > >> + dws->caps |= DW_SPI_CAP_DWC_APB_XFER32; > > I suggest to rename the flag to DW_SPI_CAP_DFS32, because it will be > set for both DW APB SSI v4 (in fact for v3.23a and newer (see the Sean' > patch log), but never mind) and DW SSI controllers. See the next comment > for details of implementation. > >> + } >> + } > > To sum up what I said above the detection procedure would look like this > (note it's untested...): > > + /* > + * Detect CTRLR0.DFS field size and offset by testing the lowest bits > + * writability. Note DW SSI controller also has the extended DFS, but > + * with zero offset. > + */ > + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { > + u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0); > + > + spi_enable_chip(dws, 0); > + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); > + cr0 = dw_readl(dws, DW_SPI_CTRLR0); > + dw_writel(dws, DW_SPI_CTRLR0, tmp); > + spi_enable_chip(dws, 1); > + if (!(cr0 & SPI_DFS_MASK)) { > + dev_dbg(dev, "Detected 32-bits max data frame size\n"); > + dws->caps |= DW_SPI_CAP_DFS32; > * dws->dfs_off = SPI_DFS32_OFFSET; > + } > + } else { > + dws->caps |= DW_SPI_CAP_DFS32; > + } > > Note I've added the DW SSI fix here as it was suggested in a comment to > your previous patchset revision... > >> + >> /* enable HW fixup for explicit CS deselect for Amazon's alpine chip */ >> if (dws->caps & DW_SPI_CAP_CS_OVERRIDE) >> dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF); >> @@ -864,7 +887,10 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) >> >> master->use_gpio_descriptors = true; >> master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP; >> - master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); > >> + if (dws->caps & DW_SPI_CAP_DWC_APB_XFER32) > > Please use the DW_SPI_CAP_DFS32 capability flag here. > >> + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32); >> + else >> + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); > > So this part of your fix will work for all types of controllers: > - DW APB SSI v3.22a and older > - DW APB SSI v3.23a and newer with SSI_MAX_XFER_SIZE == 16 > - DW APB SSI v3.23a and newer with SSI_MAX_XFER_SIZE == 32 > - DW SSI v1.* > >> master->bus_num = dws->bus_num; >> master->num_chipselect = dws->num_cs; >> master->setup = dw_spi_setup; >> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h >> index faf40cb66498..cba5c1f8c456 100644 >> --- a/drivers/spi/spi-dw.h >> +++ b/drivers/spi/spi-dw.h >> @@ -9,6 +9,7 @@ >> #include <linux/io.h> >> #include <linux/scatterlist.h> >> #include <linux/spi/spi-mem.h> >> +#include <linux/bitfield.h> >> >> /* Register offsets */ >> #define DW_SPI_CTRLR0 0x00 >> @@ -72,6 +73,12 @@ >> #define DWC_SSI_CTRLR0_FRF_OFFSET 6 >> #define DWC_SSI_CTRLR0_DFS_OFFSET 0 >> > >> +/* >> + * Data frame size bits mask in CTRLR0 for DWC_apb_ssi v4 when the >> + * controller capability supports DW_SPI_CAP_DWC_APB_XFER32. >> + */ > >> +#define DWC_APB_CTRLR0_DFS32_MASK GENMASK(20, 16) > > This won't be needed if you implement the fix as I suggest. > >> + >> /* >> * For Keem Bay, CTRLR0[31] is used to select controller mode. >> * 0: SSI is slave >> @@ -121,6 +128,7 @@ enum dw_ssi_type { >> #define DW_SPI_CAP_CS_OVERRIDE BIT(0) >> #define DW_SPI_CAP_KEEMBAY_MST BIT(1) >> #define DW_SPI_CAP_DWC_SSI BIT(2) > >> +#define DW_SPI_CAP_DWC_APB_XFER32 BIT(3) > > Please replace it with DW_SPI_CAP_DFS32, since the capability flag > will be used for the DW APB SSI v3.23a and newer and DW SSI controllers. > > -Sergey > >> >> /* Slave spi_transfer/spi_mem_op related */ >> struct dw_spi_cfg { >> -- >> 2.28.0 >>
On 2020/11/26 13:55, Sean Anderson wrote: > On 11/25/20 2:52 PM, Serge Semin wrote: [...] >>> + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { >>> + u32 cr0; >>> + >>> + /* >>> + * Detect APB SSI CTRLR0 size by looking at the data >>> + * frame size field. >>> + */ >> >>> + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); >> >> This isn't going to work because you need to disable the SSI controller >> first. Then please revert the change or cleanup the register and disable >> the controller back after finishing the detection. > > In U-Boot at least, ctrlr0 is written before every transfer, so there is > no need to restore it afterward. Same for enr. I don't know whether this > holds true for Linux. Same in Linux too. The base value for cr0 is prepared and saved in chip->cr0 field when the controller is scanned/reset and that saved base value is used for updating cr0 on every transfer. Serge point here was to have the DFS32 detection not result in the initial cr0 value changing, as some older versions of the DW controller may react to it. The cr0 save/restore in the detection does not matter for the K210 though. Since this is only done on scan/reset and not in the fast path, I think it is OK and definitely safer.
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index 2e50cc0a9291..4c16832b16fd 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -137,14 +137,16 @@ static inline u32 rx_max(struct dw_spi *dws) static void dw_writer(struct dw_spi *dws) { u32 max = tx_max(dws); - u16 txw = 0; + u32 txw = 0; while (max--) { if (dws->tx) { if (dws->n_bytes == 1) txw = *(u8 *)(dws->tx); - else + else if (dws->n_bytes == 2) txw = *(u16 *)(dws->tx); + else + txw = *(u32 *)(dws->tx); dws->tx += dws->n_bytes; } @@ -156,15 +158,17 @@ static void dw_writer(struct dw_spi *dws) static void dw_reader(struct dw_spi *dws) { u32 max = rx_max(dws); - u16 rxw; + u32 rxw; while (max--) { rxw = dw_read_io_reg(dws, DW_SPI_DR); if (dws->rx) { if (dws->n_bytes == 1) *(u8 *)(dws->rx) = rxw; - else + else if (dws->n_bytes == 2) *(u16 *)(dws->rx) = rxw; + else + *(u32 *)(dws->rx) = rxw; dws->rx += dws->n_bytes; } @@ -311,8 +315,12 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, u32 speed_hz; u16 clk_div; - /* CTRLR0[ 4/3: 0] Data Frame Size */ - cr0 |= (cfg->dfs - 1); + if (!(dws->caps & DW_SPI_CAP_DWC_APB_XFER32)) + /* CTRLR0[ 4/3: 0] Data Frame Size */ + cr0 |= (cfg->dfs - 1); + else + /* CTRLR0[20: 16] Data Frame Size */ + cr0 |= FIELD_PREP(DWC_APB_CTRLR0_DFS32_MASK, cfg->dfs - 1); if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) /* CTRLR0[ 9:8] Transfer Mode */ @@ -828,6 +836,21 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws) dev_dbg(dev, "Detected FIFO size: %u bytes\n", dws->fifo_len); } + if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) { + u32 cr0; + + /* + * Detect APB SSI CTRLR0 size by looking at the data + * frame size field. + */ + dw_writel(dws, DW_SPI_CTRLR0, 0xffffffff); + cr0 = dw_readl(dws, DW_SPI_CTRLR0); + if (FIELD_GET(DWC_APB_CTRLR0_DFS32_MASK, cr0)) { + dev_dbg(dev, "Detected 32-bits max data frame size\n"); + dws->caps |= DW_SPI_CAP_DWC_APB_XFER32; + } + } + /* enable HW fixup for explicit CS deselect for Amazon's alpine chip */ if (dws->caps & DW_SPI_CAP_CS_OVERRIDE) dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF); @@ -864,7 +887,10 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) master->use_gpio_descriptors = true; master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP; - master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); + if (dws->caps & DW_SPI_CAP_DWC_APB_XFER32) + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32); + else + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 16); master->bus_num = dws->bus_num; master->num_chipselect = dws->num_cs; master->setup = dw_spi_setup; diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index faf40cb66498..cba5c1f8c456 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -9,6 +9,7 @@ #include <linux/io.h> #include <linux/scatterlist.h> #include <linux/spi/spi-mem.h> +#include <linux/bitfield.h> /* Register offsets */ #define DW_SPI_CTRLR0 0x00 @@ -72,6 +73,12 @@ #define DWC_SSI_CTRLR0_FRF_OFFSET 6 #define DWC_SSI_CTRLR0_DFS_OFFSET 0 +/* + * Data frame size bits mask in CTRLR0 for DWC_apb_ssi v4 when the + * controller capability supports DW_SPI_CAP_DWC_APB_XFER32. + */ +#define DWC_APB_CTRLR0_DFS32_MASK GENMASK(20, 16) + /* * For Keem Bay, CTRLR0[31] is used to select controller mode. * 0: SSI is slave @@ -121,6 +128,7 @@ enum dw_ssi_type { #define DW_SPI_CAP_CS_OVERRIDE BIT(0) #define DW_SPI_CAP_KEEMBAY_MST BIT(1) #define DW_SPI_CAP_DWC_SSI BIT(2) +#define DW_SPI_CAP_DWC_APB_XFER32 BIT(3) /* Slave spi_transfer/spi_mem_op related */ struct dw_spi_cfg {
Synopsis DesignWare DW_apb_ssi version 3.23 onward defines a 32-bits maximum transfer size synthesis parameter (SSI_MAX_XFER_SIZE=32) in addition to the legacy 16-bits configuration (SSI_MAX_XFER_SIZE=16) for SPI controllers. When SSI_MAX_XFER_SIZE=32, the layout of the ctrlr0 register changes and RX/TX FIFO words can be up to 32-bits. The layout of ctrlr0 change for the 32-bits configuration moves the data frame format field to bits 16..20 instead of bits 3..0 when the controller is configured with SSI_MAX_XFER_SIZE=16. Introduce the DW SPI capability flag DW_SPI_CAP_DWC_APB_XFER32 to indicate in that the controller is configured with SSI_MAX_XFER_SIZE=32. Since the SSI_MAX_XFER_SIZE parameter is a controller synthesis parameter not accessible through a register, the detection of this parameter value is done in spi_hw_init() by writing and reading the ctrlr0 register and testing the value of bits 20..16. The DW_SPI_CAP_DWC_APB_XFER32 flag is used in dw_spi_update_config() to set the data frame size field at the correct position and in dw_spi_add_host() to extend bits_per_word_mask to 32-bits. dw_reader() and dw_writer() are also modified to handle 32-bits FIFO words. Suggested-by: Sean Anderson <seanga2@gmail.com> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- drivers/spi/spi-dw-core.c | 40 ++++++++++++++++++++++++++++++++------- drivers/spi/spi-dw.h | 8 ++++++++ 2 files changed, 41 insertions(+), 7 deletions(-)