| Message ID | 20220802175755.6530-2-sudip.mukherjee@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for enhanced SPI for Designware SPI controllers | expand |
On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote: > Some Synopsys SSI controllers support enhanced SPI which includes > Dual mode, Quad mode and Octal mode. Define the capability and mention > it in the controller supported modes. > +#define DW_SPI_CAP_EXT_SPI BIT(2) This isn't at all descriptive, it'd be better to have a capability bit for the various multi-line data modes (or possibly individual bits for them, though board setup will stop us using things that aren't supported in a given design anyway so it's a bit redundant) - that'd be a lot clearer and avoid confusion further down the line when some other feature gets added.
On Tue, Aug 2, 2022 at 7:48 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote: > > > Some Synopsys SSI controllers support enhanced SPI which includes > > Dual mode, Quad mode and Octal mode. Define the capability and mention > > it in the controller supported modes. > > > +#define DW_SPI_CAP_EXT_SPI BIT(2) > > This isn't at all descriptive, it'd be better to have a capability bit > for the various multi-line data modes (or possibly individual bits for > them, though board setup will stop us using things that aren't supported > in a given design anyway so it's a bit redundant) - that'd be a lot > clearer and avoid confusion further down the line when some other > feature gets added. Do you mean to add separate capability bit like: DW_SPI_CAP_DUAL_SPI DW_SPI_CAP_QUAD_SPI and DW_SPI_CAP_OCTAL_SPI ? -- Regards Sudip
On Wed, Aug 03, 2022 at 06:34:53PM +0100, Sudip Mukherjee wrote: > On Tue, Aug 2, 2022 at 7:48 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote: > > > +#define DW_SPI_CAP_EXT_SPI BIT(2) > > This isn't at all descriptive, it'd be better to have a capability bit > > for the various multi-line data modes (or possibly individual bits for > > them, though board setup will stop us using things that aren't supported > > in a given design anyway so it's a bit redundant) - that'd be a lot > > clearer and avoid confusion further down the line when some other > > feature gets added. > Do you mean to add separate capability bit like: > DW_SPI_CAP_DUAL_SPI > DW_SPI_CAP_QUAD_SPI and > DW_SPI_CAP_OCTAL_SPI ? Either that or some rolled together capability with an at least somewhat descriptive name.
On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote: > Some Synopsys SSI controllers support enhanced SPI which includes > Dual mode, Quad mode and Octal mode. Define the capability and mention > it in the controller supported modes. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > --- > drivers/spi/spi-dw-core.c | 4 ++++ > drivers/spi/spi-dw.h | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index f87d97ccd2d6..97e72da7c120 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -917,6 +917,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; > + if (dws->caps & DW_SPI_CAP_EXT_SPI) > + master->mode_bits |= SPI_TX_DUAL | SPI_RX_DUAL | > + SPI_TX_QUAD | SPI_RX_QUAD | > + SPI_TX_OCTAL | SPI_RX_OCTAL; Is it possible to auto-detect the highest supported mode by, for instance, writing to the CTRL0.SPI_FRF field values up to the maximum one? The DW SSI hardware manual says that the unsupported mode combinations are reserved. Could the reserved modes still be written to the SPI_FRF field? If not we could use it to set the SPI_{TX,RX}_DUAL, SPI_{TX,RX}_QUAD, SPI_{TX,RX}_OCTAL in accordance with the actual device capabilities rather than setting all of them. > if (dws->caps & DW_SPI_CAP_DFS32) > master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32); > else > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 9e8eb2b52d5c..71d18e9291a3 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -32,6 +32,7 @@ > /* DW SPI controller capabilities */ > #define DW_SPI_CAP_CS_OVERRIDE BIT(0) > #define DW_SPI_CAP_DFS32 BIT(1) > +#define DW_SPI_CAP_EXT_SPI BIT(2) EXT-prefix is misleading. The feature is called "Enhanced SPI Modes", not Extended SPI modes. Perhaps something like DW_SPI_CAP_EMODE ? -Sergey > > /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */ > #define DW_SPI_CTRLR0 0x00 > -- > 2.30.2 >
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index f87d97ccd2d6..97e72da7c120 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -917,6 +917,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; + if (dws->caps & DW_SPI_CAP_EXT_SPI) + master->mode_bits |= SPI_TX_DUAL | SPI_RX_DUAL | + SPI_TX_QUAD | SPI_RX_QUAD | + SPI_TX_OCTAL | SPI_RX_OCTAL; if (dws->caps & DW_SPI_CAP_DFS32) master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32); else diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index 9e8eb2b52d5c..71d18e9291a3 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -32,6 +32,7 @@ /* DW SPI controller capabilities */ #define DW_SPI_CAP_CS_OVERRIDE BIT(0) #define DW_SPI_CAP_DFS32 BIT(1) +#define DW_SPI_CAP_EXT_SPI BIT(2) /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */ #define DW_SPI_CTRLR0 0x00
Some Synopsys SSI controllers support enhanced SPI which includes Dual mode, Quad mode and Octal mode. Define the capability and mention it in the controller supported modes. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> --- drivers/spi/spi-dw-core.c | 4 ++++ drivers/spi/spi-dw.h | 1 + 2 files changed, 5 insertions(+)