| Message ID | 20220802175755.6530-4-sudip.mukherjee@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for enhanced SPI for Designware SPI controllers | expand |
On Tue, Aug 02, 2022 at 06:57:47PM +0100, Sudip Mukherjee wrote: > The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a > configuration variable to keep the mode based on the buswidth, which will > then be used to update CR0. If the transfer is using dual/quad/octal > mode then mark enhanced_spi as true. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > --- > drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++ > drivers/spi/spi-dw.h | 7 +++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index 77529e359b6d..8c84a2e991b5 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, > /* CTRLR0[11:10] Transfer Mode */ > cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode); > > + if (dws->caps & DW_SPI_CAP_EXT_SPI) { Drop this conditional statement. Just always set the spi_frf field. > + if (cfg->spi_frf) Drop this conditional statement. Just always set the spi_frf. It shall be zero if the Enhanced SPI modes are unsupported. > + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK, Since the SPI_FRF field has different offset in the DW APB and DW AHB SSI controllers, you'll need to initialize the mode based on the IP-core ID using the dw_spi_ip_is() macro (see the as it's done for tmode). > + cfg->spi_frf); > + else > + cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK; This is redundant. The initial value never has the SPI_FRF field set (see the dw_spi_setup() and dw_spi_prepare_cr0() methods for details). > + } > + > dw_writel(dws, DW_SPI_CTRLR0, cr0); > > if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD || > @@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi) > static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) > { > struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller); > + bool enhanced_spi = false; > struct dw_spi_cfg cfg; > unsigned long flags; > int ret; > > + if (dws->caps & DW_SPI_CAP_EXT_SPI) { > + switch (op->data.buswidth) { > + case 2: > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI; > + enhanced_spi = true; > + break; > + case 4: > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI; > + enhanced_spi = true; > + break; > + case 8: > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI; > + enhanced_spi = true; > + break; > + default: > + cfg.spi_frf = 0; > + break; > + } > + } > + Please create a separate dw_spi_exec_enh_mem_op() method for the Enhanced SPI communications and parse the data.buswidth field there. Also it would be better to define a new macro DW_SSI_CTRLR0_SPI_FRF_STD_SPI and use it instead of the explicit zero SPI_FRF value. > /* > * Collect the outbound data into a single buffer to speed the > * transmission up at least on the initial stage. > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 71d18e9291a3..b8cc20e0deaa 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -96,6 +96,12 @@ > #define DW_HSSI_CTRLR0_SRL BIT(13) > #define DW_HSSI_CTRLR0_MST BIT(31) > > +/* Bit fields in CTRLR0 for enhanced SPI */ No need in the comment. The macros name is descriptive enough. > +#define DW_HSSI_CTRLR0_SPI_FRF_MASK GENMASK(23, 22) Please also add the next new macros #define DW_PSSI_CTRLR0_SPI_FRF_MASK GENMASK(22, 21) #define DW_SSI_CTRLR0_SPI_FRF_STD_SPI 0x0 and group them together with the rest of the APB SSI CTRL0 macros. > +#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI 0x1 > +#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI 0x2 > +#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI 0x3 Please move these to be defined together with the APB SSI CTRL0.SPI_FRF-related macros. -Sergey > + > /* Bit fields in CTRLR1 */ > #define DW_SPI_NDF_MASK GENMASK(15, 0) > > @@ -136,6 +142,7 @@ struct dw_spi_cfg { > u8 dfs; > u32 ndf; > u32 freq; > + u8 spi_frf; > }; > > struct dw_spi; > -- > 2.30.2 >
On Sat, Aug 27, 2022 at 01:03:22AM +0300, Serge Semin wrote: > On Tue, Aug 02, 2022 at 06:57:47PM +0100, Sudip Mukherjee wrote: > > The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a > > configuration variable to keep the mode based on the buswidth, which will > > then be used to update CR0. If the transfer is using dual/quad/octal > > mode then mark enhanced_spi as true. > > > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > > --- > > drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++ > > drivers/spi/spi-dw.h | 7 +++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > > index 77529e359b6d..8c84a2e991b5 100644 > > --- a/drivers/spi/spi-dw-core.c > > +++ b/drivers/spi/spi-dw-core.c > > @@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, > > /* CTRLR0[11:10] Transfer Mode */ > > cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode); > > > > > + if (dws->caps & DW_SPI_CAP_EXT_SPI) { > > Drop this conditional statement. Just always set the spi_frf field. > > > > + if (cfg->spi_frf) > > Drop this conditional statement. Just always set the spi_frf. It shall > be zero if the Enhanced SPI modes are unsupported. > > > + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK, > > Since the SPI_FRF field has different offset in the DW APB and DW AHB > SSI controllers, you'll need to initialize the mode based on the > IP-core ID using the dw_spi_ip_is() macro (see the as it's done for > tmode). > > > + cfg->spi_frf); > > + else > > > + cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK; > > This is redundant. The initial value never has the SPI_FRF field set > (see the dw_spi_setup() and dw_spi_prepare_cr0() methods for details). > > > + } > > + > > dw_writel(dws, DW_SPI_CTRLR0, cr0); > > > > if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD || > > @@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi) > > static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) > > { > > struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller); > > + bool enhanced_spi = false; > > struct dw_spi_cfg cfg; > > unsigned long flags; > > int ret; > > > > > + if (dws->caps & DW_SPI_CAP_EXT_SPI) { > > + switch (op->data.buswidth) { > > + case 2: > > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI; > > + enhanced_spi = true; > > + break; > > + case 4: > > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI; > > + enhanced_spi = true; > > + break; > > + case 8: > > + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI; > > + enhanced_spi = true; > > + break; > > + default: > > + cfg.spi_frf = 0; > > + break; > > + } > > + } > > + > > Please create a separate dw_spi_exec_enh_mem_op() method for the > Enhanced SPI communications and parse the data.buswidth field there. > Also it would be better to define a new macro > DW_SSI_CTRLR0_SPI_FRF_STD_SPI and use it instead of the explicit zero > SPI_FRF value. > > > /* > > * Collect the outbound data into a single buffer to speed the > > * transmission up at least on the initial stage. > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > > index 71d18e9291a3..b8cc20e0deaa 100644 > > --- a/drivers/spi/spi-dw.h > > +++ b/drivers/spi/spi-dw.h > > @@ -96,6 +96,12 @@ > > #define DW_HSSI_CTRLR0_SRL BIT(13) > > #define DW_HSSI_CTRLR0_MST BIT(31) > > > > > +/* Bit fields in CTRLR0 for enhanced SPI */ > > No need in the comment. The macros name is descriptive enough. > > > +#define DW_HSSI_CTRLR0_SPI_FRF_MASK GENMASK(23, 22) > > Please also add the next new macros > #define DW_PSSI_CTRLR0_SPI_FRF_MASK GENMASK(22, 21) > #define DW_SSI_CTRLR0_SPI_FRF_STD_SPI 0x0 > > and group them together with the rest of the APB SSI CTRL0 macros. > > > +#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI 0x1 > > +#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI 0x2 > > +#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI 0x3 One more thing s/DW_SSI/DW_SPI . All the common macros are defined with the DW_SPI prefix. -Sergey > > Please move these to be defined together with the APB SSI > CTRL0.SPI_FRF-related macros. > > -Sergey > > > + > > /* Bit fields in CTRLR1 */ > > #define DW_SPI_NDF_MASK GENMASK(15, 0) > > > > @@ -136,6 +142,7 @@ struct dw_spi_cfg { > > u8 dfs; > > u32 ndf; > > u32 freq; > > + u8 spi_frf; > > }; > > > > struct dw_spi; > > -- > > 2.30.2 > >
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index 77529e359b6d..8c84a2e991b5 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi, /* CTRLR0[11:10] Transfer Mode */ cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode); + if (dws->caps & DW_SPI_CAP_EXT_SPI) { + if (cfg->spi_frf) + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK, + cfg->spi_frf); + else + cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK; + } + dw_writel(dws, DW_SPI_CTRLR0, cr0); if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD || @@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi) static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) { struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller); + bool enhanced_spi = false; struct dw_spi_cfg cfg; unsigned long flags; int ret; + if (dws->caps & DW_SPI_CAP_EXT_SPI) { + switch (op->data.buswidth) { + case 2: + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI; + enhanced_spi = true; + break; + case 4: + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI; + enhanced_spi = true; + break; + case 8: + cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI; + enhanced_spi = true; + break; + default: + cfg.spi_frf = 0; + break; + } + } + /* * Collect the outbound data into a single buffer to speed the * transmission up at least on the initial stage. diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index 71d18e9291a3..b8cc20e0deaa 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -96,6 +96,12 @@ #define DW_HSSI_CTRLR0_SRL BIT(13) #define DW_HSSI_CTRLR0_MST BIT(31) +/* Bit fields in CTRLR0 for enhanced SPI */ +#define DW_HSSI_CTRLR0_SPI_FRF_MASK GENMASK(23, 22) +#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI 0x1 +#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI 0x2 +#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI 0x3 + /* Bit fields in CTRLR1 */ #define DW_SPI_NDF_MASK GENMASK(15, 0) @@ -136,6 +142,7 @@ struct dw_spi_cfg { u8 dfs; u32 ndf; u32 freq; + u8 spi_frf; }; struct dw_spi;
The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a configuration variable to keep the mode based on the buswidth, which will then be used to update CR0. If the transfer is using dual/quad/octal mode then mark enhanced_spi as true. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> --- drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++ drivers/spi/spi-dw.h | 7 +++++++ 2 files changed, 36 insertions(+)