* [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support
@ 2021-11-12 20:49 Serge Semin
2021-11-12 20:49 ` [PATCH 1/4] spi: dw: Discard redundant DW SSI Frame Formats enumeration Serge Semin
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Serge Semin @ 2021-11-12 20:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown, Nandhini Srikandan
Cc: Serge Semin, Andy Shevchenko, Andy Shevchenko, linux-spi, linux-kernel
This patchset consists of the changes which I was going to introduce for a
long time, but due to lack of free time couldn't make it so far.
Nandhini's series [1] made me to proceed with this task so mate would
finally have his patchset accepted and merged into the mainline kernel.
There are three cleanup patches here and one feature patch. In framework
of the former patches we suggest to better organize the code. In
particular they concern the methods and macros naming unification (using a
unified prefixes of the code object names) and CSR fields macro
implementation using the bitfield helpers available in the kernel. The
later patch introduces the Synopsys Component Version register parsing
procedure so the corresponding data could be used for a version-specific
features implementation. Nandhini will be mostly interested in the later
patch in the framework of his series [1].
Nandhini, could you please test the patchset out on your DWC SSI hardware?
After it's merged into the spi/for-next branch of the Mark' repository you
will be able to rebase your series on top of it and use the last patch
functionality for your benefit.
[1] https://lore.kernel.org/linux-spi/20211111065201.10249-4-nandhini.srikandan@intel.com/
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-spi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (4):
spi: dw: Discard redundant DW SSI Frame Formats enumeration
spi: dw: Put the driver entities naming in order
spi: dw: Convert to using the Bitfield access macros
spi: dw: Add Synopsys Component version reading and parsing
drivers/spi/spi-dw-bt1.c | 8 +-
drivers/spi/spi-dw-core.c | 165 ++++++++++++++++++++++----------------
drivers/spi/spi-dw-dma.c | 50 ++++++------
drivers/spi/spi-dw-mmio.c | 20 ++---
drivers/spi/spi-dw-pci.c | 59 +++++++-------
drivers/spi/spi-dw.h | 150 +++++++++++++++++-----------------
6 files changed, 236 insertions(+), 216 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] spi: dw: Discard redundant DW SSI Frame Formats enumeration
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
@ 2021-11-12 20:49 ` Serge Semin
2021-11-12 20:49 ` [PATCH 2/4] spi: dw: Put the driver entities naming in order Serge Semin
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2021-11-12 20:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown, Nandhini Srikandan
Cc: Serge Semin, Andy Shevchenko, Andy Shevchenko, linux-spi, linux-kernel
The dw_ssi_type enumeration describes the SPI frame formats the controller
supports, like Motorola SPI, Texas Instruments SSP and National
Semiconductors Microwire, that is the serial protocol utilized for the
SPI-transfers. Depending on the DW SSI IP-core configuration the protocol
could be either fixed or selectable. If it is changebale the protocol can
be selected by means of the CTRL0.FRF field, which possible values encoded
by the dw_ssi_type enumeration. Aside with the denoted enum the field
values are also described by a set of SPI_FRF_{SPI,SSP,MICROWIRE} macros.
Thus currently the DW SPI driver has got two entities describing the same
data. Let's get rid of the enumeration one then, since first it hasn't
been used as enumeration-type but merely as a parametrized values set and
second that would unify the macro-based CSR read/write interface of the
driver. While at it convert the macro names to be more descriptive about
the protocols they represent.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
drivers/spi/spi-dw-core.c | 4 ++--
drivers/spi/spi-dw.h | 12 +++---------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index a305074c482e..f5446d9c6f27 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -273,7 +273,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
/* CTRLR0[ 5: 4] Frame Format */
- cr0 |= SSI_MOTO_SPI << SPI_FRF_OFFSET;
+ cr0 |= SPI_FRF_MOTO_SPI << SPI_FRF_OFFSET;
/*
* SPI mode (SCPOL|SCPH)
@@ -287,7 +287,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
} else {
/* CTRLR0[ 7: 6] Frame Format */
- cr0 |= SSI_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
+ cr0 |= SPI_FRF_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
/*
* SPI mode (SCPOL|SCPH)
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index b665e040862c..467c342bfe56 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -46,9 +46,9 @@
#define SPI_DFS32_OFFSET 16
#define SPI_FRF_OFFSET 4
-#define SPI_FRF_SPI 0x0
-#define SPI_FRF_SSP 0x1
-#define SPI_FRF_MICROWIRE 0x2
+#define SPI_FRF_MOTO_SPI 0x0
+#define SPI_FRF_TI_SSP 0x1
+#define SPI_FRF_NS_MICROWIRE 0x2
#define SPI_FRF_RESV 0x3
#define SPI_MODE_OFFSET 6
@@ -114,12 +114,6 @@
#define SPI_GET_BYTE(_val, _idx) \
((_val) >> (BITS_PER_BYTE * (_idx)) & 0xff)
-enum dw_ssi_type {
- SSI_MOTO_SPI = 0,
- SSI_TI_SSP,
- SSI_NS_MICROWIRE,
-};
-
/* DW SPI capabilities */
#define DW_SPI_CAP_CS_OVERRIDE BIT(0)
#define DW_SPI_CAP_KEEMBAY_MST BIT(1)
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] spi: dw: Put the driver entities naming in order
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
2021-11-12 20:49 ` [PATCH 1/4] spi: dw: Discard redundant DW SSI Frame Formats enumeration Serge Semin
@ 2021-11-12 20:49 ` Serge Semin
2021-11-12 21:22 ` Andy Shevchenko
2021-11-12 20:49 ` [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros Serge Semin
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2021-11-12 20:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown, Nandhini Srikandan
Cc: Serge Semin, Andy Shevchenko, Andy Shevchenko, linux-spi, linux-kernel
Mostly due to a long driver history it's methods and macro names look a
bit messy. In particularly that concerns the code their prefixes. A
biggest part of the driver functions and macros have got the dw_spi/DW_SPI
prefixes. But there are some entities which have been just
"spi_/SPI_"-prefixed. Especially that concerns the CSR and their fields
macro definitions. It makes the code harder to comprehend since such
methods and macros can be easily confused with the global SPI-subsystem
exports. In this case the only possible way to more or less quickly
distinguish one naming space from another is either by context or by the
argument type, which most of the times isn't that easy anyway. In addition
to that a new DW SSI IP-core support has been added in the framework of
commit e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi"), which
introduced a new set or macro-prefixes to describe CTRLR0-specific fields
and worsen the situation. Finally there are methods with
no DW SPI driver-reference prefix at all, that make the code reading even
harder. So in order to ease the driver hacking let's bring the code naming
to a common base:
1) Each method is supposed to have "dw_spi_" prefix so to be easily
distinguished from the kernel API, e.g. SPI-subsystem methods and macros.
(Exception is the local implementation of the readl/writel methods since
being just the regspace accessors.)
2) Each generically used macro should have DW_SPI_-prefix thus being
easily comprehended as the local driver definition.
3) DW APB SSI and DW SSI specific macros should have prefixes as DW_PSSI_
and DW_HSSI_ respectively so referring to the system buses they support
(APB and AHB similarly to the DT clocks naming like pclk, hclk).
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
Folks, any ideas of a better naming scheme especially for the DW APB SSI
and DW SSI specific macros are very welcome.
---
drivers/spi/spi-dw-bt1.c | 8 +--
drivers/spi/spi-dw-core.c | 138 ++++++++++++++++++------------------
drivers/spi/spi-dw-dma.c | 50 ++++++-------
drivers/spi/spi-dw-mmio.c | 20 +++---
drivers/spi/spi-dw-pci.c | 59 ++++++++--------
drivers/spi/spi-dw.h | 145 +++++++++++++++++++-------------------
6 files changed, 211 insertions(+), 209 deletions(-)
diff --git a/drivers/spi/spi-dw-bt1.c b/drivers/spi/spi-dw-bt1.c
index 5be6b7b80c21..0411088dc443 100644
--- a/drivers/spi/spi-dw-bt1.c
+++ b/drivers/spi/spi-dw-bt1.c
@@ -123,7 +123,7 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
len = min_t(size_t, len, dwsbt1->map_len - offs);
/* Collect the controller configuration required by the operation */
- cfg.tmode = SPI_TMOD_EPROMREAD;
+ cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
cfg.dfs = 8;
cfg.ndf = 4;
cfg.freq = mem->spi->max_speed_hz;
@@ -131,13 +131,13 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
/* Make sure the corresponding CS is de-asserted on transmission */
dw_spi_set_cs(mem->spi, false);
- spi_enable_chip(dws, 0);
+ dw_spi_enable_chip(dws, 0);
dw_spi_update_config(dws, mem->spi, &cfg);
- spi_umask_intr(dws, SPI_INT_RXFI);
+ dw_spi_umask_intr(dws, DW_SPI_INT_RXFI);
- spi_enable_chip(dws, 1);
+ dw_spi_enable_chip(dws, 1);
/*
* Enable the transparent mode of the System Boot Controller.
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index f5446d9c6f27..4d91ffb5c0d8 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -24,7 +24,7 @@
#endif
/* Slave spi_device related */
-struct chip_data {
+struct dw_spi_chip_data {
u32 cr0;
u32 rx_sample_dly; /* RX sample delay */
};
@@ -109,7 +109,7 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
EXPORT_SYMBOL_GPL(dw_spi_set_cs);
/* Return the max entries we can fill into tx fifo */
-static inline u32 tx_max(struct dw_spi *dws)
+static inline u32 dw_spi_tx_max(struct dw_spi *dws)
{
u32 tx_room, rxtx_gap;
@@ -129,14 +129,14 @@ static inline u32 tx_max(struct dw_spi *dws)
}
/* Return the max entries we should read out of rx fifo */
-static inline u32 rx_max(struct dw_spi *dws)
+static inline u32 dw_spi_rx_max(struct dw_spi *dws)
{
return min_t(u32, dws->rx_len, dw_readl(dws, DW_SPI_RXFLR));
}
static void dw_writer(struct dw_spi *dws)
{
- u32 max = tx_max(dws);
+ u32 max = dw_spi_tx_max(dws);
u32 txw = 0;
while (max--) {
@@ -157,7 +157,7 @@ static void dw_writer(struct dw_spi *dws)
static void dw_reader(struct dw_spi *dws)
{
- u32 max = rx_max(dws);
+ u32 max = dw_spi_rx_max(dws);
u32 rxw;
while (max--) {
@@ -186,24 +186,24 @@ int dw_spi_check_status(struct dw_spi *dws, bool raw)
else
irq_status = dw_readl(dws, DW_SPI_ISR);
- if (irq_status & SPI_INT_RXOI) {
+ if (irq_status & DW_SPI_INT_RXOI) {
dev_err(&dws->master->dev, "RX FIFO overflow detected\n");
ret = -EIO;
}
- if (irq_status & SPI_INT_RXUI) {
+ if (irq_status & DW_SPI_INT_RXUI) {
dev_err(&dws->master->dev, "RX FIFO underflow detected\n");
ret = -EIO;
}
- if (irq_status & SPI_INT_TXOI) {
+ if (irq_status & DW_SPI_INT_TXOI) {
dev_err(&dws->master->dev, "TX FIFO overflow detected\n");
ret = -EIO;
}
/* Generically handle the erroneous situation */
if (ret) {
- spi_reset_chip(dws);
+ dw_spi_reset_chip(dws);
if (dws->master->cur_msg)
dws->master->cur_msg->status = ret;
}
@@ -230,7 +230,7 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
*/
dw_reader(dws);
if (!dws->rx_len) {
- spi_mask_intr(dws, 0xff);
+ dw_spi_mask_intr(dws, 0xff);
spi_finalize_current_transfer(dws->master);
} else if (dws->rx_len <= dw_readl(dws, DW_SPI_RXFTLR)) {
dw_writel(dws, DW_SPI_RXFTLR, dws->rx_len - 1);
@@ -241,10 +241,10 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
* disabled after the data transmission is finished so not to
* have the TXE IRQ flood at the final stage of the transfer.
*/
- if (irq_status & SPI_INT_TXEI) {
+ if (irq_status & DW_SPI_INT_TXEI) {
dw_writer(dws);
if (!dws->tx_len)
- spi_mask_intr(dws, SPI_INT_TXEI);
+ dw_spi_mask_intr(dws, DW_SPI_INT_TXEI);
}
return IRQ_HANDLED;
@@ -260,7 +260,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
return IRQ_NONE;
if (!master->cur_msg) {
- spi_mask_intr(dws, 0xff);
+ dw_spi_mask_intr(dws, 0xff);
return IRQ_HANDLED;
}
@@ -271,37 +271,37 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
{
u32 cr0 = 0;
- if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
+ if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
/* CTRLR0[ 5: 4] Frame Format */
- cr0 |= SPI_FRF_MOTO_SPI << SPI_FRF_OFFSET;
+ cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
/*
* SPI mode (SCPOL|SCPH)
* CTRLR0[ 6] Serial Clock Phase
* CTRLR0[ 7] Serial Clock Polarity
*/
- cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << SPI_SCOL_OFFSET;
- cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << SPI_SCPH_OFFSET;
+ cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
+ cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
/* CTRLR0[11] Shift Register Loop */
- cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
+ cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
} else {
/* CTRLR0[ 7: 6] Frame Format */
- cr0 |= SPI_FRF_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
+ cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
/*
* SPI mode (SCPOL|SCPH)
* CTRLR0[ 8] Serial Clock Phase
* CTRLR0[ 9] Serial Clock Polarity
*/
- cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DWC_SSI_CTRLR0_SCPOL_OFFSET;
- cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DWC_SSI_CTRLR0_SCPH_OFFSET;
+ cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
+ cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
/* CTRLR0[13] Shift Register Loop */
- cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DWC_SSI_CTRLR0_SRL_OFFSET;
+ cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
- cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
+ cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
}
return cr0;
@@ -310,7 +310,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
struct dw_spi_cfg *cfg)
{
- struct chip_data *chip = spi_get_ctldata(spi);
+ struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
u32 cr0 = chip->cr0;
u32 speed_hz;
u16 clk_div;
@@ -318,16 +318,17 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
/* CTRLR0[ 4/3: 0] or CTRLR0[ 20: 16] Data Frame Size */
cr0 |= (cfg->dfs - 1) << dws->dfs_offset;
- if (!(dws->caps & DW_SPI_CAP_DWC_SSI))
+ if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
/* CTRLR0[ 9:8] Transfer Mode */
- cr0 |= cfg->tmode << SPI_TMOD_OFFSET;
+ cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
else
/* CTRLR0[11:10] Transfer Mode */
- cr0 |= cfg->tmode << DWC_SSI_CTRLR0_TMOD_OFFSET;
+ cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
dw_writel(dws, DW_SPI_CTRLR0, cr0);
- if (cfg->tmode == SPI_TMOD_EPROMREAD || cfg->tmode == SPI_TMOD_RO)
+ if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
+ cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
/* Note DW APB SSI clock divider doesn't support odd numbers */
@@ -335,7 +336,7 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
speed_hz = dws->max_freq / clk_div;
if (dws->current_freq != speed_hz) {
- spi_set_clk(dws, clk_div);
+ dw_spi_set_clk(dws, clk_div);
dws->current_freq = speed_hz;
}
@@ -363,9 +364,9 @@ static void dw_spi_irq_setup(struct dw_spi *dws)
dws->transfer_handler = dw_spi_transfer_handler;
- imask = SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI |
- SPI_INT_RXFI;
- spi_umask_intr(dws, imask);
+ imask = DW_SPI_INT_TXEI | DW_SPI_INT_TXOI |
+ DW_SPI_INT_RXUI | DW_SPI_INT_RXOI | DW_SPI_INT_RXFI;
+ dw_spi_umask_intr(dws, imask);
}
/*
@@ -405,11 +406,12 @@ static int dw_spi_poll_transfer(struct dw_spi *dws,
}
static int dw_spi_transfer_one(struct spi_controller *master,
- struct spi_device *spi, struct spi_transfer *transfer)
+ struct spi_device *spi,
+ struct spi_transfer *transfer)
{
struct dw_spi *dws = spi_controller_get_devdata(master);
struct dw_spi_cfg cfg = {
- .tmode = SPI_TMOD_TR,
+ .tmode = DW_SPI_CTRLR0_TMOD_TR,
.dfs = transfer->bits_per_word,
.freq = transfer->speed_hz,
};
@@ -425,7 +427,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
/* Ensure the data above is visible for all CPUs */
smp_mb();
- spi_enable_chip(dws, 0);
+ dw_spi_enable_chip(dws, 0);
dw_spi_update_config(dws, spi, &cfg);
@@ -436,7 +438,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
dws->dma_mapped = master->cur_msg_mapped;
/* For poll mode just disable all interrupts */
- spi_mask_intr(dws, 0xff);
+ dw_spi_mask_intr(dws, 0xff);
if (dws->dma_mapped) {
ret = dws->dma_ops->dma_setup(dws, transfer);
@@ -444,7 +446,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
return ret;
}
- spi_enable_chip(dws, 1);
+ dw_spi_enable_chip(dws, 1);
if (dws->dma_mapped)
return dws->dma_ops->dma_transfer(dws, transfer);
@@ -457,20 +459,20 @@ static int dw_spi_transfer_one(struct spi_controller *master,
}
static void dw_spi_handle_err(struct spi_controller *master,
- struct spi_message *msg)
+ struct spi_message *msg)
{
struct dw_spi *dws = spi_controller_get_devdata(master);
if (dws->dma_mapped)
dws->dma_ops->dma_stop(dws);
- spi_reset_chip(dws);
+ dw_spi_reset_chip(dws);
}
static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
{
if (op->data.dir == SPI_MEM_DATA_IN)
- op->data.nbytes = clamp_val(op->data.nbytes, 0, SPI_NDF_MASK + 1);
+ op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);
return 0;
}
@@ -498,7 +500,7 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
if (op->data.dir == SPI_MEM_DATA_OUT)
len += op->data.nbytes;
- if (len <= SPI_BUF_SIZE) {
+ if (len <= DW_SPI_BUF_SIZE) {
out = dws->buf;
} else {
out = kzalloc(len, GFP_KERNEL);
@@ -512,9 +514,9 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
* single buffer in order to speed the data transmission up.
*/
for (i = 0; i < op->cmd.nbytes; ++i)
- out[i] = SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
+ out[i] = DW_SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
for (j = 0; j < op->addr.nbytes; ++i, ++j)
- out[i] = SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
+ out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
for (j = 0; j < op->dummy.nbytes; ++i, ++j)
out[i] = 0x0;
@@ -587,7 +589,7 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
entries = readl_relaxed(dws->regs + DW_SPI_RXFLR);
if (!entries) {
sts = readl_relaxed(dws->regs + DW_SPI_RISR);
- if (sts & SPI_INT_RXOI) {
+ if (sts & DW_SPI_INT_RXOI) {
dev_err(&dws->master->dev, "FIFO overflow on Rx\n");
return -EIO;
}
@@ -603,12 +605,12 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
static inline bool dw_spi_ctlr_busy(struct dw_spi *dws)
{
- return dw_readl(dws, DW_SPI_SR) & SR_BUSY;
+ return dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_BUSY;
}
static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
{
- int retry = SPI_WAIT_RETRIES;
+ int retry = DW_SPI_WAIT_RETRIES;
struct spi_delay delay;
unsigned long ns, us;
u32 nents;
@@ -638,9 +640,9 @@ static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
{
- spi_enable_chip(dws, 0);
+ dw_spi_enable_chip(dws, 0);
dw_spi_set_cs(spi, true);
- spi_enable_chip(dws, 1);
+ dw_spi_enable_chip(dws, 1);
}
/*
@@ -673,19 +675,19 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
cfg.dfs = 8;
cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
if (op->data.dir == SPI_MEM_DATA_IN) {
- cfg.tmode = SPI_TMOD_EPROMREAD;
+ cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
cfg.ndf = op->data.nbytes;
} else {
- cfg.tmode = SPI_TMOD_TO;
+ cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
}
- spi_enable_chip(dws, 0);
+ dw_spi_enable_chip(dws, 0);
dw_spi_update_config(dws, mem->spi, &cfg);
- spi_mask_intr(dws, 0xff);
+ dw_spi_mask_intr(dws, 0xff);
- spi_enable_chip(dws, 1);
+ dw_spi_enable_chip(dws, 1);
/*
* DW APB SSI controller has very nasty peculiarities. First originally
@@ -768,7 +770,7 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
static int dw_spi_setup(struct spi_device *spi)
{
struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
- struct chip_data *chip;
+ struct dw_spi_chip_data *chip;
/* Only alloc on first setup */
chip = spi_get_ctldata(spi);
@@ -776,7 +778,7 @@ static int dw_spi_setup(struct spi_device *spi)
struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
u32 rx_sample_dly_ns;
- chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
+ chip = kzalloc(sizeof(*chip), GFP_KERNEL);
if (!chip)
return -ENOMEM;
spi_set_ctldata(spi, chip);
@@ -803,16 +805,16 @@ static int dw_spi_setup(struct spi_device *spi)
static void dw_spi_cleanup(struct spi_device *spi)
{
- struct chip_data *chip = spi_get_ctldata(spi);
+ struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
kfree(chip);
spi_set_ctldata(spi, NULL);
}
/* Restart the controller, disable all interrupts, clean rx fifo */
-static void spi_hw_init(struct device *dev, struct dw_spi *dws)
+static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
{
- spi_reset_chip(dws);
+ dw_spi_reset_chip(dws);
/*
* Try to detect the FIFO depth if not set by interface driver,
@@ -837,18 +839,18 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
* writability. Note DWC SSI controller also has the extended DFS, but
* with zero offset.
*/
- if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
+ if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0);
- spi_enable_chip(dws, 0);
+ dw_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);
+ dw_spi_enable_chip(dws, 1);
- if (!(cr0 & SPI_DFS_MASK)) {
+ if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
dws->caps |= DW_SPI_CAP_DFS32;
- dws->dfs_offset = SPI_DFS32_OFFSET;
+ dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
dev_dbg(dev, "Detected 32-bits max data frame size\n");
}
} else {
@@ -878,7 +880,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
spi_controller_set_devdata(master, dws);
/* Basic HW init */
- spi_hw_init(dev, dws);
+ dw_spi_hw_init(dev, dws);
ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
master);
@@ -939,7 +941,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
err_dma_exit:
if (dws->dma_ops && dws->dma_ops->dma_exit)
dws->dma_ops->dma_exit(dws);
- spi_enable_chip(dws, 0);
+ dw_spi_enable_chip(dws, 0);
free_irq(dws->irq, master);
err_free_master:
spi_controller_put(master);
@@ -956,7 +958,7 @@ void dw_spi_remove_host(struct dw_spi *dws)
if (dws->dma_ops && dws->dma_ops->dma_exit)
dws->dma_ops->dma_exit(dws);
- spi_shutdown_chip(dws);
+ dw_spi_shutdown_chip(dws);
free_irq(dws->irq, dws->master);
}
@@ -970,14 +972,14 @@ int dw_spi_suspend_host(struct dw_spi *dws)
if (ret)
return ret;
- spi_shutdown_chip(dws);
+ dw_spi_shutdown_chip(dws);
return 0;
}
EXPORT_SYMBOL_GPL(dw_spi_suspend_host);
int dw_spi_resume_host(struct dw_spi *dws)
{
- spi_hw_init(&dws->master->dev, dws);
+ dw_spi_hw_init(&dws->master->dev, dws);
return spi_controller_resume(dws->master);
}
EXPORT_SYMBOL_GPL(dw_spi_resume_host);
diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
index a09831c62192..fd6a2154d2ce 100644
--- a/drivers/spi/spi-dw-dma.c
+++ b/drivers/spi/spi-dw-dma.c
@@ -17,10 +17,10 @@
#include "spi-dw.h"
-#define RX_BUSY 0
-#define RX_BURST_LEVEL 16
-#define TX_BUSY 1
-#define TX_BURST_LEVEL 16
+#define DW_SPI_RX_BUSY 0
+#define DW_SPI_RX_BURST_LEVEL 16
+#define DW_SPI_TX_BUSY 1
+#define DW_SPI_TX_BURST_LEVEL 16
static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
{
@@ -45,7 +45,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
if (!ret && caps.max_burst)
max_burst = caps.max_burst;
else
- max_burst = RX_BURST_LEVEL;
+ max_burst = DW_SPI_RX_BURST_LEVEL;
dws->rxburst = min(max_burst, def_burst);
dw_writel(dws, DW_SPI_DMARDLR, dws->rxburst - 1);
@@ -54,7 +54,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
if (!ret && caps.max_burst)
max_burst = caps.max_burst;
else
- max_burst = TX_BURST_LEVEL;
+ max_burst = DW_SPI_TX_BURST_LEVEL;
/*
* Having a Rx DMA channel serviced with higher priority than a Tx DMA
@@ -226,13 +226,13 @@ static int dw_spi_dma_wait(struct dw_spi *dws, unsigned int len, u32 speed)
static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
{
- return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
+ return !(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_TF_EMPT);
}
static int dw_spi_dma_wait_tx_done(struct dw_spi *dws,
struct spi_transfer *xfer)
{
- int retry = SPI_WAIT_RETRIES;
+ int retry = DW_SPI_WAIT_RETRIES;
struct spi_delay delay;
u32 nents;
@@ -259,8 +259,8 @@ static void dw_spi_dma_tx_done(void *arg)
{
struct dw_spi *dws = arg;
- clear_bit(TX_BUSY, &dws->dma_chan_busy);
- if (test_bit(RX_BUSY, &dws->dma_chan_busy))
+ clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
+ if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy))
return;
complete(&dws->dma_completion);
@@ -304,19 +304,19 @@ static int dw_spi_dma_submit_tx(struct dw_spi *dws, struct scatterlist *sgl,
return ret;
}
- set_bit(TX_BUSY, &dws->dma_chan_busy);
+ set_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
return 0;
}
static inline bool dw_spi_dma_rx_busy(struct dw_spi *dws)
{
- return !!(dw_readl(dws, DW_SPI_SR) & SR_RF_NOT_EMPT);
+ return !!(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_RF_NOT_EMPT);
}
static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
{
- int retry = SPI_WAIT_RETRIES;
+ int retry = DW_SPI_WAIT_RETRIES;
struct spi_delay delay;
unsigned long ns, us;
u32 nents;
@@ -360,8 +360,8 @@ static void dw_spi_dma_rx_done(void *arg)
{
struct dw_spi *dws = arg;
- clear_bit(RX_BUSY, &dws->dma_chan_busy);
- if (test_bit(TX_BUSY, &dws->dma_chan_busy))
+ clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
+ if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy))
return;
complete(&dws->dma_completion);
@@ -405,7 +405,7 @@ static int dw_spi_dma_submit_rx(struct dw_spi *dws, struct scatterlist *sgl,
return ret;
}
- set_bit(RX_BUSY, &dws->dma_chan_busy);
+ set_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
return 0;
}
@@ -430,16 +430,16 @@ static int dw_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
}
/* Set the DMA handshaking interface */
- dma_ctrl = SPI_DMA_TDMAE;
+ dma_ctrl = DW_SPI_DMACR_TDMAE;
if (xfer->rx_buf)
- dma_ctrl |= SPI_DMA_RDMAE;
+ dma_ctrl |= DW_SPI_DMACR_RDMAE;
dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
/* Set the interrupt mask */
- imr = SPI_INT_TXOI;
+ imr = DW_SPI_INT_TXOI;
if (xfer->rx_buf)
- imr |= SPI_INT_RXUI | SPI_INT_RXOI;
- spi_umask_intr(dws, imr);
+ imr |= DW_SPI_INT_RXUI | DW_SPI_INT_RXOI;
+ dw_spi_umask_intr(dws, imr);
reinit_completion(&dws->dma_completion);
@@ -615,13 +615,13 @@ static int dw_spi_dma_transfer(struct dw_spi *dws, struct spi_transfer *xfer)
static void dw_spi_dma_stop(struct dw_spi *dws)
{
- if (test_bit(TX_BUSY, &dws->dma_chan_busy)) {
+ if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy)) {
dmaengine_terminate_sync(dws->txchan);
- clear_bit(TX_BUSY, &dws->dma_chan_busy);
+ clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
}
- if (test_bit(RX_BUSY, &dws->dma_chan_busy)) {
+ if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy)) {
dmaengine_terminate_sync(dws->rxchan);
- clear_bit(RX_BUSY, &dws->dma_chan_busy);
+ clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
}
}
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 17c06039a74d..435c91aecbca 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -196,18 +196,18 @@ static int dw_spi_alpine_init(struct platform_device *pdev,
return 0;
}
-static int dw_spi_dw_apb_init(struct platform_device *pdev,
- struct dw_spi_mmio *dwsmmio)
+static int dw_spi_assi_init(struct platform_device *pdev,
+ struct dw_spi_mmio *dwsmmio)
{
dw_spi_dma_setup_generic(&dwsmmio->dws);
return 0;
}
-static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
- struct dw_spi_mmio *dwsmmio)
+static int dw_spi_hssi_init(struct platform_device *pdev,
+ struct dw_spi_mmio *dwsmmio)
{
- dwsmmio->dws.caps = DW_SPI_CAP_DWC_SSI;
+ dwsmmio->dws.caps = DW_SPI_CAP_DWC_HSSI;
dw_spi_dma_setup_generic(&dwsmmio->dws);
@@ -217,7 +217,7 @@ static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
static int dw_spi_keembay_init(struct platform_device *pdev,
struct dw_spi_mmio *dwsmmio)
{
- dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_SSI;
+ dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_HSSI;
return 0;
}
@@ -342,12 +342,12 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
}
static const struct of_device_id dw_spi_mmio_of_match[] = {
- { .compatible = "snps,dw-apb-ssi", .data = dw_spi_dw_apb_init},
+ { .compatible = "snps,dw-apb-ssi", .data = dw_spi_assi_init},
{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
- { .compatible = "renesas,rzn1-spi", .data = dw_spi_dw_apb_init},
- { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
+ { .compatible = "renesas,rzn1-spi", .data = dw_spi_assi_init},
+ { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_hssi_init},
{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
@@ -357,7 +357,7 @@ MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
#ifdef CONFIG_ACPI
static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
- {"HISI0173", (kernel_ulong_t)dw_spi_dw_apb_init},
+ {"HISI0173", (kernel_ulong_t)dw_spi_assi_init},
{},
};
MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);
diff --git a/drivers/spi/spi-dw-pci.c b/drivers/spi/spi-dw-pci.c
index 8a91cd58102f..e4a239bc3d36 100644
--- a/drivers/spi/spi-dw-pci.c
+++ b/drivers/spi/spi-dw-pci.c
@@ -24,14 +24,14 @@
#define CLK_SPI_CDIV_MASK 0x00000e00
#define CLK_SPI_DISABLE_OFFSET 8
-struct spi_pci_desc {
+struct dw_spi_pci_desc {
int (*setup)(struct dw_spi *);
u16 num_cs;
u16 bus_num;
u32 max_freq;
};
-static int spi_mid_init(struct dw_spi *dws)
+static int dw_spi_pci_mid_init(struct dw_spi *dws)
{
void __iomem *clk_reg;
u32 clk_cdiv;
@@ -53,36 +53,36 @@ static int spi_mid_init(struct dw_spi *dws)
return 0;
}
-static int spi_generic_init(struct dw_spi *dws)
+static int dw_spi_pci_generic_init(struct dw_spi *dws)
{
dw_spi_dma_setup_generic(dws);
return 0;
}
-static struct spi_pci_desc spi_pci_mid_desc_1 = {
- .setup = spi_mid_init,
+static struct dw_spi_pci_desc dw_spi_pci_mid_desc_1 = {
+ .setup = dw_spi_pci_mid_init,
.num_cs = 5,
.bus_num = 0,
};
-static struct spi_pci_desc spi_pci_mid_desc_2 = {
- .setup = spi_mid_init,
+static struct dw_spi_pci_desc dw_spi_pci_mid_desc_2 = {
+ .setup = dw_spi_pci_mid_init,
.num_cs = 2,
.bus_num = 1,
};
-static struct spi_pci_desc spi_pci_ehl_desc = {
- .setup = spi_generic_init,
+static struct dw_spi_pci_desc dw_spi_pci_ehl_desc = {
+ .setup = dw_spi_pci_generic_init,
.num_cs = 2,
.bus_num = -1,
.max_freq = 100000000,
};
-static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+static int dw_spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
+ struct dw_spi_pci_desc *desc = (struct dw_spi_pci_desc *)ent->driver_data;
struct dw_spi *dws;
- struct spi_pci_desc *desc = (struct spi_pci_desc *)ent->driver_data;
int pci_bar = 0;
int ret;
@@ -150,7 +150,7 @@ static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return ret;
}
-static void spi_pci_remove(struct pci_dev *pdev)
+static void dw_spi_pci_remove(struct pci_dev *pdev)
{
struct dw_spi *dws = pci_get_drvdata(pdev);
@@ -162,14 +162,14 @@ static void spi_pci_remove(struct pci_dev *pdev)
}
#ifdef CONFIG_PM_SLEEP
-static int spi_suspend(struct device *dev)
+static int dw_spi_pci_suspend(struct device *dev)
{
struct dw_spi *dws = dev_get_drvdata(dev);
return dw_spi_suspend_host(dws);
}
-static int spi_resume(struct device *dev)
+static int dw_spi_pci_resume(struct device *dev)
{
struct dw_spi *dws = dev_get_drvdata(dev);
@@ -177,38 +177,37 @@ static int spi_resume(struct device *dev)
}
#endif
-static SIMPLE_DEV_PM_OPS(dw_spi_pm_ops, spi_suspend, spi_resume);
+static SIMPLE_DEV_PM_OPS(dw_spi_pci_pm_ops, dw_spi_pci_suspend, dw_spi_pci_resume);
-static const struct pci_device_id pci_ids[] = {
+static const struct pci_device_id dw_spi_pci_ids[] = {
/* Intel MID platform SPI controller 0 */
/*
* The access to the device 8086:0801 is disabled by HW, since it's
* exclusively used by SCU to communicate with MSIC.
*/
/* Intel MID platform SPI controller 1 */
- { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&spi_pci_mid_desc_1},
+ { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&dw_spi_pci_mid_desc_1},
/* Intel MID platform SPI controller 2 */
- { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&spi_pci_mid_desc_2},
+ { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&dw_spi_pci_mid_desc_2},
/* Intel Elkhart Lake PSE SPI controllers */
- { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&spi_pci_ehl_desc},
- { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&spi_pci_ehl_desc},
- { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&spi_pci_ehl_desc},
- { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&spi_pci_ehl_desc},
+ { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
+ { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
+ { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
+ { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
{},
};
-MODULE_DEVICE_TABLE(pci, pci_ids);
+MODULE_DEVICE_TABLE(pci, dw_spi_pci_ids);
-static struct pci_driver dw_spi_driver = {
+static struct pci_driver dw_spi_pci_driver = {
.name = DRIVER_NAME,
- .id_table = pci_ids,
- .probe = spi_pci_probe,
- .remove = spi_pci_remove,
+ .id_table = dw_spi_pci_ids,
+ .probe = dw_spi_pci_probe,
+ .remove = dw_spi_pci_remove,
.driver = {
- .pm = &dw_spi_pm_ops,
+ .pm = &dw_spi_pci_pm_ops,
},
};
-
-module_pci_driver(dw_spi_driver);
+module_pci_driver(dw_spi_pci_driver);
MODULE_AUTHOR("Feng Tang <feng.tang@intel.com>");
MODULE_DESCRIPTION("PCI interface driver for DW SPI Core");
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 467c342bfe56..893b78c43a50 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef DW_SPI_HEADER_H
-#define DW_SPI_HEADER_H
+#ifndef __SPI_DW_H__
+#define __SPI_DW_H__
#include <linux/bits.h>
#include <linux/completion.h>
@@ -11,7 +11,7 @@
#include <linux/spi/spi-mem.h>
#include <linux/bitfield.h>
-/* Register offsets */
+/* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
#define DW_SPI_CTRLR0 0x00
#define DW_SPI_CTRLR1 0x04
#define DW_SPI_SSIENR 0x08
@@ -40,84 +40,85 @@
#define DW_SPI_RX_SAMPLE_DLY 0xf0
#define DW_SPI_CS_OVERRIDE 0xf4
-/* 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_MOTO_SPI 0x0
-#define SPI_FRF_TI_SSP 0x1
-#define SPI_FRF_NS_MICROWIRE 0x2
-#define SPI_FRF_RESV 0x3
-
-#define SPI_MODE_OFFSET 6
-#define SPI_SCPH_OFFSET 6
-#define SPI_SCOL_OFFSET 7
-
-#define SPI_TMOD_OFFSET 8
-#define SPI_TMOD_MASK (0x3 << SPI_TMOD_OFFSET)
-#define SPI_TMOD_TR 0x0 /* xmit & recv */
-#define SPI_TMOD_TO 0x1 /* xmit only */
-#define SPI_TMOD_RO 0x2 /* recv only */
-#define SPI_TMOD_EPROMREAD 0x3 /* eeprom read mode */
-
-#define SPI_SLVOE_OFFSET 10
-#define SPI_SRL_OFFSET 11
-#define SPI_CFS_OFFSET 12
-
-/* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
-#define DWC_SSI_CTRLR0_SRL_OFFSET 13
-#define DWC_SSI_CTRLR0_TMOD_OFFSET 10
-#define DWC_SSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
-#define DWC_SSI_CTRLR0_SCPOL_OFFSET 9
-#define DWC_SSI_CTRLR0_SCPH_OFFSET 8
-#define DWC_SSI_CTRLR0_FRF_OFFSET 6
-#define DWC_SSI_CTRLR0_DFS_OFFSET 0
+/* Bit fields in CTRLR0 (DWC APB SSI) */
+#define DW_PSSI_CTRLR0_DFS_OFFSET 0
+#define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
+#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
+
+#define DW_PSSI_CTRLR0_FRF_OFFSET 4
+#define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
+#define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
+#define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
+#define DW_SPI_CTRLR0_FRF_RESV 0x3
+
+#define DW_PSSI_CTRLR0_MODE_OFFSET 6
+#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
+#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
+
+#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
+#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
+#define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
+#define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
+#define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
+#define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
+
+#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
+#define DW_PSSI_CTRLR0_SRL_OFFSET 11
+#define DW_PSSI_CTRLR0_CFS_OFFSET 12
+
+/* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
+#define DW_HSSI_CTRLR0_SRL_OFFSET 13
+#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
+#define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
+#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
+#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
+#define DW_HSSI_CTRLR0_FRF_OFFSET 6
+#define DW_HSSI_CTRLR0_DFS_OFFSET 0
/*
* For Keem Bay, CTRLR0[31] is used to select controller mode.
* 0: SSI is slave
* 1: SSI is master
*/
-#define DWC_SSI_CTRLR0_KEEMBAY_MST BIT(31)
+#define DW_HSSI_CTRLR0_KEEMBAY_MST BIT(31)
/* Bit fields in CTRLR1 */
-#define SPI_NDF_MASK GENMASK(15, 0)
+#define DW_SPI_NDF_MASK GENMASK(15, 0)
/* Bit fields in SR, 7 bits */
-#define SR_MASK 0x7f /* cover 7 bits */
-#define SR_BUSY (1 << 0)
-#define SR_TF_NOT_FULL (1 << 1)
-#define SR_TF_EMPT (1 << 2)
-#define SR_RF_NOT_EMPT (1 << 3)
-#define SR_RF_FULL (1 << 4)
-#define SR_TX_ERR (1 << 5)
-#define SR_DCOL (1 << 6)
+#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
+#define DW_SPI_SR_BUSY (1 << 0)
+#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
+#define DW_SPI_SR_TF_EMPT (1 << 2)
+#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
+#define DW_SPI_SR_RF_FULL (1 << 4)
+#define DW_SPI_SR_TX_ERR (1 << 5)
+#define DW_SPI_SR_DCOL (1 << 6)
/* Bit fields in ISR, IMR, RISR, 7 bits */
-#define SPI_INT_TXEI (1 << 0)
-#define SPI_INT_TXOI (1 << 1)
-#define SPI_INT_RXUI (1 << 2)
-#define SPI_INT_RXOI (1 << 3)
-#define SPI_INT_RXFI (1 << 4)
-#define SPI_INT_MSTI (1 << 5)
+#define DW_SPI_INT_TXEI (1 << 0)
+#define DW_SPI_INT_TXOI (1 << 1)
+#define DW_SPI_INT_RXUI (1 << 2)
+#define DW_SPI_INT_RXOI (1 << 3)
+#define DW_SPI_INT_RXFI (1 << 4)
+#define DW_SPI_INT_MSTI (1 << 5)
/* Bit fields in DMACR */
-#define SPI_DMA_RDMAE (1 << 0)
-#define SPI_DMA_TDMAE (1 << 1)
+#define DW_SPI_DMACR_RDMAE (1 << 0)
+#define DW_SPI_DMACR_TDMAE (1 << 1)
-#define SPI_WAIT_RETRIES 5
-#define SPI_BUF_SIZE \
+/* Mem/DMA operations helpers */
+#define DW_SPI_WAIT_RETRIES 5
+#define DW_SPI_BUF_SIZE \
(sizeof_field(struct spi_mem_op, cmd.opcode) + \
sizeof_field(struct spi_mem_op, addr.val) + 256)
-#define SPI_GET_BYTE(_val, _idx) \
+#define DW_SPI_GET_BYTE(_val, _idx) \
((_val) >> (BITS_PER_BYTE * (_idx)) & 0xff)
/* DW SPI capabilities */
#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_HSSI BIT(2)
#define DW_SPI_CAP_DFS32 BIT(3)
/* Slave spi_transfer/spi_mem_op related */
@@ -162,7 +163,7 @@ struct dw_spi {
unsigned int tx_len;
void *rx;
unsigned int rx_len;
- u8 buf[SPI_BUF_SIZE];
+ u8 buf[DW_SPI_BUF_SIZE];
int dma_mapped;
u8 n_bytes; /* current is a 1/2 bytes op */
irqreturn_t (*transfer_handler)(struct dw_spi *dws);
@@ -224,18 +225,18 @@ static inline void dw_write_io_reg(struct dw_spi *dws, u32 offset, u32 val)
}
}
-static inline void spi_enable_chip(struct dw_spi *dws, int enable)
+static inline void dw_spi_enable_chip(struct dw_spi *dws, int enable)
{
dw_writel(dws, DW_SPI_SSIENR, (enable ? 1 : 0));
}
-static inline void spi_set_clk(struct dw_spi *dws, u16 div)
+static inline void dw_spi_set_clk(struct dw_spi *dws, u16 div)
{
dw_writel(dws, DW_SPI_BAUDR, div);
}
/* Disable IRQ bits */
-static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
+static inline void dw_spi_mask_intr(struct dw_spi *dws, u32 mask)
{
u32 new_mask;
@@ -244,7 +245,7 @@ static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
}
/* Enable IRQ bits */
-static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
+static inline void dw_spi_umask_intr(struct dw_spi *dws, u32 mask)
{
u32 new_mask;
@@ -257,19 +258,19 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
* and CS, then re-enables the controller back. Transmit and receive FIFO
* buffers are cleared when the device is disabled.
*/
-static inline void spi_reset_chip(struct dw_spi *dws)
+static inline void dw_spi_reset_chip(struct dw_spi *dws)
{
- spi_enable_chip(dws, 0);
- spi_mask_intr(dws, 0xff);
+ dw_spi_enable_chip(dws, 0);
+ dw_spi_mask_intr(dws, 0xff);
dw_readl(dws, DW_SPI_ICR);
dw_writel(dws, DW_SPI_SER, 0);
- spi_enable_chip(dws, 1);
+ dw_spi_enable_chip(dws, 1);
}
-static inline void spi_shutdown_chip(struct dw_spi *dws)
+static inline void dw_spi_shutdown_chip(struct dw_spi *dws)
{
- spi_enable_chip(dws, 0);
- spi_set_clk(dws, 0);
+ dw_spi_enable_chip(dws, 0);
+ dw_spi_set_clk(dws, 0);
}
extern void dw_spi_set_cs(struct spi_device *spi, bool enable);
@@ -293,4 +294,4 @@ static inline void dw_spi_dma_setup_generic(struct dw_spi *dws) {}
#endif /* !CONFIG_SPI_DW_DMA */
-#endif /* DW_SPI_HEADER_H */
+#endif /* __SPI_DW_H__ */
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
2021-11-12 20:49 ` [PATCH 1/4] spi: dw: Discard redundant DW SSI Frame Formats enumeration Serge Semin
2021-11-12 20:49 ` [PATCH 2/4] spi: dw: Put the driver entities naming in order Serge Semin
@ 2021-11-12 20:49 ` Serge Semin
2021-11-12 21:23 ` Andy Shevchenko
2021-11-12 20:49 ` [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing Serge Semin
2021-11-12 21:30 ` [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Andy Shevchenko
4 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2021-11-12 20:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown, Nandhini Srikandan
Cc: Serge Semin, Andy Shevchenko, Andy Shevchenko, linux-spi, linux-kernel
The driver has been using the offset/bitwise-shift-based approach for the
CSR fields R/W operations since it was merged into the kernel. It can be
simplified by using the macros defined in the linux/bitfield.h and
linux/bit.h header files like BIT(), GENMASK(), FIELD_PREP(), FIELD_GET(),
etc where it is required, for instance in the cached cr0 preparation
method. Thus in order to have the FIELD_*()-macros utilized we just need
to convert the macros with the CSR-fields offsets to the masks with the
corresponding registers fields definition. That's where the GENMASK() and
BIT() macros come in handy. After that the masks can be used in the
FIELD_*()-macros where it's appropriate.
We also need to convert the macros with the CRS-bit flags using the manual
bitwise shift operations (x << y) to using the BIT() macro. Thus we'll
have a more coherent set of the CSR-related macros.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
drivers/spi/spi-dw-core.c | 31 +++++++++++--------
drivers/spi/spi-dw.h | 64 +++++++++++++++++++--------------------
2 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 4d91ffb5c0d8..b4cbcd38eaba 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -5,6 +5,7 @@
* Copyright (c) 2009, Intel Corporation.
*/
+#include <linux/bitfield.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
#include <linux/module.h>
@@ -254,7 +255,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
{
struct spi_controller *master = dev_id;
struct dw_spi *dws = spi_controller_get_devdata(master);
- u16 irq_status = dw_readl(dws, DW_SPI_ISR) & 0x3f;
+ u16 irq_status = dw_readl(dws, DW_SPI_ISR) & DW_SPI_INT_MASK;
if (!irq_status)
return IRQ_NONE;
@@ -273,32 +274,38 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
/* CTRLR0[ 5: 4] Frame Format */
- cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
+ cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
/*
* SPI mode (SCPOL|SCPH)
* CTRLR0[ 6] Serial Clock Phase
* CTRLR0[ 7] Serial Clock Polarity
*/
- cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
- cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
+ if (spi->mode & SPI_CPOL)
+ cr0 |= DW_PSSI_CTRLR0_SCPOL;
+ if (spi->mode & SPI_CPHA)
+ cr0 |= DW_PSSI_CTRLR0_SCPHA;
/* CTRLR0[11] Shift Register Loop */
- cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
+ if (spi->mode & SPI_LOOP)
+ cr0 |= DW_PSSI_CTRLR0_SRL;
} else {
/* CTRLR0[ 7: 6] Frame Format */
- cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
+ cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
/*
* SPI mode (SCPOL|SCPH)
* CTRLR0[ 8] Serial Clock Phase
* CTRLR0[ 9] Serial Clock Polarity
*/
- cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
- cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
+ if (spi->mode & SPI_CPOL)
+ cr0 |= DW_HSSI_CTRLR0_SCPOL;
+ if (spi->mode & SPI_CPHA)
+ cr0 |= DW_HSSI_CTRLR0_SCPHA;
/* CTRLR0[13] Shift Register Loop */
- cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
+ if (spi->mode & SPI_LOOP)
+ cr0 |= DW_HSSI_CTRLR0_SRL;
if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
@@ -320,10 +327,10 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
/* CTRLR0[ 9:8] Transfer Mode */
- cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
+ cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_TMOD_MASK, cfg->tmode);
else
/* CTRLR0[11:10] Transfer Mode */
- cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
+ cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
dw_writel(dws, DW_SPI_CTRLR0, cr0);
@@ -850,7 +857,7 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
dws->caps |= DW_SPI_CAP_DFS32;
- dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
+ dws->dfs_offset = __bf_shf(DW_PSSI_CTRLR0_DFS32_MASK);
dev_dbg(dev, "Detected 32-bits max data frame size\n");
}
} else {
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 893b78c43a50..634085eadad1 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -41,39 +41,36 @@
#define DW_SPI_CS_OVERRIDE 0xf4
/* Bit fields in CTRLR0 (DWC APB SSI) */
-#define DW_PSSI_CTRLR0_DFS_OFFSET 0
#define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
-#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
+#define DW_PSSI_CTRLR0_DFS32_MASK GENMASK(20, 16)
-#define DW_PSSI_CTRLR0_FRF_OFFSET 4
+#define DW_PSSI_CTRLR0_FRF_MASK GENMASK(5, 4)
#define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
#define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
#define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
#define DW_SPI_CTRLR0_FRF_RESV 0x3
-#define DW_PSSI_CTRLR0_MODE_OFFSET 6
-#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
-#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
+#define DW_PSSI_CTRLR0_MODE_MASK GENMASK(7, 6)
+#define DW_PSSI_CTRLR0_SCPHA BIT(6)
+#define DW_PSSI_CTRLR0_SCPOL BIT(7)
-#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
-#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
+#define DW_PSSI_CTRLR0_TMOD_MASK GENMASK(9, 8)
#define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
#define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
#define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
#define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
-#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
-#define DW_PSSI_CTRLR0_SRL_OFFSET 11
-#define DW_PSSI_CTRLR0_CFS_OFFSET 12
+#define DW_PSSI_CTRLR0_SLV_OE BIT(10)
+#define DW_PSSI_CTRLR0_SRL BIT(11)
+#define DW_PSSI_CTRLR0_CFS BIT(12)
/* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
-#define DW_HSSI_CTRLR0_SRL_OFFSET 13
-#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
+#define DW_HSSI_CTRLR0_DFS_MASK GENMASK(4, 0)
+#define DW_HSSI_CTRLR0_FRF_MASK GENMASK(7, 6)
+#define DW_HSSI_CTRLR0_SCPHA BIT(8)
+#define DW_HSSI_CTRLR0_SCPOL BIT(9)
#define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
-#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
-#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
-#define DW_HSSI_CTRLR0_FRF_OFFSET 6
-#define DW_HSSI_CTRLR0_DFS_OFFSET 0
+#define DW_HSSI_CTRLR0_SRL BIT(13)
/*
* For Keem Bay, CTRLR0[31] is used to select controller mode.
@@ -86,26 +83,27 @@
#define DW_SPI_NDF_MASK GENMASK(15, 0)
/* Bit fields in SR, 7 bits */
-#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
-#define DW_SPI_SR_BUSY (1 << 0)
-#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
-#define DW_SPI_SR_TF_EMPT (1 << 2)
-#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
-#define DW_SPI_SR_RF_FULL (1 << 4)
-#define DW_SPI_SR_TX_ERR (1 << 5)
-#define DW_SPI_SR_DCOL (1 << 6)
+#define DW_SPI_SR_MASK GENMASK(6, 0)
+#define DW_SPI_SR_BUSY BIT(0)
+#define DW_SPI_SR_TF_NOT_FULL BIT(1)
+#define DW_SPI_SR_TF_EMPT BIT(2)
+#define DW_SPI_SR_RF_NOT_EMPT BIT(3)
+#define DW_SPI_SR_RF_FULL BIT(4)
+#define DW_SPI_SR_TX_ERR BIT(5)
+#define DW_SPI_SR_DCOL BIT(6)
/* Bit fields in ISR, IMR, RISR, 7 bits */
-#define DW_SPI_INT_TXEI (1 << 0)
-#define DW_SPI_INT_TXOI (1 << 1)
-#define DW_SPI_INT_RXUI (1 << 2)
-#define DW_SPI_INT_RXOI (1 << 3)
-#define DW_SPI_INT_RXFI (1 << 4)
-#define DW_SPI_INT_MSTI (1 << 5)
+#define DW_SPI_INT_MASK GENMASK(5, 0)
+#define DW_SPI_INT_TXEI BIT(0)
+#define DW_SPI_INT_TXOI BIT(1)
+#define DW_SPI_INT_RXUI BIT(2)
+#define DW_SPI_INT_RXOI BIT(3)
+#define DW_SPI_INT_RXFI BIT(4)
+#define DW_SPI_INT_MSTI BIT(5)
/* Bit fields in DMACR */
-#define DW_SPI_DMACR_RDMAE (1 << 0)
-#define DW_SPI_DMACR_TDMAE (1 << 1)
+#define DW_SPI_DMACR_RDMAE BIT(0)
+#define DW_SPI_DMACR_TDMAE BIT(1)
/* Mem/DMA operations helpers */
#define DW_SPI_WAIT_RETRIES 5
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
` (2 preceding siblings ...)
2021-11-12 20:49 ` [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros Serge Semin
@ 2021-11-12 20:49 ` Serge Semin
2021-11-12 21:27 ` Andy Shevchenko
2021-11-12 21:30 ` [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Andy Shevchenko
4 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2021-11-12 20:49 UTC (permalink / raw)
To: Serge Semin, Mark Brown, Nandhini Srikandan
Cc: Serge Semin, Andy Shevchenko, Andy Shevchenko, linux-spi, linux-kernel
Each Synopsys DWC SSI controller is equipped with a Synopsys Component
version register, which encodes a version ID of an IP-core the
controller has been synthesized from. That can be useful in future for the
version-based conditional features implementation in the driver.
Note the component version is encoded as an ASCII string so we need to
convert it from the string to a normal unsigned integer to be easily used
then in the driver statements.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
drivers/spi/spi-dw-core.c | 18 ++++++++++++++++++
drivers/spi/spi-dw.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index b4cbcd38eaba..1766a29ca790 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -823,6 +823,24 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
{
dw_spi_reset_chip(dws);
+ /*
+ * Retrieve the Synopsys component version if it hasn't been specified
+ * by the platform. Note the CoreKit version ID is encoded as a 4bytes
+ * ASCII string enclosed with '*' symbol.
+ */
+ if (!dws->ver) {
+ u32 comp;
+
+ comp = dw_readl(dws, DW_SPI_VERSION);
+ dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
+ dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
+ dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
+
+ dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
+ (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ",
+ dws->ver / 100, dws->ver % 100);
+ }
+
/*
* Try to detect the FIFO depth if not set by interface driver,
* the depth could be from 2 to 256 from HW spec
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 634085eadad1..d06857d8d173 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -149,6 +149,7 @@ struct dw_spi {
u32 max_mem_freq; /* max mem-ops bus freq */
u32 max_freq; /* max bus freq supported */
+ u32 ver; /* Synopsys component version */
u32 caps; /* DW SPI capabilities */
u32 reg_io_width; /* DR I/O width in bytes */
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] spi: dw: Put the driver entities naming in order
2021-11-12 20:49 ` [PATCH 2/4] spi: dw: Put the driver entities naming in order Serge Semin
@ 2021-11-12 21:22 ` Andy Shevchenko
2021-11-12 21:30 ` Serge Semin
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-12 21:22 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 10:51 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> Mostly due to a long driver history it's methods and macro names look a
> bit messy. In particularly that concerns the code their prefixes. A
> biggest part of the driver functions and macros have got the dw_spi/DW_SPI
> prefixes. But there are some entities which have been just
> "spi_/SPI_"-prefixed. Especially that concerns the CSR and their fields
> macro definitions. It makes the code harder to comprehend since such
> methods and macros can be easily confused with the global SPI-subsystem
> exports. In this case the only possible way to more or less quickly
> distinguish one naming space from another is either by context or by the
> argument type, which most of the times isn't that easy anyway. In addition
> to that a new DW SSI IP-core support has been added in the framework of
> commit e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi"), which
> introduced a new set or macro-prefixes to describe CTRLR0-specific fields
> and worsen the situation. Finally there are methods with
> no DW SPI driver-reference prefix at all, that make the code reading even
> harder. So in order to ease the driver hacking let's bring the code naming
> to a common base:
> 1) Each method is supposed to have "dw_spi_" prefix so to be easily
> distinguished from the kernel API, e.g. SPI-subsystem methods and macros.
> (Exception is the local implementation of the readl/writel methods since
> being just the regspace accessors.)
> 2) Each generically used macro should have DW_SPI_-prefix thus being
> easily comprehended as the local driver definition.
> 3) DW APB SSI and DW SSI specific macros should have prefixes as DW_PSSI_
In the compatible strings the parameter has "assi" and not "pssi".
What did I miss?
> and DW_HSSI_ respectively so referring to the system buses they support
> (APB and AHB similarly to the DT clocks naming like pclk, hclk).
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Folks, any ideas of a better naming scheme especially for the DW APB SSI
> and DW SSI specific macros are very welcome.
> ---
> drivers/spi/spi-dw-bt1.c | 8 +--
> drivers/spi/spi-dw-core.c | 138 ++++++++++++++++++------------------
> drivers/spi/spi-dw-dma.c | 50 ++++++-------
> drivers/spi/spi-dw-mmio.c | 20 +++---
> drivers/spi/spi-dw-pci.c | 59 ++++++++--------
> drivers/spi/spi-dw.h | 145 +++++++++++++++++++-------------------
> 6 files changed, 211 insertions(+), 209 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-bt1.c b/drivers/spi/spi-dw-bt1.c
> index 5be6b7b80c21..0411088dc443 100644
> --- a/drivers/spi/spi-dw-bt1.c
> +++ b/drivers/spi/spi-dw-bt1.c
> @@ -123,7 +123,7 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
> len = min_t(size_t, len, dwsbt1->map_len - offs);
>
> /* Collect the controller configuration required by the operation */
> - cfg.tmode = SPI_TMOD_EPROMREAD;
> + cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> cfg.dfs = 8;
> cfg.ndf = 4;
> cfg.freq = mem->spi->max_speed_hz;
> @@ -131,13 +131,13 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
> /* Make sure the corresponding CS is de-asserted on transmission */
> dw_spi_set_cs(mem->spi, false);
>
> - spi_enable_chip(dws, 0);
> + dw_spi_enable_chip(dws, 0);
>
> dw_spi_update_config(dws, mem->spi, &cfg);
>
> - spi_umask_intr(dws, SPI_INT_RXFI);
> + dw_spi_umask_intr(dws, DW_SPI_INT_RXFI);
>
> - spi_enable_chip(dws, 1);
> + dw_spi_enable_chip(dws, 1);
>
> /*
> * Enable the transparent mode of the System Boot Controller.
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index f5446d9c6f27..4d91ffb5c0d8 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -24,7 +24,7 @@
> #endif
>
> /* Slave spi_device related */
> -struct chip_data {
> +struct dw_spi_chip_data {
> u32 cr0;
> u32 rx_sample_dly; /* RX sample delay */
> };
> @@ -109,7 +109,7 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
> EXPORT_SYMBOL_GPL(dw_spi_set_cs);
>
> /* Return the max entries we can fill into tx fifo */
> -static inline u32 tx_max(struct dw_spi *dws)
> +static inline u32 dw_spi_tx_max(struct dw_spi *dws)
> {
> u32 tx_room, rxtx_gap;
>
> @@ -129,14 +129,14 @@ static inline u32 tx_max(struct dw_spi *dws)
> }
>
> /* Return the max entries we should read out of rx fifo */
> -static inline u32 rx_max(struct dw_spi *dws)
> +static inline u32 dw_spi_rx_max(struct dw_spi *dws)
> {
> return min_t(u32, dws->rx_len, dw_readl(dws, DW_SPI_RXFLR));
> }
>
> static void dw_writer(struct dw_spi *dws)
> {
> - u32 max = tx_max(dws);
> + u32 max = dw_spi_tx_max(dws);
> u32 txw = 0;
>
> while (max--) {
> @@ -157,7 +157,7 @@ static void dw_writer(struct dw_spi *dws)
>
> static void dw_reader(struct dw_spi *dws)
> {
> - u32 max = rx_max(dws);
> + u32 max = dw_spi_rx_max(dws);
> u32 rxw;
>
> while (max--) {
> @@ -186,24 +186,24 @@ int dw_spi_check_status(struct dw_spi *dws, bool raw)
> else
> irq_status = dw_readl(dws, DW_SPI_ISR);
>
> - if (irq_status & SPI_INT_RXOI) {
> + if (irq_status & DW_SPI_INT_RXOI) {
> dev_err(&dws->master->dev, "RX FIFO overflow detected\n");
> ret = -EIO;
> }
>
> - if (irq_status & SPI_INT_RXUI) {
> + if (irq_status & DW_SPI_INT_RXUI) {
> dev_err(&dws->master->dev, "RX FIFO underflow detected\n");
> ret = -EIO;
> }
>
> - if (irq_status & SPI_INT_TXOI) {
> + if (irq_status & DW_SPI_INT_TXOI) {
> dev_err(&dws->master->dev, "TX FIFO overflow detected\n");
> ret = -EIO;
> }
>
> /* Generically handle the erroneous situation */
> if (ret) {
> - spi_reset_chip(dws);
> + dw_spi_reset_chip(dws);
> if (dws->master->cur_msg)
> dws->master->cur_msg->status = ret;
> }
> @@ -230,7 +230,7 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
> */
> dw_reader(dws);
> if (!dws->rx_len) {
> - spi_mask_intr(dws, 0xff);
> + dw_spi_mask_intr(dws, 0xff);
> spi_finalize_current_transfer(dws->master);
> } else if (dws->rx_len <= dw_readl(dws, DW_SPI_RXFTLR)) {
> dw_writel(dws, DW_SPI_RXFTLR, dws->rx_len - 1);
> @@ -241,10 +241,10 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
> * disabled after the data transmission is finished so not to
> * have the TXE IRQ flood at the final stage of the transfer.
> */
> - if (irq_status & SPI_INT_TXEI) {
> + if (irq_status & DW_SPI_INT_TXEI) {
> dw_writer(dws);
> if (!dws->tx_len)
> - spi_mask_intr(dws, SPI_INT_TXEI);
> + dw_spi_mask_intr(dws, DW_SPI_INT_TXEI);
> }
>
> return IRQ_HANDLED;
> @@ -260,7 +260,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
> return IRQ_NONE;
>
> if (!master->cur_msg) {
> - spi_mask_intr(dws, 0xff);
> + dw_spi_mask_intr(dws, 0xff);
> return IRQ_HANDLED;
> }
>
> @@ -271,37 +271,37 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
> {
> u32 cr0 = 0;
>
> - if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
> + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> /* CTRLR0[ 5: 4] Frame Format */
> - cr0 |= SPI_FRF_MOTO_SPI << SPI_FRF_OFFSET;
> + cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
>
> /*
> * SPI mode (SCPOL|SCPH)
> * CTRLR0[ 6] Serial Clock Phase
> * CTRLR0[ 7] Serial Clock Polarity
> */
> - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << SPI_SCOL_OFFSET;
> - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << SPI_SCPH_OFFSET;
> + cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
> + cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
>
> /* CTRLR0[11] Shift Register Loop */
> - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
> + cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
> } else {
> /* CTRLR0[ 7: 6] Frame Format */
> - cr0 |= SPI_FRF_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
> + cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
>
> /*
> * SPI mode (SCPOL|SCPH)
> * CTRLR0[ 8] Serial Clock Phase
> * CTRLR0[ 9] Serial Clock Polarity
> */
> - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DWC_SSI_CTRLR0_SCPOL_OFFSET;
> - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DWC_SSI_CTRLR0_SCPH_OFFSET;
> + cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
> + cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
>
> /* CTRLR0[13] Shift Register Loop */
> - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DWC_SSI_CTRLR0_SRL_OFFSET;
> + cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
>
> if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> - cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> + cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> }
>
> return cr0;
> @@ -310,7 +310,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
> void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> struct dw_spi_cfg *cfg)
> {
> - struct chip_data *chip = spi_get_ctldata(spi);
> + struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
> u32 cr0 = chip->cr0;
> u32 speed_hz;
> u16 clk_div;
> @@ -318,16 +318,17 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> /* CTRLR0[ 4/3: 0] or CTRLR0[ 20: 16] Data Frame Size */
> cr0 |= (cfg->dfs - 1) << dws->dfs_offset;
>
> - if (!(dws->caps & DW_SPI_CAP_DWC_SSI))
> + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
> /* CTRLR0[ 9:8] Transfer Mode */
> - cr0 |= cfg->tmode << SPI_TMOD_OFFSET;
> + cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
> else
> /* CTRLR0[11:10] Transfer Mode */
> - cr0 |= cfg->tmode << DWC_SSI_CTRLR0_TMOD_OFFSET;
> + cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
>
> dw_writel(dws, DW_SPI_CTRLR0, cr0);
>
> - if (cfg->tmode == SPI_TMOD_EPROMREAD || cfg->tmode == SPI_TMOD_RO)
> + if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> + cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
> dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
>
> /* Note DW APB SSI clock divider doesn't support odd numbers */
> @@ -335,7 +336,7 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> speed_hz = dws->max_freq / clk_div;
>
> if (dws->current_freq != speed_hz) {
> - spi_set_clk(dws, clk_div);
> + dw_spi_set_clk(dws, clk_div);
> dws->current_freq = speed_hz;
> }
>
> @@ -363,9 +364,9 @@ static void dw_spi_irq_setup(struct dw_spi *dws)
>
> dws->transfer_handler = dw_spi_transfer_handler;
>
> - imask = SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI |
> - SPI_INT_RXFI;
> - spi_umask_intr(dws, imask);
> + imask = DW_SPI_INT_TXEI | DW_SPI_INT_TXOI |
> + DW_SPI_INT_RXUI | DW_SPI_INT_RXOI | DW_SPI_INT_RXFI;
> + dw_spi_umask_intr(dws, imask);
> }
>
> /*
> @@ -405,11 +406,12 @@ static int dw_spi_poll_transfer(struct dw_spi *dws,
> }
>
> static int dw_spi_transfer_one(struct spi_controller *master,
> - struct spi_device *spi, struct spi_transfer *transfer)
> + struct spi_device *spi,
> + struct spi_transfer *transfer)
> {
> struct dw_spi *dws = spi_controller_get_devdata(master);
> struct dw_spi_cfg cfg = {
> - .tmode = SPI_TMOD_TR,
> + .tmode = DW_SPI_CTRLR0_TMOD_TR,
> .dfs = transfer->bits_per_word,
> .freq = transfer->speed_hz,
> };
> @@ -425,7 +427,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> /* Ensure the data above is visible for all CPUs */
> smp_mb();
>
> - spi_enable_chip(dws, 0);
> + dw_spi_enable_chip(dws, 0);
>
> dw_spi_update_config(dws, spi, &cfg);
>
> @@ -436,7 +438,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> dws->dma_mapped = master->cur_msg_mapped;
>
> /* For poll mode just disable all interrupts */
> - spi_mask_intr(dws, 0xff);
> + dw_spi_mask_intr(dws, 0xff);
>
> if (dws->dma_mapped) {
> ret = dws->dma_ops->dma_setup(dws, transfer);
> @@ -444,7 +446,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> return ret;
> }
>
> - spi_enable_chip(dws, 1);
> + dw_spi_enable_chip(dws, 1);
>
> if (dws->dma_mapped)
> return dws->dma_ops->dma_transfer(dws, transfer);
> @@ -457,20 +459,20 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> }
>
> static void dw_spi_handle_err(struct spi_controller *master,
> - struct spi_message *msg)
> + struct spi_message *msg)
> {
> struct dw_spi *dws = spi_controller_get_devdata(master);
>
> if (dws->dma_mapped)
> dws->dma_ops->dma_stop(dws);
>
> - spi_reset_chip(dws);
> + dw_spi_reset_chip(dws);
> }
>
> static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> {
> if (op->data.dir == SPI_MEM_DATA_IN)
> - op->data.nbytes = clamp_val(op->data.nbytes, 0, SPI_NDF_MASK + 1);
> + op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);
>
> return 0;
> }
> @@ -498,7 +500,7 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
> if (op->data.dir == SPI_MEM_DATA_OUT)
> len += op->data.nbytes;
>
> - if (len <= SPI_BUF_SIZE) {
> + if (len <= DW_SPI_BUF_SIZE) {
> out = dws->buf;
> } else {
> out = kzalloc(len, GFP_KERNEL);
> @@ -512,9 +514,9 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
> * single buffer in order to speed the data transmission up.
> */
> for (i = 0; i < op->cmd.nbytes; ++i)
> - out[i] = SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
> + out[i] = DW_SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
> for (j = 0; j < op->addr.nbytes; ++i, ++j)
> - out[i] = SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> + out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> for (j = 0; j < op->dummy.nbytes; ++i, ++j)
> out[i] = 0x0;
>
> @@ -587,7 +589,7 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
> entries = readl_relaxed(dws->regs + DW_SPI_RXFLR);
> if (!entries) {
> sts = readl_relaxed(dws->regs + DW_SPI_RISR);
> - if (sts & SPI_INT_RXOI) {
> + if (sts & DW_SPI_INT_RXOI) {
> dev_err(&dws->master->dev, "FIFO overflow on Rx\n");
> return -EIO;
> }
> @@ -603,12 +605,12 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
>
> static inline bool dw_spi_ctlr_busy(struct dw_spi *dws)
> {
> - return dw_readl(dws, DW_SPI_SR) & SR_BUSY;
> + return dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_BUSY;
> }
>
> static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
> {
> - int retry = SPI_WAIT_RETRIES;
> + int retry = DW_SPI_WAIT_RETRIES;
> struct spi_delay delay;
> unsigned long ns, us;
> u32 nents;
> @@ -638,9 +640,9 @@ static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
>
> static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
> {
> - spi_enable_chip(dws, 0);
> + dw_spi_enable_chip(dws, 0);
> dw_spi_set_cs(spi, true);
> - spi_enable_chip(dws, 1);
> + dw_spi_enable_chip(dws, 1);
> }
>
> /*
> @@ -673,19 +675,19 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> cfg.dfs = 8;
> cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
> if (op->data.dir == SPI_MEM_DATA_IN) {
> - cfg.tmode = SPI_TMOD_EPROMREAD;
> + cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> cfg.ndf = op->data.nbytes;
> } else {
> - cfg.tmode = SPI_TMOD_TO;
> + cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
> }
>
> - spi_enable_chip(dws, 0);
> + dw_spi_enable_chip(dws, 0);
>
> dw_spi_update_config(dws, mem->spi, &cfg);
>
> - spi_mask_intr(dws, 0xff);
> + dw_spi_mask_intr(dws, 0xff);
>
> - spi_enable_chip(dws, 1);
> + dw_spi_enable_chip(dws, 1);
>
> /*
> * DW APB SSI controller has very nasty peculiarities. First originally
> @@ -768,7 +770,7 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
> static int dw_spi_setup(struct spi_device *spi)
> {
> struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
> - struct chip_data *chip;
> + struct dw_spi_chip_data *chip;
>
> /* Only alloc on first setup */
> chip = spi_get_ctldata(spi);
> @@ -776,7 +778,7 @@ static int dw_spi_setup(struct spi_device *spi)
> struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
> u32 rx_sample_dly_ns;
>
> - chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
> + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> if (!chip)
> return -ENOMEM;
> spi_set_ctldata(spi, chip);
> @@ -803,16 +805,16 @@ static int dw_spi_setup(struct spi_device *spi)
>
> static void dw_spi_cleanup(struct spi_device *spi)
> {
> - struct chip_data *chip = spi_get_ctldata(spi);
> + struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
>
> kfree(chip);
> spi_set_ctldata(spi, NULL);
> }
>
> /* Restart the controller, disable all interrupts, clean rx fifo */
> -static void spi_hw_init(struct device *dev, struct dw_spi *dws)
> +static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
> {
> - spi_reset_chip(dws);
> + dw_spi_reset_chip(dws);
>
> /*
> * Try to detect the FIFO depth if not set by interface driver,
> @@ -837,18 +839,18 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
> * writability. Note DWC SSI controller also has the extended DFS, but
> * with zero offset.
> */
> - if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
> + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0);
>
> - spi_enable_chip(dws, 0);
> + dw_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);
> + dw_spi_enable_chip(dws, 1);
>
> - if (!(cr0 & SPI_DFS_MASK)) {
> + if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
> dws->caps |= DW_SPI_CAP_DFS32;
> - dws->dfs_offset = SPI_DFS32_OFFSET;
> + dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
> dev_dbg(dev, "Detected 32-bits max data frame size\n");
> }
> } else {
> @@ -878,7 +880,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> spi_controller_set_devdata(master, dws);
>
> /* Basic HW init */
> - spi_hw_init(dev, dws);
> + dw_spi_hw_init(dev, dws);
>
> ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
> master);
> @@ -939,7 +941,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> err_dma_exit:
> if (dws->dma_ops && dws->dma_ops->dma_exit)
> dws->dma_ops->dma_exit(dws);
> - spi_enable_chip(dws, 0);
> + dw_spi_enable_chip(dws, 0);
> free_irq(dws->irq, master);
> err_free_master:
> spi_controller_put(master);
> @@ -956,7 +958,7 @@ void dw_spi_remove_host(struct dw_spi *dws)
> if (dws->dma_ops && dws->dma_ops->dma_exit)
> dws->dma_ops->dma_exit(dws);
>
> - spi_shutdown_chip(dws);
> + dw_spi_shutdown_chip(dws);
>
> free_irq(dws->irq, dws->master);
> }
> @@ -970,14 +972,14 @@ int dw_spi_suspend_host(struct dw_spi *dws)
> if (ret)
> return ret;
>
> - spi_shutdown_chip(dws);
> + dw_spi_shutdown_chip(dws);
> return 0;
> }
> EXPORT_SYMBOL_GPL(dw_spi_suspend_host);
>
> int dw_spi_resume_host(struct dw_spi *dws)
> {
> - spi_hw_init(&dws->master->dev, dws);
> + dw_spi_hw_init(&dws->master->dev, dws);
> return spi_controller_resume(dws->master);
> }
> EXPORT_SYMBOL_GPL(dw_spi_resume_host);
> diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> index a09831c62192..fd6a2154d2ce 100644
> --- a/drivers/spi/spi-dw-dma.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -17,10 +17,10 @@
>
> #include "spi-dw.h"
>
> -#define RX_BUSY 0
> -#define RX_BURST_LEVEL 16
> -#define TX_BUSY 1
> -#define TX_BURST_LEVEL 16
> +#define DW_SPI_RX_BUSY 0
> +#define DW_SPI_RX_BURST_LEVEL 16
> +#define DW_SPI_TX_BUSY 1
> +#define DW_SPI_TX_BURST_LEVEL 16
>
> static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
> {
> @@ -45,7 +45,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> if (!ret && caps.max_burst)
> max_burst = caps.max_burst;
> else
> - max_burst = RX_BURST_LEVEL;
> + max_burst = DW_SPI_RX_BURST_LEVEL;
>
> dws->rxburst = min(max_burst, def_burst);
> dw_writel(dws, DW_SPI_DMARDLR, dws->rxburst - 1);
> @@ -54,7 +54,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> if (!ret && caps.max_burst)
> max_burst = caps.max_burst;
> else
> - max_burst = TX_BURST_LEVEL;
> + max_burst = DW_SPI_TX_BURST_LEVEL;
>
> /*
> * Having a Rx DMA channel serviced with higher priority than a Tx DMA
> @@ -226,13 +226,13 @@ static int dw_spi_dma_wait(struct dw_spi *dws, unsigned int len, u32 speed)
>
> static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
> {
> - return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
> + return !(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_TF_EMPT);
> }
>
> static int dw_spi_dma_wait_tx_done(struct dw_spi *dws,
> struct spi_transfer *xfer)
> {
> - int retry = SPI_WAIT_RETRIES;
> + int retry = DW_SPI_WAIT_RETRIES;
> struct spi_delay delay;
> u32 nents;
>
> @@ -259,8 +259,8 @@ static void dw_spi_dma_tx_done(void *arg)
> {
> struct dw_spi *dws = arg;
>
> - clear_bit(TX_BUSY, &dws->dma_chan_busy);
> - if (test_bit(RX_BUSY, &dws->dma_chan_busy))
> + clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
> + if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy))
> return;
>
> complete(&dws->dma_completion);
> @@ -304,19 +304,19 @@ static int dw_spi_dma_submit_tx(struct dw_spi *dws, struct scatterlist *sgl,
> return ret;
> }
>
> - set_bit(TX_BUSY, &dws->dma_chan_busy);
> + set_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
>
> return 0;
> }
>
> static inline bool dw_spi_dma_rx_busy(struct dw_spi *dws)
> {
> - return !!(dw_readl(dws, DW_SPI_SR) & SR_RF_NOT_EMPT);
> + return !!(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_RF_NOT_EMPT);
> }
>
> static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
> {
> - int retry = SPI_WAIT_RETRIES;
> + int retry = DW_SPI_WAIT_RETRIES;
> struct spi_delay delay;
> unsigned long ns, us;
> u32 nents;
> @@ -360,8 +360,8 @@ static void dw_spi_dma_rx_done(void *arg)
> {
> struct dw_spi *dws = arg;
>
> - clear_bit(RX_BUSY, &dws->dma_chan_busy);
> - if (test_bit(TX_BUSY, &dws->dma_chan_busy))
> + clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
> + if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy))
> return;
>
> complete(&dws->dma_completion);
> @@ -405,7 +405,7 @@ static int dw_spi_dma_submit_rx(struct dw_spi *dws, struct scatterlist *sgl,
> return ret;
> }
>
> - set_bit(RX_BUSY, &dws->dma_chan_busy);
> + set_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
>
> return 0;
> }
> @@ -430,16 +430,16 @@ static int dw_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
> }
>
> /* Set the DMA handshaking interface */
> - dma_ctrl = SPI_DMA_TDMAE;
> + dma_ctrl = DW_SPI_DMACR_TDMAE;
> if (xfer->rx_buf)
> - dma_ctrl |= SPI_DMA_RDMAE;
> + dma_ctrl |= DW_SPI_DMACR_RDMAE;
> dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
>
> /* Set the interrupt mask */
> - imr = SPI_INT_TXOI;
> + imr = DW_SPI_INT_TXOI;
> if (xfer->rx_buf)
> - imr |= SPI_INT_RXUI | SPI_INT_RXOI;
> - spi_umask_intr(dws, imr);
> + imr |= DW_SPI_INT_RXUI | DW_SPI_INT_RXOI;
> + dw_spi_umask_intr(dws, imr);
>
> reinit_completion(&dws->dma_completion);
>
> @@ -615,13 +615,13 @@ static int dw_spi_dma_transfer(struct dw_spi *dws, struct spi_transfer *xfer)
>
> static void dw_spi_dma_stop(struct dw_spi *dws)
> {
> - if (test_bit(TX_BUSY, &dws->dma_chan_busy)) {
> + if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy)) {
> dmaengine_terminate_sync(dws->txchan);
> - clear_bit(TX_BUSY, &dws->dma_chan_busy);
> + clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
> }
> - if (test_bit(RX_BUSY, &dws->dma_chan_busy)) {
> + if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy)) {
> dmaengine_terminate_sync(dws->rxchan);
> - clear_bit(RX_BUSY, &dws->dma_chan_busy);
> + clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
> }
> }
>
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 17c06039a74d..435c91aecbca 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -196,18 +196,18 @@ static int dw_spi_alpine_init(struct platform_device *pdev,
> return 0;
> }
>
> -static int dw_spi_dw_apb_init(struct platform_device *pdev,
> - struct dw_spi_mmio *dwsmmio)
> +static int dw_spi_assi_init(struct platform_device *pdev,
> + struct dw_spi_mmio *dwsmmio)
> {
> dw_spi_dma_setup_generic(&dwsmmio->dws);
>
> return 0;
> }
>
> -static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
> - struct dw_spi_mmio *dwsmmio)
> +static int dw_spi_hssi_init(struct platform_device *pdev,
> + struct dw_spi_mmio *dwsmmio)
> {
> - dwsmmio->dws.caps = DW_SPI_CAP_DWC_SSI;
> + dwsmmio->dws.caps = DW_SPI_CAP_DWC_HSSI;
>
> dw_spi_dma_setup_generic(&dwsmmio->dws);
>
> @@ -217,7 +217,7 @@ static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
> static int dw_spi_keembay_init(struct platform_device *pdev,
> struct dw_spi_mmio *dwsmmio)
> {
> - dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_SSI;
> + dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_HSSI;
>
> return 0;
> }
> @@ -342,12 +342,12 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id dw_spi_mmio_of_match[] = {
> - { .compatible = "snps,dw-apb-ssi", .data = dw_spi_dw_apb_init},
> + { .compatible = "snps,dw-apb-ssi", .data = dw_spi_assi_init},
> { .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
> { .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
> { .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
> - { .compatible = "renesas,rzn1-spi", .data = dw_spi_dw_apb_init},
> - { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
> + { .compatible = "renesas,rzn1-spi", .data = dw_spi_assi_init},
> + { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_hssi_init},
> { .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
> { .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> { .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> @@ -357,7 +357,7 @@ MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
>
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
> - {"HISI0173", (kernel_ulong_t)dw_spi_dw_apb_init},
> + {"HISI0173", (kernel_ulong_t)dw_spi_assi_init},
> {},
> };
> MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);
> diff --git a/drivers/spi/spi-dw-pci.c b/drivers/spi/spi-dw-pci.c
> index 8a91cd58102f..e4a239bc3d36 100644
> --- a/drivers/spi/spi-dw-pci.c
> +++ b/drivers/spi/spi-dw-pci.c
> @@ -24,14 +24,14 @@
> #define CLK_SPI_CDIV_MASK 0x00000e00
> #define CLK_SPI_DISABLE_OFFSET 8
>
> -struct spi_pci_desc {
> +struct dw_spi_pci_desc {
> int (*setup)(struct dw_spi *);
> u16 num_cs;
> u16 bus_num;
> u32 max_freq;
> };
>
> -static int spi_mid_init(struct dw_spi *dws)
> +static int dw_spi_pci_mid_init(struct dw_spi *dws)
> {
> void __iomem *clk_reg;
> u32 clk_cdiv;
> @@ -53,36 +53,36 @@ static int spi_mid_init(struct dw_spi *dws)
> return 0;
> }
>
> -static int spi_generic_init(struct dw_spi *dws)
> +static int dw_spi_pci_generic_init(struct dw_spi *dws)
> {
> dw_spi_dma_setup_generic(dws);
>
> return 0;
> }
>
> -static struct spi_pci_desc spi_pci_mid_desc_1 = {
> - .setup = spi_mid_init,
> +static struct dw_spi_pci_desc dw_spi_pci_mid_desc_1 = {
> + .setup = dw_spi_pci_mid_init,
> .num_cs = 5,
> .bus_num = 0,
> };
>
> -static struct spi_pci_desc spi_pci_mid_desc_2 = {
> - .setup = spi_mid_init,
> +static struct dw_spi_pci_desc dw_spi_pci_mid_desc_2 = {
> + .setup = dw_spi_pci_mid_init,
> .num_cs = 2,
> .bus_num = 1,
> };
>
> -static struct spi_pci_desc spi_pci_ehl_desc = {
> - .setup = spi_generic_init,
> +static struct dw_spi_pci_desc dw_spi_pci_ehl_desc = {
> + .setup = dw_spi_pci_generic_init,
> .num_cs = 2,
> .bus_num = -1,
> .max_freq = 100000000,
> };
>
> -static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +static int dw_spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> + struct dw_spi_pci_desc *desc = (struct dw_spi_pci_desc *)ent->driver_data;
> struct dw_spi *dws;
> - struct spi_pci_desc *desc = (struct spi_pci_desc *)ent->driver_data;
> int pci_bar = 0;
> int ret;
>
> @@ -150,7 +150,7 @@ static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> return ret;
> }
>
> -static void spi_pci_remove(struct pci_dev *pdev)
> +static void dw_spi_pci_remove(struct pci_dev *pdev)
> {
> struct dw_spi *dws = pci_get_drvdata(pdev);
>
> @@ -162,14 +162,14 @@ static void spi_pci_remove(struct pci_dev *pdev)
> }
>
> #ifdef CONFIG_PM_SLEEP
> -static int spi_suspend(struct device *dev)
> +static int dw_spi_pci_suspend(struct device *dev)
> {
> struct dw_spi *dws = dev_get_drvdata(dev);
>
> return dw_spi_suspend_host(dws);
> }
>
> -static int spi_resume(struct device *dev)
> +static int dw_spi_pci_resume(struct device *dev)
> {
> struct dw_spi *dws = dev_get_drvdata(dev);
>
> @@ -177,38 +177,37 @@ static int spi_resume(struct device *dev)
> }
> #endif
>
> -static SIMPLE_DEV_PM_OPS(dw_spi_pm_ops, spi_suspend, spi_resume);
> +static SIMPLE_DEV_PM_OPS(dw_spi_pci_pm_ops, dw_spi_pci_suspend, dw_spi_pci_resume);
>
> -static const struct pci_device_id pci_ids[] = {
> +static const struct pci_device_id dw_spi_pci_ids[] = {
> /* Intel MID platform SPI controller 0 */
> /*
> * The access to the device 8086:0801 is disabled by HW, since it's
> * exclusively used by SCU to communicate with MSIC.
> */
> /* Intel MID platform SPI controller 1 */
> - { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&spi_pci_mid_desc_1},
> + { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&dw_spi_pci_mid_desc_1},
> /* Intel MID platform SPI controller 2 */
> - { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&spi_pci_mid_desc_2},
> + { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&dw_spi_pci_mid_desc_2},
> /* Intel Elkhart Lake PSE SPI controllers */
> - { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&spi_pci_ehl_desc},
> - { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&spi_pci_ehl_desc},
> - { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&spi_pci_ehl_desc},
> - { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&spi_pci_ehl_desc},
> + { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> + { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> + { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> + { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> {},
> };
> -MODULE_DEVICE_TABLE(pci, pci_ids);
> +MODULE_DEVICE_TABLE(pci, dw_spi_pci_ids);
>
> -static struct pci_driver dw_spi_driver = {
> +static struct pci_driver dw_spi_pci_driver = {
> .name = DRIVER_NAME,
> - .id_table = pci_ids,
> - .probe = spi_pci_probe,
> - .remove = spi_pci_remove,
> + .id_table = dw_spi_pci_ids,
> + .probe = dw_spi_pci_probe,
> + .remove = dw_spi_pci_remove,
> .driver = {
> - .pm = &dw_spi_pm_ops,
> + .pm = &dw_spi_pci_pm_ops,
> },
> };
> -
> -module_pci_driver(dw_spi_driver);
> +module_pci_driver(dw_spi_pci_driver);
>
> MODULE_AUTHOR("Feng Tang <feng.tang@intel.com>");
> MODULE_DESCRIPTION("PCI interface driver for DW SPI Core");
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 467c342bfe56..893b78c43a50 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef DW_SPI_HEADER_H
> -#define DW_SPI_HEADER_H
> +#ifndef __SPI_DW_H__
> +#define __SPI_DW_H__
>
> #include <linux/bits.h>
> #include <linux/completion.h>
> @@ -11,7 +11,7 @@
> #include <linux/spi/spi-mem.h>
> #include <linux/bitfield.h>
>
> -/* Register offsets */
> +/* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
> #define DW_SPI_CTRLR0 0x00
> #define DW_SPI_CTRLR1 0x04
> #define DW_SPI_SSIENR 0x08
> @@ -40,84 +40,85 @@
> #define DW_SPI_RX_SAMPLE_DLY 0xf0
> #define DW_SPI_CS_OVERRIDE 0xf4
>
> -/* 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_MOTO_SPI 0x0
> -#define SPI_FRF_TI_SSP 0x1
> -#define SPI_FRF_NS_MICROWIRE 0x2
> -#define SPI_FRF_RESV 0x3
> -
> -#define SPI_MODE_OFFSET 6
> -#define SPI_SCPH_OFFSET 6
> -#define SPI_SCOL_OFFSET 7
> -
> -#define SPI_TMOD_OFFSET 8
> -#define SPI_TMOD_MASK (0x3 << SPI_TMOD_OFFSET)
> -#define SPI_TMOD_TR 0x0 /* xmit & recv */
> -#define SPI_TMOD_TO 0x1 /* xmit only */
> -#define SPI_TMOD_RO 0x2 /* recv only */
> -#define SPI_TMOD_EPROMREAD 0x3 /* eeprom read mode */
> -
> -#define SPI_SLVOE_OFFSET 10
> -#define SPI_SRL_OFFSET 11
> -#define SPI_CFS_OFFSET 12
> -
> -/* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
> -#define DWC_SSI_CTRLR0_SRL_OFFSET 13
> -#define DWC_SSI_CTRLR0_TMOD_OFFSET 10
> -#define DWC_SSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> -#define DWC_SSI_CTRLR0_SCPOL_OFFSET 9
> -#define DWC_SSI_CTRLR0_SCPH_OFFSET 8
> -#define DWC_SSI_CTRLR0_FRF_OFFSET 6
> -#define DWC_SSI_CTRLR0_DFS_OFFSET 0
> +/* Bit fields in CTRLR0 (DWC APB SSI) */
> +#define DW_PSSI_CTRLR0_DFS_OFFSET 0
> +#define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
> +#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
> +
> +#define DW_PSSI_CTRLR0_FRF_OFFSET 4
> +#define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
> +#define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
> +#define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
> +#define DW_SPI_CTRLR0_FRF_RESV 0x3
> +
> +#define DW_PSSI_CTRLR0_MODE_OFFSET 6
> +#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
> +#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
> +
> +#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
> +#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
> +#define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
> +#define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
> +#define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
> +#define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
> +
> +#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
> +#define DW_PSSI_CTRLR0_SRL_OFFSET 11
> +#define DW_PSSI_CTRLR0_CFS_OFFSET 12
> +
> +/* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
> +#define DW_HSSI_CTRLR0_SRL_OFFSET 13
> +#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
> +#define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> +#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
> +#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
> +#define DW_HSSI_CTRLR0_FRF_OFFSET 6
> +#define DW_HSSI_CTRLR0_DFS_OFFSET 0
>
> /*
> * For Keem Bay, CTRLR0[31] is used to select controller mode.
> * 0: SSI is slave
> * 1: SSI is master
> */
> -#define DWC_SSI_CTRLR0_KEEMBAY_MST BIT(31)
> +#define DW_HSSI_CTRLR0_KEEMBAY_MST BIT(31)
>
> /* Bit fields in CTRLR1 */
> -#define SPI_NDF_MASK GENMASK(15, 0)
> +#define DW_SPI_NDF_MASK GENMASK(15, 0)
>
> /* Bit fields in SR, 7 bits */
> -#define SR_MASK 0x7f /* cover 7 bits */
> -#define SR_BUSY (1 << 0)
> -#define SR_TF_NOT_FULL (1 << 1)
> -#define SR_TF_EMPT (1 << 2)
> -#define SR_RF_NOT_EMPT (1 << 3)
> -#define SR_RF_FULL (1 << 4)
> -#define SR_TX_ERR (1 << 5)
> -#define SR_DCOL (1 << 6)
> +#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
> +#define DW_SPI_SR_BUSY (1 << 0)
> +#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
> +#define DW_SPI_SR_TF_EMPT (1 << 2)
> +#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
> +#define DW_SPI_SR_RF_FULL (1 << 4)
> +#define DW_SPI_SR_TX_ERR (1 << 5)
> +#define DW_SPI_SR_DCOL (1 << 6)
>
> /* Bit fields in ISR, IMR, RISR, 7 bits */
> -#define SPI_INT_TXEI (1 << 0)
> -#define SPI_INT_TXOI (1 << 1)
> -#define SPI_INT_RXUI (1 << 2)
> -#define SPI_INT_RXOI (1 << 3)
> -#define SPI_INT_RXFI (1 << 4)
> -#define SPI_INT_MSTI (1 << 5)
> +#define DW_SPI_INT_TXEI (1 << 0)
> +#define DW_SPI_INT_TXOI (1 << 1)
> +#define DW_SPI_INT_RXUI (1 << 2)
> +#define DW_SPI_INT_RXOI (1 << 3)
> +#define DW_SPI_INT_RXFI (1 << 4)
> +#define DW_SPI_INT_MSTI (1 << 5)
>
> /* Bit fields in DMACR */
> -#define SPI_DMA_RDMAE (1 << 0)
> -#define SPI_DMA_TDMAE (1 << 1)
> +#define DW_SPI_DMACR_RDMAE (1 << 0)
> +#define DW_SPI_DMACR_TDMAE (1 << 1)
>
> -#define SPI_WAIT_RETRIES 5
> -#define SPI_BUF_SIZE \
> +/* Mem/DMA operations helpers */
> +#define DW_SPI_WAIT_RETRIES 5
> +#define DW_SPI_BUF_SIZE \
> (sizeof_field(struct spi_mem_op, cmd.opcode) + \
> sizeof_field(struct spi_mem_op, addr.val) + 256)
> -#define SPI_GET_BYTE(_val, _idx) \
> +#define DW_SPI_GET_BYTE(_val, _idx) \
> ((_val) >> (BITS_PER_BYTE * (_idx)) & 0xff)
>
> /* DW SPI capabilities */
> #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_HSSI BIT(2)
> #define DW_SPI_CAP_DFS32 BIT(3)
>
> /* Slave spi_transfer/spi_mem_op related */
> @@ -162,7 +163,7 @@ struct dw_spi {
> unsigned int tx_len;
> void *rx;
> unsigned int rx_len;
> - u8 buf[SPI_BUF_SIZE];
> + u8 buf[DW_SPI_BUF_SIZE];
> int dma_mapped;
> u8 n_bytes; /* current is a 1/2 bytes op */
> irqreturn_t (*transfer_handler)(struct dw_spi *dws);
> @@ -224,18 +225,18 @@ static inline void dw_write_io_reg(struct dw_spi *dws, u32 offset, u32 val)
> }
> }
>
> -static inline void spi_enable_chip(struct dw_spi *dws, int enable)
> +static inline void dw_spi_enable_chip(struct dw_spi *dws, int enable)
> {
> dw_writel(dws, DW_SPI_SSIENR, (enable ? 1 : 0));
> }
>
> -static inline void spi_set_clk(struct dw_spi *dws, u16 div)
> +static inline void dw_spi_set_clk(struct dw_spi *dws, u16 div)
> {
> dw_writel(dws, DW_SPI_BAUDR, div);
> }
>
> /* Disable IRQ bits */
> -static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
> +static inline void dw_spi_mask_intr(struct dw_spi *dws, u32 mask)
> {
> u32 new_mask;
>
> @@ -244,7 +245,7 @@ static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
> }
>
> /* Enable IRQ bits */
> -static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
> +static inline void dw_spi_umask_intr(struct dw_spi *dws, u32 mask)
> {
> u32 new_mask;
>
> @@ -257,19 +258,19 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
> * and CS, then re-enables the controller back. Transmit and receive FIFO
> * buffers are cleared when the device is disabled.
> */
> -static inline void spi_reset_chip(struct dw_spi *dws)
> +static inline void dw_spi_reset_chip(struct dw_spi *dws)
> {
> - spi_enable_chip(dws, 0);
> - spi_mask_intr(dws, 0xff);
> + dw_spi_enable_chip(dws, 0);
> + dw_spi_mask_intr(dws, 0xff);
> dw_readl(dws, DW_SPI_ICR);
> dw_writel(dws, DW_SPI_SER, 0);
> - spi_enable_chip(dws, 1);
> + dw_spi_enable_chip(dws, 1);
> }
>
> -static inline void spi_shutdown_chip(struct dw_spi *dws)
> +static inline void dw_spi_shutdown_chip(struct dw_spi *dws)
> {
> - spi_enable_chip(dws, 0);
> - spi_set_clk(dws, 0);
> + dw_spi_enable_chip(dws, 0);
> + dw_spi_set_clk(dws, 0);
> }
>
> extern void dw_spi_set_cs(struct spi_device *spi, bool enable);
> @@ -293,4 +294,4 @@ static inline void dw_spi_dma_setup_generic(struct dw_spi *dws) {}
>
> #endif /* !CONFIG_SPI_DW_DMA */
>
> -#endif /* DW_SPI_HEADER_H */
> +#endif /* __SPI_DW_H__ */
> --
> 2.33.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros
2021-11-12 20:49 ` [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros Serge Semin
@ 2021-11-12 21:23 ` Andy Shevchenko
2021-11-12 21:43 ` Serge Semin
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-12 21:23 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 10:51 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> The driver has been using the offset/bitwise-shift-based approach for the
> CSR fields R/W operations since it was merged into the kernel. It can be
> simplified by using the macros defined in the linux/bitfield.h and
> linux/bit.h header files like BIT(), GENMASK(), FIELD_PREP(), FIELD_GET(),
> etc where it is required, for instance in the cached cr0 preparation
> method. Thus in order to have the FIELD_*()-macros utilized we just need
> to convert the macros with the CSR-fields offsets to the masks with the
> corresponding registers fields definition. That's where the GENMASK() and
> BIT() macros come in handy. After that the masks can be used in the
> FIELD_*()-macros where it's appropriate.
>
> We also need to convert the macros with the CRS-bit flags using the manual
> bitwise shift operations (x << y) to using the BIT() macro. Thus we'll
> have a more coherent set of the CSR-related macros.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
> drivers/spi/spi-dw-core.c | 31 +++++++++++--------
> drivers/spi/spi-dw.h | 64 +++++++++++++++++++--------------------
> 2 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 4d91ffb5c0d8..b4cbcd38eaba 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2009, Intel Corporation.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/dma-mapping.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> @@ -254,7 +255,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
> {
> struct spi_controller *master = dev_id;
> struct dw_spi *dws = spi_controller_get_devdata(master);
> - u16 irq_status = dw_readl(dws, DW_SPI_ISR) & 0x3f;
> + u16 irq_status = dw_readl(dws, DW_SPI_ISR) & DW_SPI_INT_MASK;
>
> if (!irq_status)
> return IRQ_NONE;
> @@ -273,32 +274,38 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>
> if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> /* CTRLR0[ 5: 4] Frame Format */
> - cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
> + cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
>
> /*
> * SPI mode (SCPOL|SCPH)
> * CTRLR0[ 6] Serial Clock Phase
> * CTRLR0[ 7] Serial Clock Polarity
> */
> - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
> - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
> + if (spi->mode & SPI_CPOL)
> + cr0 |= DW_PSSI_CTRLR0_SCPOL;
> + if (spi->mode & SPI_CPHA)
> + cr0 |= DW_PSSI_CTRLR0_SCPHA;
>
> /* CTRLR0[11] Shift Register Loop */
> - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
> + if (spi->mode & SPI_LOOP)
> + cr0 |= DW_PSSI_CTRLR0_SRL;
> } else {
> /* CTRLR0[ 7: 6] Frame Format */
> - cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
> + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
>
> /*
> * SPI mode (SCPOL|SCPH)
> * CTRLR0[ 8] Serial Clock Phase
> * CTRLR0[ 9] Serial Clock Polarity
> */
> - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
> - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
> + if (spi->mode & SPI_CPOL)
> + cr0 |= DW_HSSI_CTRLR0_SCPOL;
> + if (spi->mode & SPI_CPHA)
> + cr0 |= DW_HSSI_CTRLR0_SCPHA;
>
> /* CTRLR0[13] Shift Register Loop */
> - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
> + if (spi->mode & SPI_LOOP)
> + cr0 |= DW_HSSI_CTRLR0_SRL;
>
> if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> @@ -320,10 +327,10 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>
> if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
> /* CTRLR0[ 9:8] Transfer Mode */
> - cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
> + cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_TMOD_MASK, cfg->tmode);
> else
> /* CTRLR0[11:10] Transfer Mode */
> - cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
> + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
>
> dw_writel(dws, DW_SPI_CTRLR0, cr0);
>
> @@ -850,7 +857,7 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
>
> if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
> dws->caps |= DW_SPI_CAP_DFS32;
> - dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
> + dws->dfs_offset = __bf_shf(DW_PSSI_CTRLR0_DFS32_MASK);
> dev_dbg(dev, "Detected 32-bits max data frame size\n");
> }
> } else {
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 893b78c43a50..634085eadad1 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
Haven't deeply checked this, but below changes require to have bits.h
to be included.
Please, double check this is included already.
> @@ -41,39 +41,36 @@
> #define DW_SPI_CS_OVERRIDE 0xf4
>
> /* Bit fields in CTRLR0 (DWC APB SSI) */
> -#define DW_PSSI_CTRLR0_DFS_OFFSET 0
> #define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
> -#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
> +#define DW_PSSI_CTRLR0_DFS32_MASK GENMASK(20, 16)
>
> -#define DW_PSSI_CTRLR0_FRF_OFFSET 4
> +#define DW_PSSI_CTRLR0_FRF_MASK GENMASK(5, 4)
> #define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
> #define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
> #define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
> #define DW_SPI_CTRLR0_FRF_RESV 0x3
>
> -#define DW_PSSI_CTRLR0_MODE_OFFSET 6
> -#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
> -#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
> +#define DW_PSSI_CTRLR0_MODE_MASK GENMASK(7, 6)
> +#define DW_PSSI_CTRLR0_SCPHA BIT(6)
> +#define DW_PSSI_CTRLR0_SCPOL BIT(7)
>
> -#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
> -#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
> +#define DW_PSSI_CTRLR0_TMOD_MASK GENMASK(9, 8)
> #define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
> #define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
> #define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
> #define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
>
> -#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
> -#define DW_PSSI_CTRLR0_SRL_OFFSET 11
> -#define DW_PSSI_CTRLR0_CFS_OFFSET 12
> +#define DW_PSSI_CTRLR0_SLV_OE BIT(10)
> +#define DW_PSSI_CTRLR0_SRL BIT(11)
> +#define DW_PSSI_CTRLR0_CFS BIT(12)
>
> /* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
> -#define DW_HSSI_CTRLR0_SRL_OFFSET 13
> -#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
> +#define DW_HSSI_CTRLR0_DFS_MASK GENMASK(4, 0)
> +#define DW_HSSI_CTRLR0_FRF_MASK GENMASK(7, 6)
> +#define DW_HSSI_CTRLR0_SCPHA BIT(8)
> +#define DW_HSSI_CTRLR0_SCPOL BIT(9)
> #define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> -#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
> -#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
> -#define DW_HSSI_CTRLR0_FRF_OFFSET 6
> -#define DW_HSSI_CTRLR0_DFS_OFFSET 0
> +#define DW_HSSI_CTRLR0_SRL BIT(13)
>
> /*
> * For Keem Bay, CTRLR0[31] is used to select controller mode.
> @@ -86,26 +83,27 @@
> #define DW_SPI_NDF_MASK GENMASK(15, 0)
>
> /* Bit fields in SR, 7 bits */
> -#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
> -#define DW_SPI_SR_BUSY (1 << 0)
> -#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
> -#define DW_SPI_SR_TF_EMPT (1 << 2)
> -#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
> -#define DW_SPI_SR_RF_FULL (1 << 4)
> -#define DW_SPI_SR_TX_ERR (1 << 5)
> -#define DW_SPI_SR_DCOL (1 << 6)
> +#define DW_SPI_SR_MASK GENMASK(6, 0)
> +#define DW_SPI_SR_BUSY BIT(0)
> +#define DW_SPI_SR_TF_NOT_FULL BIT(1)
> +#define DW_SPI_SR_TF_EMPT BIT(2)
> +#define DW_SPI_SR_RF_NOT_EMPT BIT(3)
> +#define DW_SPI_SR_RF_FULL BIT(4)
> +#define DW_SPI_SR_TX_ERR BIT(5)
> +#define DW_SPI_SR_DCOL BIT(6)
>
> /* Bit fields in ISR, IMR, RISR, 7 bits */
> -#define DW_SPI_INT_TXEI (1 << 0)
> -#define DW_SPI_INT_TXOI (1 << 1)
> -#define DW_SPI_INT_RXUI (1 << 2)
> -#define DW_SPI_INT_RXOI (1 << 3)
> -#define DW_SPI_INT_RXFI (1 << 4)
> -#define DW_SPI_INT_MSTI (1 << 5)
> +#define DW_SPI_INT_MASK GENMASK(5, 0)
> +#define DW_SPI_INT_TXEI BIT(0)
> +#define DW_SPI_INT_TXOI BIT(1)
> +#define DW_SPI_INT_RXUI BIT(2)
> +#define DW_SPI_INT_RXOI BIT(3)
> +#define DW_SPI_INT_RXFI BIT(4)
> +#define DW_SPI_INT_MSTI BIT(5)
>
> /* Bit fields in DMACR */
> -#define DW_SPI_DMACR_RDMAE (1 << 0)
> -#define DW_SPI_DMACR_TDMAE (1 << 1)
> +#define DW_SPI_DMACR_RDMAE BIT(0)
> +#define DW_SPI_DMACR_TDMAE BIT(1)
>
> /* Mem/DMA operations helpers */
> #define DW_SPI_WAIT_RETRIES 5
> --
> 2.33.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-12 20:49 ` [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing Serge Semin
@ 2021-11-12 21:27 ` Andy Shevchenko
2021-11-12 21:37 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-12 21:27 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> Each Synopsys DWC SSI controller is equipped with a Synopsys Component
> version register, which encodes a version ID of an IP-core the
> controller has been synthesized from. That can be useful in future for the
> version-based conditional features implementation in the driver.
>
> Note the component version is encoded as an ASCII string so we need to
> convert it from the string to a normal unsigned integer to be easily used
> then in the driver statements.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
> drivers/spi/spi-dw-core.c | 18 ++++++++++++++++++
> drivers/spi/spi-dw.h | 1 +
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index b4cbcd38eaba..1766a29ca790 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -823,6 +823,24 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
> {
> dw_spi_reset_chip(dws);
>
> + /*
> + * Retrieve the Synopsys component version if it hasn't been specified
> + * by the platform. Note the CoreKit version ID is encoded as a 4bytes
> + * ASCII string enclosed with '*' symbol.
> + */
> + if (!dws->ver) {
> + u32 comp;
> +
> + comp = dw_readl(dws, DW_SPI_VERSION);
> + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> +
> + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ",
> + dws->ver / 100, dws->ver % 100);
Oh là là, first you multiply then you divide in the same piece of code!
What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
we have %p4cc)
> + }
> +
> /*
> * Try to detect the FIFO depth if not set by interface driver,
> * the depth could be from 2 to 256 from HW spec
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 634085eadad1..d06857d8d173 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -149,6 +149,7 @@ struct dw_spi {
> u32 max_mem_freq; /* max mem-ops bus freq */
> u32 max_freq; /* max bus freq supported */
>
> + u32 ver; /* Synopsys component version */
> u32 caps; /* DW SPI capabilities */
>
> u32 reg_io_width; /* DR I/O width in bytes */
> --
> 2.33.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] spi: dw: Put the driver entities naming in order
2021-11-12 21:22 ` Andy Shevchenko
@ 2021-11-12 21:30 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2021-11-12 21:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
Hello Andy
On Fri, Nov 12, 2021 at 11:22:02PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 12, 2021 at 10:51 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > Mostly due to a long driver history it's methods and macro names look a
> > bit messy. In particularly that concerns the code their prefixes. A
> > biggest part of the driver functions and macros have got the dw_spi/DW_SPI
> > prefixes. But there are some entities which have been just
> > "spi_/SPI_"-prefixed. Especially that concerns the CSR and their fields
> > macro definitions. It makes the code harder to comprehend since such
> > methods and macros can be easily confused with the global SPI-subsystem
> > exports. In this case the only possible way to more or less quickly
> > distinguish one naming space from another is either by context or by the
> > argument type, which most of the times isn't that easy anyway. In addition
> > to that a new DW SSI IP-core support has been added in the framework of
> > commit e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi"), which
> > introduced a new set or macro-prefixes to describe CTRLR0-specific fields
> > and worsen the situation. Finally there are methods with
> > no DW SPI driver-reference prefix at all, that make the code reading even
> > harder. So in order to ease the driver hacking let's bring the code naming
> > to a common base:
> > 1) Each method is supposed to have "dw_spi_" prefix so to be easily
> > distinguished from the kernel API, e.g. SPI-subsystem methods and macros.
> > (Exception is the local implementation of the readl/writel methods since
> > being just the regspace accessors.)
> > 2) Each generically used macro should have DW_SPI_-prefix thus being
> > easily comprehended as the local driver definition.
> > 3) DW APB SSI and DW SSI specific macros should have prefixes as DW_PSSI_
>
> In the compatible strings the parameter has "assi" and not "pssi".
> What did I miss?
You missed none. It's my mistake. Leftover from patch v0. Thanks for the heads up.
I'll fix it in v2.
-Sergey
>
> > and DW_HSSI_ respectively so referring to the system buses they support
> > (APB and AHB similarly to the DT clocks naming like pclk, hclk).
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> >
> > ---
> >
> > Folks, any ideas of a better naming scheme especially for the DW APB SSI
> > and DW SSI specific macros are very welcome.
> > ---
> > drivers/spi/spi-dw-bt1.c | 8 +--
> > drivers/spi/spi-dw-core.c | 138 ++++++++++++++++++------------------
> > drivers/spi/spi-dw-dma.c | 50 ++++++-------
> > drivers/spi/spi-dw-mmio.c | 20 +++---
> > drivers/spi/spi-dw-pci.c | 59 ++++++++--------
> > drivers/spi/spi-dw.h | 145 +++++++++++++++++++-------------------
> > 6 files changed, 211 insertions(+), 209 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-bt1.c b/drivers/spi/spi-dw-bt1.c
> > index 5be6b7b80c21..0411088dc443 100644
> > --- a/drivers/spi/spi-dw-bt1.c
> > +++ b/drivers/spi/spi-dw-bt1.c
> > @@ -123,7 +123,7 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > len = min_t(size_t, len, dwsbt1->map_len - offs);
> >
> > /* Collect the controller configuration required by the operation */
> > - cfg.tmode = SPI_TMOD_EPROMREAD;
> > + cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> > cfg.dfs = 8;
> > cfg.ndf = 4;
> > cfg.freq = mem->spi->max_speed_hz;
> > @@ -131,13 +131,13 @@ static ssize_t dw_spi_bt1_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > /* Make sure the corresponding CS is de-asserted on transmission */
> > dw_spi_set_cs(mem->spi, false);
> >
> > - spi_enable_chip(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> >
> > dw_spi_update_config(dws, mem->spi, &cfg);
> >
> > - spi_umask_intr(dws, SPI_INT_RXFI);
> > + dw_spi_umask_intr(dws, DW_SPI_INT_RXFI);
> >
> > - spi_enable_chip(dws, 1);
> > + dw_spi_enable_chip(dws, 1);
> >
> > /*
> > * Enable the transparent mode of the System Boot Controller.
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index f5446d9c6f27..4d91ffb5c0d8 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -24,7 +24,7 @@
> > #endif
> >
> > /* Slave spi_device related */
> > -struct chip_data {
> > +struct dw_spi_chip_data {
> > u32 cr0;
> > u32 rx_sample_dly; /* RX sample delay */
> > };
> > @@ -109,7 +109,7 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
> > EXPORT_SYMBOL_GPL(dw_spi_set_cs);
> >
> > /* Return the max entries we can fill into tx fifo */
> > -static inline u32 tx_max(struct dw_spi *dws)
> > +static inline u32 dw_spi_tx_max(struct dw_spi *dws)
> > {
> > u32 tx_room, rxtx_gap;
> >
> > @@ -129,14 +129,14 @@ static inline u32 tx_max(struct dw_spi *dws)
> > }
> >
> > /* Return the max entries we should read out of rx fifo */
> > -static inline u32 rx_max(struct dw_spi *dws)
> > +static inline u32 dw_spi_rx_max(struct dw_spi *dws)
> > {
> > return min_t(u32, dws->rx_len, dw_readl(dws, DW_SPI_RXFLR));
> > }
> >
> > static void dw_writer(struct dw_spi *dws)
> > {
> > - u32 max = tx_max(dws);
> > + u32 max = dw_spi_tx_max(dws);
> > u32 txw = 0;
> >
> > while (max--) {
> > @@ -157,7 +157,7 @@ static void dw_writer(struct dw_spi *dws)
> >
> > static void dw_reader(struct dw_spi *dws)
> > {
> > - u32 max = rx_max(dws);
> > + u32 max = dw_spi_rx_max(dws);
> > u32 rxw;
> >
> > while (max--) {
> > @@ -186,24 +186,24 @@ int dw_spi_check_status(struct dw_spi *dws, bool raw)
> > else
> > irq_status = dw_readl(dws, DW_SPI_ISR);
> >
> > - if (irq_status & SPI_INT_RXOI) {
> > + if (irq_status & DW_SPI_INT_RXOI) {
> > dev_err(&dws->master->dev, "RX FIFO overflow detected\n");
> > ret = -EIO;
> > }
> >
> > - if (irq_status & SPI_INT_RXUI) {
> > + if (irq_status & DW_SPI_INT_RXUI) {
> > dev_err(&dws->master->dev, "RX FIFO underflow detected\n");
> > ret = -EIO;
> > }
> >
> > - if (irq_status & SPI_INT_TXOI) {
> > + if (irq_status & DW_SPI_INT_TXOI) {
> > dev_err(&dws->master->dev, "TX FIFO overflow detected\n");
> > ret = -EIO;
> > }
> >
> > /* Generically handle the erroneous situation */
> > if (ret) {
> > - spi_reset_chip(dws);
> > + dw_spi_reset_chip(dws);
> > if (dws->master->cur_msg)
> > dws->master->cur_msg->status = ret;
> > }
> > @@ -230,7 +230,7 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
> > */
> > dw_reader(dws);
> > if (!dws->rx_len) {
> > - spi_mask_intr(dws, 0xff);
> > + dw_spi_mask_intr(dws, 0xff);
> > spi_finalize_current_transfer(dws->master);
> > } else if (dws->rx_len <= dw_readl(dws, DW_SPI_RXFTLR)) {
> > dw_writel(dws, DW_SPI_RXFTLR, dws->rx_len - 1);
> > @@ -241,10 +241,10 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
> > * disabled after the data transmission is finished so not to
> > * have the TXE IRQ flood at the final stage of the transfer.
> > */
> > - if (irq_status & SPI_INT_TXEI) {
> > + if (irq_status & DW_SPI_INT_TXEI) {
> > dw_writer(dws);
> > if (!dws->tx_len)
> > - spi_mask_intr(dws, SPI_INT_TXEI);
> > + dw_spi_mask_intr(dws, DW_SPI_INT_TXEI);
> > }
> >
> > return IRQ_HANDLED;
> > @@ -260,7 +260,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
> > return IRQ_NONE;
> >
> > if (!master->cur_msg) {
> > - spi_mask_intr(dws, 0xff);
> > + dw_spi_mask_intr(dws, 0xff);
> > return IRQ_HANDLED;
> > }
> >
> > @@ -271,37 +271,37 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
> > {
> > u32 cr0 = 0;
> >
> > - if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
> > + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> > /* CTRLR0[ 5: 4] Frame Format */
> > - cr0 |= SPI_FRF_MOTO_SPI << SPI_FRF_OFFSET;
> > + cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
> >
> > /*
> > * SPI mode (SCPOL|SCPH)
> > * CTRLR0[ 6] Serial Clock Phase
> > * CTRLR0[ 7] Serial Clock Polarity
> > */
> > - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << SPI_SCOL_OFFSET;
> > - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << SPI_SCPH_OFFSET;
> > + cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
> > + cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
> >
> > /* CTRLR0[11] Shift Register Loop */
> > - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
> > + cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
> > } else {
> > /* CTRLR0[ 7: 6] Frame Format */
> > - cr0 |= SPI_FRF_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
> > + cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
> >
> > /*
> > * SPI mode (SCPOL|SCPH)
> > * CTRLR0[ 8] Serial Clock Phase
> > * CTRLR0[ 9] Serial Clock Polarity
> > */
> > - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DWC_SSI_CTRLR0_SCPOL_OFFSET;
> > - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DWC_SSI_CTRLR0_SCPH_OFFSET;
> > + cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
> > + cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
> >
> > /* CTRLR0[13] Shift Register Loop */
> > - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DWC_SSI_CTRLR0_SRL_OFFSET;
> > + cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
> >
> > if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > - cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> > + cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > }
> >
> > return cr0;
> > @@ -310,7 +310,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
> > void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> > struct dw_spi_cfg *cfg)
> > {
> > - struct chip_data *chip = spi_get_ctldata(spi);
> > + struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
> > u32 cr0 = chip->cr0;
> > u32 speed_hz;
> > u16 clk_div;
> > @@ -318,16 +318,17 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> > /* CTRLR0[ 4/3: 0] or CTRLR0[ 20: 16] Data Frame Size */
> > cr0 |= (cfg->dfs - 1) << dws->dfs_offset;
> >
> > - if (!(dws->caps & DW_SPI_CAP_DWC_SSI))
> > + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
> > /* CTRLR0[ 9:8] Transfer Mode */
> > - cr0 |= cfg->tmode << SPI_TMOD_OFFSET;
> > + cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
> > else
> > /* CTRLR0[11:10] Transfer Mode */
> > - cr0 |= cfg->tmode << DWC_SSI_CTRLR0_TMOD_OFFSET;
> > + cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
> >
> > dw_writel(dws, DW_SPI_CTRLR0, cr0);
> >
> > - if (cfg->tmode == SPI_TMOD_EPROMREAD || cfg->tmode == SPI_TMOD_RO)
> > + if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> > + cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
> > dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
> >
> > /* Note DW APB SSI clock divider doesn't support odd numbers */
> > @@ -335,7 +336,7 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> > speed_hz = dws->max_freq / clk_div;
> >
> > if (dws->current_freq != speed_hz) {
> > - spi_set_clk(dws, clk_div);
> > + dw_spi_set_clk(dws, clk_div);
> > dws->current_freq = speed_hz;
> > }
> >
> > @@ -363,9 +364,9 @@ static void dw_spi_irq_setup(struct dw_spi *dws)
> >
> > dws->transfer_handler = dw_spi_transfer_handler;
> >
> > - imask = SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI |
> > - SPI_INT_RXFI;
> > - spi_umask_intr(dws, imask);
> > + imask = DW_SPI_INT_TXEI | DW_SPI_INT_TXOI |
> > + DW_SPI_INT_RXUI | DW_SPI_INT_RXOI | DW_SPI_INT_RXFI;
> > + dw_spi_umask_intr(dws, imask);
> > }
> >
> > /*
> > @@ -405,11 +406,12 @@ static int dw_spi_poll_transfer(struct dw_spi *dws,
> > }
> >
> > static int dw_spi_transfer_one(struct spi_controller *master,
> > - struct spi_device *spi, struct spi_transfer *transfer)
> > + struct spi_device *spi,
> > + struct spi_transfer *transfer)
> > {
> > struct dw_spi *dws = spi_controller_get_devdata(master);
> > struct dw_spi_cfg cfg = {
> > - .tmode = SPI_TMOD_TR,
> > + .tmode = DW_SPI_CTRLR0_TMOD_TR,
> > .dfs = transfer->bits_per_word,
> > .freq = transfer->speed_hz,
> > };
> > @@ -425,7 +427,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> > /* Ensure the data above is visible for all CPUs */
> > smp_mb();
> >
> > - spi_enable_chip(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> >
> > dw_spi_update_config(dws, spi, &cfg);
> >
> > @@ -436,7 +438,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> > dws->dma_mapped = master->cur_msg_mapped;
> >
> > /* For poll mode just disable all interrupts */
> > - spi_mask_intr(dws, 0xff);
> > + dw_spi_mask_intr(dws, 0xff);
> >
> > if (dws->dma_mapped) {
> > ret = dws->dma_ops->dma_setup(dws, transfer);
> > @@ -444,7 +446,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> > return ret;
> > }
> >
> > - spi_enable_chip(dws, 1);
> > + dw_spi_enable_chip(dws, 1);
> >
> > if (dws->dma_mapped)
> > return dws->dma_ops->dma_transfer(dws, transfer);
> > @@ -457,20 +459,20 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> > }
> >
> > static void dw_spi_handle_err(struct spi_controller *master,
> > - struct spi_message *msg)
> > + struct spi_message *msg)
> > {
> > struct dw_spi *dws = spi_controller_get_devdata(master);
> >
> > if (dws->dma_mapped)
> > dws->dma_ops->dma_stop(dws);
> >
> > - spi_reset_chip(dws);
> > + dw_spi_reset_chip(dws);
> > }
> >
> > static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> > {
> > if (op->data.dir == SPI_MEM_DATA_IN)
> > - op->data.nbytes = clamp_val(op->data.nbytes, 0, SPI_NDF_MASK + 1);
> > + op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);
> >
> > return 0;
> > }
> > @@ -498,7 +500,7 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
> > if (op->data.dir == SPI_MEM_DATA_OUT)
> > len += op->data.nbytes;
> >
> > - if (len <= SPI_BUF_SIZE) {
> > + if (len <= DW_SPI_BUF_SIZE) {
> > out = dws->buf;
> > } else {
> > out = kzalloc(len, GFP_KERNEL);
> > @@ -512,9 +514,9 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
> > * single buffer in order to speed the data transmission up.
> > */
> > for (i = 0; i < op->cmd.nbytes; ++i)
> > - out[i] = SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
> > + out[i] = DW_SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
> > for (j = 0; j < op->addr.nbytes; ++i, ++j)
> > - out[i] = SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> > + out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> > for (j = 0; j < op->dummy.nbytes; ++i, ++j)
> > out[i] = 0x0;
> >
> > @@ -587,7 +589,7 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
> > entries = readl_relaxed(dws->regs + DW_SPI_RXFLR);
> > if (!entries) {
> > sts = readl_relaxed(dws->regs + DW_SPI_RISR);
> > - if (sts & SPI_INT_RXOI) {
> > + if (sts & DW_SPI_INT_RXOI) {
> > dev_err(&dws->master->dev, "FIFO overflow on Rx\n");
> > return -EIO;
> > }
> > @@ -603,12 +605,12 @@ static int dw_spi_write_then_read(struct dw_spi *dws, struct spi_device *spi)
> >
> > static inline bool dw_spi_ctlr_busy(struct dw_spi *dws)
> > {
> > - return dw_readl(dws, DW_SPI_SR) & SR_BUSY;
> > + return dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_BUSY;
> > }
> >
> > static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
> > {
> > - int retry = SPI_WAIT_RETRIES;
> > + int retry = DW_SPI_WAIT_RETRIES;
> > struct spi_delay delay;
> > unsigned long ns, us;
> > u32 nents;
> > @@ -638,9 +640,9 @@ static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
> >
> > static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
> > {
> > - spi_enable_chip(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> > dw_spi_set_cs(spi, true);
> > - spi_enable_chip(dws, 1);
> > + dw_spi_enable_chip(dws, 1);
> > }
> >
> > /*
> > @@ -673,19 +675,19 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> > cfg.dfs = 8;
> > cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
> > if (op->data.dir == SPI_MEM_DATA_IN) {
> > - cfg.tmode = SPI_TMOD_EPROMREAD;
> > + cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> > cfg.ndf = op->data.nbytes;
> > } else {
> > - cfg.tmode = SPI_TMOD_TO;
> > + cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
> > }
> >
> > - spi_enable_chip(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> >
> > dw_spi_update_config(dws, mem->spi, &cfg);
> >
> > - spi_mask_intr(dws, 0xff);
> > + dw_spi_mask_intr(dws, 0xff);
> >
> > - spi_enable_chip(dws, 1);
> > + dw_spi_enable_chip(dws, 1);
> >
> > /*
> > * DW APB SSI controller has very nasty peculiarities. First originally
> > @@ -768,7 +770,7 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
> > static int dw_spi_setup(struct spi_device *spi)
> > {
> > struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
> > - struct chip_data *chip;
> > + struct dw_spi_chip_data *chip;
> >
> > /* Only alloc on first setup */
> > chip = spi_get_ctldata(spi);
> > @@ -776,7 +778,7 @@ static int dw_spi_setup(struct spi_device *spi)
> > struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
> > u32 rx_sample_dly_ns;
> >
> > - chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
> > + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> > if (!chip)
> > return -ENOMEM;
> > spi_set_ctldata(spi, chip);
> > @@ -803,16 +805,16 @@ static int dw_spi_setup(struct spi_device *spi)
> >
> > static void dw_spi_cleanup(struct spi_device *spi)
> > {
> > - struct chip_data *chip = spi_get_ctldata(spi);
> > + struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
> >
> > kfree(chip);
> > spi_set_ctldata(spi, NULL);
> > }
> >
> > /* Restart the controller, disable all interrupts, clean rx fifo */
> > -static void spi_hw_init(struct device *dev, struct dw_spi *dws)
> > +static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
> > {
> > - spi_reset_chip(dws);
> > + dw_spi_reset_chip(dws);
> >
> > /*
> > * Try to detect the FIFO depth if not set by interface driver,
> > @@ -837,18 +839,18 @@ static void spi_hw_init(struct device *dev, struct dw_spi *dws)
> > * writability. Note DWC SSI controller also has the extended DFS, but
> > * with zero offset.
> > */
> > - if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
> > + if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> > u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0);
> >
> > - spi_enable_chip(dws, 0);
> > + dw_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);
> > + dw_spi_enable_chip(dws, 1);
> >
> > - if (!(cr0 & SPI_DFS_MASK)) {
> > + if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
> > dws->caps |= DW_SPI_CAP_DFS32;
> > - dws->dfs_offset = SPI_DFS32_OFFSET;
> > + dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
> > dev_dbg(dev, "Detected 32-bits max data frame size\n");
> > }
> > } else {
> > @@ -878,7 +880,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> > spi_controller_set_devdata(master, dws);
> >
> > /* Basic HW init */
> > - spi_hw_init(dev, dws);
> > + dw_spi_hw_init(dev, dws);
> >
> > ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
> > master);
> > @@ -939,7 +941,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> > err_dma_exit:
> > if (dws->dma_ops && dws->dma_ops->dma_exit)
> > dws->dma_ops->dma_exit(dws);
> > - spi_enable_chip(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> > free_irq(dws->irq, master);
> > err_free_master:
> > spi_controller_put(master);
> > @@ -956,7 +958,7 @@ void dw_spi_remove_host(struct dw_spi *dws)
> > if (dws->dma_ops && dws->dma_ops->dma_exit)
> > dws->dma_ops->dma_exit(dws);
> >
> > - spi_shutdown_chip(dws);
> > + dw_spi_shutdown_chip(dws);
> >
> > free_irq(dws->irq, dws->master);
> > }
> > @@ -970,14 +972,14 @@ int dw_spi_suspend_host(struct dw_spi *dws)
> > if (ret)
> > return ret;
> >
> > - spi_shutdown_chip(dws);
> > + dw_spi_shutdown_chip(dws);
> > return 0;
> > }
> > EXPORT_SYMBOL_GPL(dw_spi_suspend_host);
> >
> > int dw_spi_resume_host(struct dw_spi *dws)
> > {
> > - spi_hw_init(&dws->master->dev, dws);
> > + dw_spi_hw_init(&dws->master->dev, dws);
> > return spi_controller_resume(dws->master);
> > }
> > EXPORT_SYMBOL_GPL(dw_spi_resume_host);
> > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > index a09831c62192..fd6a2154d2ce 100644
> > --- a/drivers/spi/spi-dw-dma.c
> > +++ b/drivers/spi/spi-dw-dma.c
> > @@ -17,10 +17,10 @@
> >
> > #include "spi-dw.h"
> >
> > -#define RX_BUSY 0
> > -#define RX_BURST_LEVEL 16
> > -#define TX_BUSY 1
> > -#define TX_BURST_LEVEL 16
> > +#define DW_SPI_RX_BUSY 0
> > +#define DW_SPI_RX_BURST_LEVEL 16
> > +#define DW_SPI_TX_BUSY 1
> > +#define DW_SPI_TX_BURST_LEVEL 16
> >
> > static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
> > {
> > @@ -45,7 +45,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> > if (!ret && caps.max_burst)
> > max_burst = caps.max_burst;
> > else
> > - max_burst = RX_BURST_LEVEL;
> > + max_burst = DW_SPI_RX_BURST_LEVEL;
> >
> > dws->rxburst = min(max_burst, def_burst);
> > dw_writel(dws, DW_SPI_DMARDLR, dws->rxburst - 1);
> > @@ -54,7 +54,7 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> > if (!ret && caps.max_burst)
> > max_burst = caps.max_burst;
> > else
> > - max_burst = TX_BURST_LEVEL;
> > + max_burst = DW_SPI_TX_BURST_LEVEL;
> >
> > /*
> > * Having a Rx DMA channel serviced with higher priority than a Tx DMA
> > @@ -226,13 +226,13 @@ static int dw_spi_dma_wait(struct dw_spi *dws, unsigned int len, u32 speed)
> >
> > static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
> > {
> > - return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
> > + return !(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_TF_EMPT);
> > }
> >
> > static int dw_spi_dma_wait_tx_done(struct dw_spi *dws,
> > struct spi_transfer *xfer)
> > {
> > - int retry = SPI_WAIT_RETRIES;
> > + int retry = DW_SPI_WAIT_RETRIES;
> > struct spi_delay delay;
> > u32 nents;
> >
> > @@ -259,8 +259,8 @@ static void dw_spi_dma_tx_done(void *arg)
> > {
> > struct dw_spi *dws = arg;
> >
> > - clear_bit(TX_BUSY, &dws->dma_chan_busy);
> > - if (test_bit(RX_BUSY, &dws->dma_chan_busy))
> > + clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
> > + if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy))
> > return;
> >
> > complete(&dws->dma_completion);
> > @@ -304,19 +304,19 @@ static int dw_spi_dma_submit_tx(struct dw_spi *dws, struct scatterlist *sgl,
> > return ret;
> > }
> >
> > - set_bit(TX_BUSY, &dws->dma_chan_busy);
> > + set_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
> >
> > return 0;
> > }
> >
> > static inline bool dw_spi_dma_rx_busy(struct dw_spi *dws)
> > {
> > - return !!(dw_readl(dws, DW_SPI_SR) & SR_RF_NOT_EMPT);
> > + return !!(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_RF_NOT_EMPT);
> > }
> >
> > static int dw_spi_dma_wait_rx_done(struct dw_spi *dws)
> > {
> > - int retry = SPI_WAIT_RETRIES;
> > + int retry = DW_SPI_WAIT_RETRIES;
> > struct spi_delay delay;
> > unsigned long ns, us;
> > u32 nents;
> > @@ -360,8 +360,8 @@ static void dw_spi_dma_rx_done(void *arg)
> > {
> > struct dw_spi *dws = arg;
> >
> > - clear_bit(RX_BUSY, &dws->dma_chan_busy);
> > - if (test_bit(TX_BUSY, &dws->dma_chan_busy))
> > + clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
> > + if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy))
> > return;
> >
> > complete(&dws->dma_completion);
> > @@ -405,7 +405,7 @@ static int dw_spi_dma_submit_rx(struct dw_spi *dws, struct scatterlist *sgl,
> > return ret;
> > }
> >
> > - set_bit(RX_BUSY, &dws->dma_chan_busy);
> > + set_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
> >
> > return 0;
> > }
> > @@ -430,16 +430,16 @@ static int dw_spi_dma_setup(struct dw_spi *dws, struct spi_transfer *xfer)
> > }
> >
> > /* Set the DMA handshaking interface */
> > - dma_ctrl = SPI_DMA_TDMAE;
> > + dma_ctrl = DW_SPI_DMACR_TDMAE;
> > if (xfer->rx_buf)
> > - dma_ctrl |= SPI_DMA_RDMAE;
> > + dma_ctrl |= DW_SPI_DMACR_RDMAE;
> > dw_writel(dws, DW_SPI_DMACR, dma_ctrl);
> >
> > /* Set the interrupt mask */
> > - imr = SPI_INT_TXOI;
> > + imr = DW_SPI_INT_TXOI;
> > if (xfer->rx_buf)
> > - imr |= SPI_INT_RXUI | SPI_INT_RXOI;
> > - spi_umask_intr(dws, imr);
> > + imr |= DW_SPI_INT_RXUI | DW_SPI_INT_RXOI;
> > + dw_spi_umask_intr(dws, imr);
> >
> > reinit_completion(&dws->dma_completion);
> >
> > @@ -615,13 +615,13 @@ static int dw_spi_dma_transfer(struct dw_spi *dws, struct spi_transfer *xfer)
> >
> > static void dw_spi_dma_stop(struct dw_spi *dws)
> > {
> > - if (test_bit(TX_BUSY, &dws->dma_chan_busy)) {
> > + if (test_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy)) {
> > dmaengine_terminate_sync(dws->txchan);
> > - clear_bit(TX_BUSY, &dws->dma_chan_busy);
> > + clear_bit(DW_SPI_TX_BUSY, &dws->dma_chan_busy);
> > }
> > - if (test_bit(RX_BUSY, &dws->dma_chan_busy)) {
> > + if (test_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy)) {
> > dmaengine_terminate_sync(dws->rxchan);
> > - clear_bit(RX_BUSY, &dws->dma_chan_busy);
> > + clear_bit(DW_SPI_RX_BUSY, &dws->dma_chan_busy);
> > }
> > }
> >
> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > index 17c06039a74d..435c91aecbca 100644
> > --- a/drivers/spi/spi-dw-mmio.c
> > +++ b/drivers/spi/spi-dw-mmio.c
> > @@ -196,18 +196,18 @@ static int dw_spi_alpine_init(struct platform_device *pdev,
> > return 0;
> > }
> >
> > -static int dw_spi_dw_apb_init(struct platform_device *pdev,
> > - struct dw_spi_mmio *dwsmmio)
> > +static int dw_spi_assi_init(struct platform_device *pdev,
> > + struct dw_spi_mmio *dwsmmio)
> > {
> > dw_spi_dma_setup_generic(&dwsmmio->dws);
> >
> > return 0;
> > }
> >
> > -static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
> > - struct dw_spi_mmio *dwsmmio)
> > +static int dw_spi_hssi_init(struct platform_device *pdev,
> > + struct dw_spi_mmio *dwsmmio)
> > {
> > - dwsmmio->dws.caps = DW_SPI_CAP_DWC_SSI;
> > + dwsmmio->dws.caps = DW_SPI_CAP_DWC_HSSI;
> >
> > dw_spi_dma_setup_generic(&dwsmmio->dws);
> >
> > @@ -217,7 +217,7 @@ static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
> > static int dw_spi_keembay_init(struct platform_device *pdev,
> > struct dw_spi_mmio *dwsmmio)
> > {
> > - dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_SSI;
> > + dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_HSSI;
> >
> > return 0;
> > }
> > @@ -342,12 +342,12 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> > }
> >
> > static const struct of_device_id dw_spi_mmio_of_match[] = {
> > - { .compatible = "snps,dw-apb-ssi", .data = dw_spi_dw_apb_init},
> > + { .compatible = "snps,dw-apb-ssi", .data = dw_spi_assi_init},
> > { .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
> > { .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
> > { .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
> > - { .compatible = "renesas,rzn1-spi", .data = dw_spi_dw_apb_init},
> > - { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
> > + { .compatible = "renesas,rzn1-spi", .data = dw_spi_assi_init},
> > + { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_hssi_init},
> > { .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
> > { .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> > { .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> > @@ -357,7 +357,7 @@ MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> >
> > #ifdef CONFIG_ACPI
> > static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
> > - {"HISI0173", (kernel_ulong_t)dw_spi_dw_apb_init},
> > + {"HISI0173", (kernel_ulong_t)dw_spi_assi_init},
> > {},
> > };
> > MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);
> > diff --git a/drivers/spi/spi-dw-pci.c b/drivers/spi/spi-dw-pci.c
> > index 8a91cd58102f..e4a239bc3d36 100644
> > --- a/drivers/spi/spi-dw-pci.c
> > +++ b/drivers/spi/spi-dw-pci.c
> > @@ -24,14 +24,14 @@
> > #define CLK_SPI_CDIV_MASK 0x00000e00
> > #define CLK_SPI_DISABLE_OFFSET 8
> >
> > -struct spi_pci_desc {
> > +struct dw_spi_pci_desc {
> > int (*setup)(struct dw_spi *);
> > u16 num_cs;
> > u16 bus_num;
> > u32 max_freq;
> > };
> >
> > -static int spi_mid_init(struct dw_spi *dws)
> > +static int dw_spi_pci_mid_init(struct dw_spi *dws)
> > {
> > void __iomem *clk_reg;
> > u32 clk_cdiv;
> > @@ -53,36 +53,36 @@ static int spi_mid_init(struct dw_spi *dws)
> > return 0;
> > }
> >
> > -static int spi_generic_init(struct dw_spi *dws)
> > +static int dw_spi_pci_generic_init(struct dw_spi *dws)
> > {
> > dw_spi_dma_setup_generic(dws);
> >
> > return 0;
> > }
> >
> > -static struct spi_pci_desc spi_pci_mid_desc_1 = {
> > - .setup = spi_mid_init,
> > +static struct dw_spi_pci_desc dw_spi_pci_mid_desc_1 = {
> > + .setup = dw_spi_pci_mid_init,
> > .num_cs = 5,
> > .bus_num = 0,
> > };
> >
> > -static struct spi_pci_desc spi_pci_mid_desc_2 = {
> > - .setup = spi_mid_init,
> > +static struct dw_spi_pci_desc dw_spi_pci_mid_desc_2 = {
> > + .setup = dw_spi_pci_mid_init,
> > .num_cs = 2,
> > .bus_num = 1,
> > };
> >
> > -static struct spi_pci_desc spi_pci_ehl_desc = {
> > - .setup = spi_generic_init,
> > +static struct dw_spi_pci_desc dw_spi_pci_ehl_desc = {
> > + .setup = dw_spi_pci_generic_init,
> > .num_cs = 2,
> > .bus_num = -1,
> > .max_freq = 100000000,
> > };
> >
> > -static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > +static int dw_spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > {
> > + struct dw_spi_pci_desc *desc = (struct dw_spi_pci_desc *)ent->driver_data;
> > struct dw_spi *dws;
> > - struct spi_pci_desc *desc = (struct spi_pci_desc *)ent->driver_data;
> > int pci_bar = 0;
> > int ret;
> >
> > @@ -150,7 +150,7 @@ static int spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > return ret;
> > }
> >
> > -static void spi_pci_remove(struct pci_dev *pdev)
> > +static void dw_spi_pci_remove(struct pci_dev *pdev)
> > {
> > struct dw_spi *dws = pci_get_drvdata(pdev);
> >
> > @@ -162,14 +162,14 @@ static void spi_pci_remove(struct pci_dev *pdev)
> > }
> >
> > #ifdef CONFIG_PM_SLEEP
> > -static int spi_suspend(struct device *dev)
> > +static int dw_spi_pci_suspend(struct device *dev)
> > {
> > struct dw_spi *dws = dev_get_drvdata(dev);
> >
> > return dw_spi_suspend_host(dws);
> > }
> >
> > -static int spi_resume(struct device *dev)
> > +static int dw_spi_pci_resume(struct device *dev)
> > {
> > struct dw_spi *dws = dev_get_drvdata(dev);
> >
> > @@ -177,38 +177,37 @@ static int spi_resume(struct device *dev)
> > }
> > #endif
> >
> > -static SIMPLE_DEV_PM_OPS(dw_spi_pm_ops, spi_suspend, spi_resume);
> > +static SIMPLE_DEV_PM_OPS(dw_spi_pci_pm_ops, dw_spi_pci_suspend, dw_spi_pci_resume);
> >
> > -static const struct pci_device_id pci_ids[] = {
> > +static const struct pci_device_id dw_spi_pci_ids[] = {
> > /* Intel MID platform SPI controller 0 */
> > /*
> > * The access to the device 8086:0801 is disabled by HW, since it's
> > * exclusively used by SCU to communicate with MSIC.
> > */
> > /* Intel MID platform SPI controller 1 */
> > - { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&spi_pci_mid_desc_1},
> > + { PCI_VDEVICE(INTEL, 0x0800), (kernel_ulong_t)&dw_spi_pci_mid_desc_1},
> > /* Intel MID platform SPI controller 2 */
> > - { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&spi_pci_mid_desc_2},
> > + { PCI_VDEVICE(INTEL, 0x0812), (kernel_ulong_t)&dw_spi_pci_mid_desc_2},
> > /* Intel Elkhart Lake PSE SPI controllers */
> > - { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&spi_pci_ehl_desc},
> > - { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&spi_pci_ehl_desc},
> > - { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&spi_pci_ehl_desc},
> > - { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&spi_pci_ehl_desc},
> > + { PCI_VDEVICE(INTEL, 0x4b84), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> > + { PCI_VDEVICE(INTEL, 0x4b85), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> > + { PCI_VDEVICE(INTEL, 0x4b86), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> > + { PCI_VDEVICE(INTEL, 0x4b87), (kernel_ulong_t)&dw_spi_pci_ehl_desc},
> > {},
> > };
> > -MODULE_DEVICE_TABLE(pci, pci_ids);
> > +MODULE_DEVICE_TABLE(pci, dw_spi_pci_ids);
> >
> > -static struct pci_driver dw_spi_driver = {
> > +static struct pci_driver dw_spi_pci_driver = {
> > .name = DRIVER_NAME,
> > - .id_table = pci_ids,
> > - .probe = spi_pci_probe,
> > - .remove = spi_pci_remove,
> > + .id_table = dw_spi_pci_ids,
> > + .probe = dw_spi_pci_probe,
> > + .remove = dw_spi_pci_remove,
> > .driver = {
> > - .pm = &dw_spi_pm_ops,
> > + .pm = &dw_spi_pci_pm_ops,
> > },
> > };
> > -
> > -module_pci_driver(dw_spi_driver);
> > +module_pci_driver(dw_spi_pci_driver);
> >
> > MODULE_AUTHOR("Feng Tang <feng.tang@intel.com>");
> > MODULE_DESCRIPTION("PCI interface driver for DW SPI Core");
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> > index 467c342bfe56..893b78c43a50 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
> > @@ -1,6 +1,6 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > -#ifndef DW_SPI_HEADER_H
> > -#define DW_SPI_HEADER_H
> > +#ifndef __SPI_DW_H__
> > +#define __SPI_DW_H__
> >
> > #include <linux/bits.h>
> > #include <linux/completion.h>
> > @@ -11,7 +11,7 @@
> > #include <linux/spi/spi-mem.h>
> > #include <linux/bitfield.h>
> >
> > -/* Register offsets */
> > +/* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
> > #define DW_SPI_CTRLR0 0x00
> > #define DW_SPI_CTRLR1 0x04
> > #define DW_SPI_SSIENR 0x08
> > @@ -40,84 +40,85 @@
> > #define DW_SPI_RX_SAMPLE_DLY 0xf0
> > #define DW_SPI_CS_OVERRIDE 0xf4
> >
> > -/* 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_MOTO_SPI 0x0
> > -#define SPI_FRF_TI_SSP 0x1
> > -#define SPI_FRF_NS_MICROWIRE 0x2
> > -#define SPI_FRF_RESV 0x3
> > -
> > -#define SPI_MODE_OFFSET 6
> > -#define SPI_SCPH_OFFSET 6
> > -#define SPI_SCOL_OFFSET 7
> > -
> > -#define SPI_TMOD_OFFSET 8
> > -#define SPI_TMOD_MASK (0x3 << SPI_TMOD_OFFSET)
> > -#define SPI_TMOD_TR 0x0 /* xmit & recv */
> > -#define SPI_TMOD_TO 0x1 /* xmit only */
> > -#define SPI_TMOD_RO 0x2 /* recv only */
> > -#define SPI_TMOD_EPROMREAD 0x3 /* eeprom read mode */
> > -
> > -#define SPI_SLVOE_OFFSET 10
> > -#define SPI_SRL_OFFSET 11
> > -#define SPI_CFS_OFFSET 12
> > -
> > -/* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
> > -#define DWC_SSI_CTRLR0_SRL_OFFSET 13
> > -#define DWC_SSI_CTRLR0_TMOD_OFFSET 10
> > -#define DWC_SSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> > -#define DWC_SSI_CTRLR0_SCPOL_OFFSET 9
> > -#define DWC_SSI_CTRLR0_SCPH_OFFSET 8
> > -#define DWC_SSI_CTRLR0_FRF_OFFSET 6
> > -#define DWC_SSI_CTRLR0_DFS_OFFSET 0
> > +/* Bit fields in CTRLR0 (DWC APB SSI) */
> > +#define DW_PSSI_CTRLR0_DFS_OFFSET 0
> > +#define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
> > +#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
> > +
> > +#define DW_PSSI_CTRLR0_FRF_OFFSET 4
> > +#define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
> > +#define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
> > +#define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
> > +#define DW_SPI_CTRLR0_FRF_RESV 0x3
> > +
> > +#define DW_PSSI_CTRLR0_MODE_OFFSET 6
> > +#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
> > +#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
> > +
> > +#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
> > +#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
> > +#define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
> > +#define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
> > +#define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
> > +#define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
> > +
> > +#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
> > +#define DW_PSSI_CTRLR0_SRL_OFFSET 11
> > +#define DW_PSSI_CTRLR0_CFS_OFFSET 12
> > +
> > +/* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
> > +#define DW_HSSI_CTRLR0_SRL_OFFSET 13
> > +#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
> > +#define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> > +#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
> > +#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
> > +#define DW_HSSI_CTRLR0_FRF_OFFSET 6
> > +#define DW_HSSI_CTRLR0_DFS_OFFSET 0
> >
> > /*
> > * For Keem Bay, CTRLR0[31] is used to select controller mode.
> > * 0: SSI is slave
> > * 1: SSI is master
> > */
> > -#define DWC_SSI_CTRLR0_KEEMBAY_MST BIT(31)
> > +#define DW_HSSI_CTRLR0_KEEMBAY_MST BIT(31)
> >
> > /* Bit fields in CTRLR1 */
> > -#define SPI_NDF_MASK GENMASK(15, 0)
> > +#define DW_SPI_NDF_MASK GENMASK(15, 0)
> >
> > /* Bit fields in SR, 7 bits */
> > -#define SR_MASK 0x7f /* cover 7 bits */
> > -#define SR_BUSY (1 << 0)
> > -#define SR_TF_NOT_FULL (1 << 1)
> > -#define SR_TF_EMPT (1 << 2)
> > -#define SR_RF_NOT_EMPT (1 << 3)
> > -#define SR_RF_FULL (1 << 4)
> > -#define SR_TX_ERR (1 << 5)
> > -#define SR_DCOL (1 << 6)
> > +#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
> > +#define DW_SPI_SR_BUSY (1 << 0)
> > +#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
> > +#define DW_SPI_SR_TF_EMPT (1 << 2)
> > +#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
> > +#define DW_SPI_SR_RF_FULL (1 << 4)
> > +#define DW_SPI_SR_TX_ERR (1 << 5)
> > +#define DW_SPI_SR_DCOL (1 << 6)
> >
> > /* Bit fields in ISR, IMR, RISR, 7 bits */
> > -#define SPI_INT_TXEI (1 << 0)
> > -#define SPI_INT_TXOI (1 << 1)
> > -#define SPI_INT_RXUI (1 << 2)
> > -#define SPI_INT_RXOI (1 << 3)
> > -#define SPI_INT_RXFI (1 << 4)
> > -#define SPI_INT_MSTI (1 << 5)
> > +#define DW_SPI_INT_TXEI (1 << 0)
> > +#define DW_SPI_INT_TXOI (1 << 1)
> > +#define DW_SPI_INT_RXUI (1 << 2)
> > +#define DW_SPI_INT_RXOI (1 << 3)
> > +#define DW_SPI_INT_RXFI (1 << 4)
> > +#define DW_SPI_INT_MSTI (1 << 5)
> >
> > /* Bit fields in DMACR */
> > -#define SPI_DMA_RDMAE (1 << 0)
> > -#define SPI_DMA_TDMAE (1 << 1)
> > +#define DW_SPI_DMACR_RDMAE (1 << 0)
> > +#define DW_SPI_DMACR_TDMAE (1 << 1)
> >
> > -#define SPI_WAIT_RETRIES 5
> > -#define SPI_BUF_SIZE \
> > +/* Mem/DMA operations helpers */
> > +#define DW_SPI_WAIT_RETRIES 5
> > +#define DW_SPI_BUF_SIZE \
> > (sizeof_field(struct spi_mem_op, cmd.opcode) + \
> > sizeof_field(struct spi_mem_op, addr.val) + 256)
> > -#define SPI_GET_BYTE(_val, _idx) \
> > +#define DW_SPI_GET_BYTE(_val, _idx) \
> > ((_val) >> (BITS_PER_BYTE * (_idx)) & 0xff)
> >
> > /* DW SPI capabilities */
> > #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_HSSI BIT(2)
> > #define DW_SPI_CAP_DFS32 BIT(3)
> >
> > /* Slave spi_transfer/spi_mem_op related */
> > @@ -162,7 +163,7 @@ struct dw_spi {
> > unsigned int tx_len;
> > void *rx;
> > unsigned int rx_len;
> > - u8 buf[SPI_BUF_SIZE];
> > + u8 buf[DW_SPI_BUF_SIZE];
> > int dma_mapped;
> > u8 n_bytes; /* current is a 1/2 bytes op */
> > irqreturn_t (*transfer_handler)(struct dw_spi *dws);
> > @@ -224,18 +225,18 @@ static inline void dw_write_io_reg(struct dw_spi *dws, u32 offset, u32 val)
> > }
> > }
> >
> > -static inline void spi_enable_chip(struct dw_spi *dws, int enable)
> > +static inline void dw_spi_enable_chip(struct dw_spi *dws, int enable)
> > {
> > dw_writel(dws, DW_SPI_SSIENR, (enable ? 1 : 0));
> > }
> >
> > -static inline void spi_set_clk(struct dw_spi *dws, u16 div)
> > +static inline void dw_spi_set_clk(struct dw_spi *dws, u16 div)
> > {
> > dw_writel(dws, DW_SPI_BAUDR, div);
> > }
> >
> > /* Disable IRQ bits */
> > -static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
> > +static inline void dw_spi_mask_intr(struct dw_spi *dws, u32 mask)
> > {
> > u32 new_mask;
> >
> > @@ -244,7 +245,7 @@ static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
> > }
> >
> > /* Enable IRQ bits */
> > -static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
> > +static inline void dw_spi_umask_intr(struct dw_spi *dws, u32 mask)
> > {
> > u32 new_mask;
> >
> > @@ -257,19 +258,19 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
> > * and CS, then re-enables the controller back. Transmit and receive FIFO
> > * buffers are cleared when the device is disabled.
> > */
> > -static inline void spi_reset_chip(struct dw_spi *dws)
> > +static inline void dw_spi_reset_chip(struct dw_spi *dws)
> > {
> > - spi_enable_chip(dws, 0);
> > - spi_mask_intr(dws, 0xff);
> > + dw_spi_enable_chip(dws, 0);
> > + dw_spi_mask_intr(dws, 0xff);
> > dw_readl(dws, DW_SPI_ICR);
> > dw_writel(dws, DW_SPI_SER, 0);
> > - spi_enable_chip(dws, 1);
> > + dw_spi_enable_chip(dws, 1);
> > }
> >
> > -static inline void spi_shutdown_chip(struct dw_spi *dws)
> > +static inline void dw_spi_shutdown_chip(struct dw_spi *dws)
> > {
> > - spi_enable_chip(dws, 0);
> > - spi_set_clk(dws, 0);
> > + dw_spi_enable_chip(dws, 0);
> > + dw_spi_set_clk(dws, 0);
> > }
> >
> > extern void dw_spi_set_cs(struct spi_device *spi, bool enable);
> > @@ -293,4 +294,4 @@ static inline void dw_spi_dma_setup_generic(struct dw_spi *dws) {}
> >
> > #endif /* !CONFIG_SPI_DW_DMA */
> >
> > -#endif /* DW_SPI_HEADER_H */
> > +#endif /* __SPI_DW_H__ */
> > --
> > 2.33.0
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
` (3 preceding siblings ...)
2021-11-12 20:49 ` [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing Serge Semin
@ 2021-11-12 21:30 ` Andy Shevchenko
4 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-12 21:30 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 10:51 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> This patchset consists of the changes which I was going to introduce for a
> long time, but due to lack of free time couldn't make it so far.
> Nandhini's series [1] made me to proceed with this task so mate would
> finally have his patchset accepted and merged into the mainline kernel.
>
> There are three cleanup patches here and one feature patch. In framework
> of the former patches we suggest to better organize the code. In
> particular they concern the methods and macros naming unification (using a
> unified prefixes of the code object names) and CSR fields macro
> implementation using the bitfield helpers available in the kernel. The
> later patch introduces the Synopsys Component Version register parsing
> procedure so the corresponding data could be used for a version-specific
> features implementation. Nandhini will be mostly interested in the later
> patch in the framework of his series [1].
>
> Nandhini, could you please test the patchset out on your DWC SSI hardware?
> After it's merged into the spi/for-next branch of the Mark' repository you
> will be able to rebase your series on top of it and use the last patch
> functionality for your benefit.
Thanks!
I have no objection to the series, just a few nit-picks here and
there. Most important one is 4cc usage.
On top of that, please consider switching to use
EXPORT_SYMBOL_GPL_NS() (in a separate patch, perhaps as a
prerequisite).
Nevertheless,
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
And with addressed above
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> [1] https://lore.kernel.org/linux-spi/20211111065201.10249-4-nandhini.srikandan@intel.com/
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: linux-spi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> Serge Semin (4):
> spi: dw: Discard redundant DW SSI Frame Formats enumeration
> spi: dw: Put the driver entities naming in order
> spi: dw: Convert to using the Bitfield access macros
> spi: dw: Add Synopsys Component version reading and parsing
>
> drivers/spi/spi-dw-bt1.c | 8 +-
> drivers/spi/spi-dw-core.c | 165 ++++++++++++++++++++++----------------
> drivers/spi/spi-dw-dma.c | 50 ++++++------
> drivers/spi/spi-dw-mmio.c | 20 ++---
> drivers/spi/spi-dw-pci.c | 59 +++++++-------
> drivers/spi/spi-dw.h | 150 +++++++++++++++++-----------------
> 6 files changed, 236 insertions(+), 216 deletions(-)
>
> --
> 2.33.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-12 21:27 ` Andy Shevchenko
@ 2021-11-12 21:37 ` Andy Shevchenko
2021-11-12 22:05 ` Serge Semin
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-12 21:37 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> > + /*
> > + * Retrieve the Synopsys component version if it hasn't been specified
> > + * by the platform. Note the CoreKit version ID is encoded as a 4bytes
> > + * ASCII string enclosed with '*' symbol.
> > + */
> > + if (!dws->ver) {
> > + u32 comp;
> > +
> > + comp = dw_readl(dws, DW_SPI_VERSION);
> > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> > +
> > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ",
> > + dws->ver / 100, dws->ver % 100);
>
> Oh là là, first you multiply then you divide in the same piece of code!
> What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
> we have %p4cc)
>
> > + }
Have you seen this, btw?
https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_dwlib.c#L93
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros
2021-11-12 21:23 ` Andy Shevchenko
@ 2021-11-12 21:43 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2021-11-12 21:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 11:23:54PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 12, 2021 at 10:51 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > The driver has been using the offset/bitwise-shift-based approach for the
> > CSR fields R/W operations since it was merged into the kernel. It can be
> > simplified by using the macros defined in the linux/bitfield.h and
> > linux/bit.h header files like BIT(), GENMASK(), FIELD_PREP(), FIELD_GET(),
> > etc where it is required, for instance in the cached cr0 preparation
> > method. Thus in order to have the FIELD_*()-macros utilized we just need
> > to convert the macros with the CSR-fields offsets to the masks with the
> > corresponding registers fields definition. That's where the GENMASK() and
> > BIT() macros come in handy. After that the masks can be used in the
> > FIELD_*()-macros where it's appropriate.
> >
> > We also need to convert the macros with the CRS-bit flags using the manual
> > bitwise shift operations (x << y) to using the BIT() macro. Thus we'll
> > have a more coherent set of the CSR-related macros.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> > drivers/spi/spi-dw-core.c | 31 +++++++++++--------
> > drivers/spi/spi-dw.h | 64 +++++++++++++++++++--------------------
> > 2 files changed, 50 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index 4d91ffb5c0d8..b4cbcd38eaba 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -5,6 +5,7 @@
> > * Copyright (c) 2009, Intel Corporation.
> > */
> >
> > +#include <linux/bitfield.h>
> > #include <linux/dma-mapping.h>
> > #include <linux/interrupt.h>
> > #include <linux/module.h>
> > @@ -254,7 +255,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
> > {
> > struct spi_controller *master = dev_id;
> > struct dw_spi *dws = spi_controller_get_devdata(master);
> > - u16 irq_status = dw_readl(dws, DW_SPI_ISR) & 0x3f;
> > + u16 irq_status = dw_readl(dws, DW_SPI_ISR) & DW_SPI_INT_MASK;
> >
> > if (!irq_status)
> > return IRQ_NONE;
> > @@ -273,32 +274,38 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
> >
> > if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> > /* CTRLR0[ 5: 4] Frame Format */
> > - cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_PSSI_CTRLR0_FRF_OFFSET;
> > + cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
> >
> > /*
> > * SPI mode (SCPOL|SCPH)
> > * CTRLR0[ 6] Serial Clock Phase
> > * CTRLR0[ 7] Serial Clock Polarity
> > */
> > - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_PSSI_CTRLR0_SCOL_OFFSET;
> > - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_PSSI_CTRLR0_SCPH_OFFSET;
> > + if (spi->mode & SPI_CPOL)
> > + cr0 |= DW_PSSI_CTRLR0_SCPOL;
> > + if (spi->mode & SPI_CPHA)
> > + cr0 |= DW_PSSI_CTRLR0_SCPHA;
> >
> > /* CTRLR0[11] Shift Register Loop */
> > - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_PSSI_CTRLR0_SRL_OFFSET;
> > + if (spi->mode & SPI_LOOP)
> > + cr0 |= DW_PSSI_CTRLR0_SRL;
> > } else {
> > /* CTRLR0[ 7: 6] Frame Format */
> > - cr0 |= DW_SPI_CTRLR0_FRF_MOTO_SPI << DW_HSSI_CTRLR0_FRF_OFFSET;
> > + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
> >
> > /*
> > * SPI mode (SCPOL|SCPH)
> > * CTRLR0[ 8] Serial Clock Phase
> > * CTRLR0[ 9] Serial Clock Polarity
> > */
> > - cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DW_HSSI_CTRLR0_SCPOL_OFFSET;
> > - cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DW_HSSI_CTRLR0_SCPH_OFFSET;
> > + if (spi->mode & SPI_CPOL)
> > + cr0 |= DW_HSSI_CTRLR0_SCPOL;
> > + if (spi->mode & SPI_CPHA)
> > + cr0 |= DW_HSSI_CTRLR0_SCPHA;
> >
> > /* CTRLR0[13] Shift Register Loop */
> > - cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DW_HSSI_CTRLR0_SRL_OFFSET;
> > + if (spi->mode & SPI_LOOP)
> > + cr0 |= DW_HSSI_CTRLR0_SRL;
> >
> > if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > @@ -320,10 +327,10 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> >
> > if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
> > /* CTRLR0[ 9:8] Transfer Mode */
> > - cr0 |= cfg->tmode << DW_PSSI_CTRLR0_TMOD_OFFSET;
> > + cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_TMOD_MASK, cfg->tmode);
> > else
> > /* CTRLR0[11:10] Transfer Mode */
> > - cr0 |= cfg->tmode << DW_HSSI_CTRLR0_TMOD_OFFSET;
> > + cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
> >
> > dw_writel(dws, DW_SPI_CTRLR0, cr0);
> >
> > @@ -850,7 +857,7 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
> >
> > if (!(cr0 & DW_PSSI_CTRLR0_DFS_MASK)) {
> > dws->caps |= DW_SPI_CAP_DFS32;
> > - dws->dfs_offset = DW_PSSI_CTRLR0_DFS32_OFFSET;
> > + dws->dfs_offset = __bf_shf(DW_PSSI_CTRLR0_DFS32_MASK);
> > dev_dbg(dev, "Detected 32-bits max data frame size\n");
> > }
> > } else {
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> > index 893b78c43a50..634085eadad1 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
>
> Haven't deeply checked this, but below changes require to have bits.h
> to be included.
> Please, double check this is included already.
Checked. It's there. The bits.h file inclusion has been added
in commit cc760f3143f5 ("spi: dw: Convert CS-override to DW SPI
capabilities"). Thanks for pointing this out anyway.
-Sergey
>
> > @@ -41,39 +41,36 @@
> > #define DW_SPI_CS_OVERRIDE 0xf4
> >
> > /* Bit fields in CTRLR0 (DWC APB SSI) */
> > -#define DW_PSSI_CTRLR0_DFS_OFFSET 0
> > #define DW_PSSI_CTRLR0_DFS_MASK GENMASK(3, 0)
> > -#define DW_PSSI_CTRLR0_DFS32_OFFSET 16
> > +#define DW_PSSI_CTRLR0_DFS32_MASK GENMASK(20, 16)
> >
> > -#define DW_PSSI_CTRLR0_FRF_OFFSET 4
> > +#define DW_PSSI_CTRLR0_FRF_MASK GENMASK(5, 4)
> > #define DW_SPI_CTRLR0_FRF_MOTO_SPI 0x0
> > #define DW_SPI_CTRLR0_FRF_TI_SSP 0x1
> > #define DW_SPI_CTRLR0_FRF_NS_MICROWIRE 0x2
> > #define DW_SPI_CTRLR0_FRF_RESV 0x3
> >
> > -#define DW_PSSI_CTRLR0_MODE_OFFSET 6
> > -#define DW_PSSI_CTRLR0_SCPH_OFFSET 6
> > -#define DW_PSSI_CTRLR0_SCOL_OFFSET 7
> > +#define DW_PSSI_CTRLR0_MODE_MASK GENMASK(7, 6)
> > +#define DW_PSSI_CTRLR0_SCPHA BIT(6)
> > +#define DW_PSSI_CTRLR0_SCPOL BIT(7)
> >
> > -#define DW_PSSI_CTRLR0_TMOD_OFFSET 8
> > -#define DW_PSSI_CTRLR0_TMOD_MASK (0x3 << DW_PSSI_CTRLR0_TMOD_OFFSET)
> > +#define DW_PSSI_CTRLR0_TMOD_MASK GENMASK(9, 8)
> > #define DW_SPI_CTRLR0_TMOD_TR 0x0 /* xmit & recv */
> > #define DW_SPI_CTRLR0_TMOD_TO 0x1 /* xmit only */
> > #define DW_SPI_CTRLR0_TMOD_RO 0x2 /* recv only */
> > #define DW_SPI_CTRLR0_TMOD_EPROMREAD 0x3 /* eeprom read mode */
> >
> > -#define DW_PSSI_CTRLR0_SLVOE_OFFSET 10
> > -#define DW_PSSI_CTRLR0_SRL_OFFSET 11
> > -#define DW_PSSI_CTRLR0_CFS_OFFSET 12
> > +#define DW_PSSI_CTRLR0_SLV_OE BIT(10)
> > +#define DW_PSSI_CTRLR0_SRL BIT(11)
> > +#define DW_PSSI_CTRLR0_CFS BIT(12)
> >
> > /* Bit fields in CTRLR0 (DWC SSI with AHB interface) */
> > -#define DW_HSSI_CTRLR0_SRL_OFFSET 13
> > -#define DW_HSSI_CTRLR0_TMOD_OFFSET 10
> > +#define DW_HSSI_CTRLR0_DFS_MASK GENMASK(4, 0)
> > +#define DW_HSSI_CTRLR0_FRF_MASK GENMASK(7, 6)
> > +#define DW_HSSI_CTRLR0_SCPHA BIT(8)
> > +#define DW_HSSI_CTRLR0_SCPOL BIT(9)
> > #define DW_HSSI_CTRLR0_TMOD_MASK GENMASK(11, 10)
> > -#define DW_HSSI_CTRLR0_SCPOL_OFFSET 9
> > -#define DW_HSSI_CTRLR0_SCPH_OFFSET 8
> > -#define DW_HSSI_CTRLR0_FRF_OFFSET 6
> > -#define DW_HSSI_CTRLR0_DFS_OFFSET 0
> > +#define DW_HSSI_CTRLR0_SRL BIT(13)
> >
> > /*
> > * For Keem Bay, CTRLR0[31] is used to select controller mode.
> > @@ -86,26 +83,27 @@
> > #define DW_SPI_NDF_MASK GENMASK(15, 0)
> >
> > /* Bit fields in SR, 7 bits */
> > -#define DW_SPI_SR_MASK 0x7f /* cover 7 bits */
> > -#define DW_SPI_SR_BUSY (1 << 0)
> > -#define DW_SPI_SR_TF_NOT_FULL (1 << 1)
> > -#define DW_SPI_SR_TF_EMPT (1 << 2)
> > -#define DW_SPI_SR_RF_NOT_EMPT (1 << 3)
> > -#define DW_SPI_SR_RF_FULL (1 << 4)
> > -#define DW_SPI_SR_TX_ERR (1 << 5)
> > -#define DW_SPI_SR_DCOL (1 << 6)
> > +#define DW_SPI_SR_MASK GENMASK(6, 0)
> > +#define DW_SPI_SR_BUSY BIT(0)
> > +#define DW_SPI_SR_TF_NOT_FULL BIT(1)
> > +#define DW_SPI_SR_TF_EMPT BIT(2)
> > +#define DW_SPI_SR_RF_NOT_EMPT BIT(3)
> > +#define DW_SPI_SR_RF_FULL BIT(4)
> > +#define DW_SPI_SR_TX_ERR BIT(5)
> > +#define DW_SPI_SR_DCOL BIT(6)
> >
> > /* Bit fields in ISR, IMR, RISR, 7 bits */
> > -#define DW_SPI_INT_TXEI (1 << 0)
> > -#define DW_SPI_INT_TXOI (1 << 1)
> > -#define DW_SPI_INT_RXUI (1 << 2)
> > -#define DW_SPI_INT_RXOI (1 << 3)
> > -#define DW_SPI_INT_RXFI (1 << 4)
> > -#define DW_SPI_INT_MSTI (1 << 5)
> > +#define DW_SPI_INT_MASK GENMASK(5, 0)
> > +#define DW_SPI_INT_TXEI BIT(0)
> > +#define DW_SPI_INT_TXOI BIT(1)
> > +#define DW_SPI_INT_RXUI BIT(2)
> > +#define DW_SPI_INT_RXOI BIT(3)
> > +#define DW_SPI_INT_RXFI BIT(4)
> > +#define DW_SPI_INT_MSTI BIT(5)
> >
> > /* Bit fields in DMACR */
> > -#define DW_SPI_DMACR_RDMAE (1 << 0)
> > -#define DW_SPI_DMACR_TDMAE (1 << 1)
> > +#define DW_SPI_DMACR_RDMAE BIT(0)
> > +#define DW_SPI_DMACR_TDMAE BIT(1)
> >
> > /* Mem/DMA operations helpers */
> > #define DW_SPI_WAIT_RETRIES 5
> > --
> > 2.33.0
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-12 21:37 ` Andy Shevchenko
@ 2021-11-12 22:05 ` Serge Semin
[not found] ` <CAHp75VdP6WL=cFn2eaDC8VH7C+fd11xKxp5_qrFuArubD4KgEQ@mail.gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2021-11-12 22:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
>
> > > + /*
> > > + * Retrieve the Synopsys component version if it hasn't been specified
> > > + * by the platform. Note the CoreKit version ID is encoded as a 4bytes
> > > + * ASCII string enclosed with '*' symbol.
> > > + */
> > > + if (!dws->ver) {
> > > + u32 comp;
> > > +
> > > + comp = dw_readl(dws, DW_SPI_VERSION);
> > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> > > +
> > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ",
> > > + dws->ver / 100, dws->ver % 100);
> >
> > Oh là là, first you multiply then you divide in the same piece of code!
> > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
> > we have %p4cc)
Please note that's just a dev_DBG() print. So division has been used
in there to check whether the conversion was correct. The whole idea
behind the code above it was to retrieve the Component version as a
single number so then it could be used by the driver code in a simple
statement with a normal integer operation. For instance in case if we
need to check whether DW SSI IP-core version is greater than 1.01 we'd
have something like this: if (dws->ver > 101). Here 101 looks at least
close to the original 1.01. How would the statement look with four
chars? Of course we could add an another macro which would look like
this:
#define DW_SSI_VER(_maj, _mid, _min) \
((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*')
and use it with raw version ID, like this
(dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look
better if not worse.
Alternatively we could split the version ID into two parts with
major and minor numbers. But normally one doesn't make much sense
without another so each statement would need to check both of them
at once anyway. So I decided to stick with a simplest solution and
combined them into a single storage. Have you got a better idea of
how to implement this functionality?
> >
> > > + }
>
> Have you seen this, btw?
>
> https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_dwlib.c#L93
It doesn't utilized version ID for something functional, but just
prints it to the console. So it isn't that good reference in this
case.
-Sergey
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
[not found] ` <CAHp75VdP6WL=cFn2eaDC8VH7C+fd11xKxp5_qrFuArubD4KgEQ@mail.gmail.com>
@ 2021-11-13 0:19 ` Serge Semin
2021-11-13 13:15 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2021-11-13 0:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote:
> On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote:
>
> > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote:
> > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
> > > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > > > + /*
> > > > > + * Retrieve the Synopsys component version if it hasn't been
> > specified
> > > > > + * by the platform. Note the CoreKit version ID is encoded
> > as a 4bytes
> > > > > + * ASCII string enclosed with '*' symbol.
> > > > > + */
> > > > > + if (!dws->ver) {
> > > > > + u32 comp;
> > > > > +
> > > > > + comp = dw_readl(dws, DW_SPI_VERSION);
> > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> > > > > +
> > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : "
> > APB ",
> > > > > + dws->ver / 100, dws->ver % 100);
> > > >
> >
> > > > Oh là là, first you multiply then you divide in the same piece of code!
> > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
> > > > we have %p4cc)
> >
> > Please note that's just a dev_DBG() print. So division has been used
> > in there to check whether the conversion was correct. The whole idea
> > behind the code above it was to retrieve the Component version as a
> > single number so then it could be used by the driver code in a simple
> > statement with a normal integer operation. For instance in case if we
> > need to check whether DW SSI IP-core version is greater than 1.01 we'd
> > have something like this: if (dws->ver > 101). Here 101 looks at least
> > close to the original 1.01. How would the statement look with four
> > chars? Of course we could add an another macro which would look like
> > this:
> > #define DW_SSI_VER(_maj, _mid, _min) \
> > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*')
> > and use it with raw version ID, like this
> > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look
> > better if not worse.
> > Alternatively we could split the version ID into two parts with
> > major and minor numbers. But normally one doesn't make much sense
> > without another so each statement would need to check both of them
> > at once anyway. So I decided to stick with a simplest solution and
> > combined them into a single storage. Have you got a better idea of
> > how to implement this functionality?
>
>
>
> Then check DWC3 driver which relies on IZp version a lot.
I'm still not convinced that the DWC3 solution would be better in this case.
(I had a similar approach in mind though.) Although it might be suitable
here seeing we could take the IP generation into account in a single
macro. But at the same time having macros defined for each version may
eventually turn into a clumsy set of macros space as it happened in DWC3.
I don't understand what do you see wrong in the suggested here
solution except a math in the debug string? Why would you prefer the
DWC3 approach better than the one implemented in my patch?
I don't really see much benefits in it:
if (dws->ver > 101)
or
if (DW_SPI_VER_AFTER(dws, 101))
In both cases version ID isn't represented in the original
Vendor-defined structure, like "1.01". The only part which could be
considered as better in DWC3 approach is having a macro name, which gives
a bit better notion about the operation. But does it really worth
introducing a new abstraction in the driver?
On the other hand we could intermix the approaches. For instance decode
the Component version as I suggested in this patch and implement a set of
version checking macro. Thus we won't need so many additional macro
encoding the SSI_COMP_VERSION content.
If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no
performance drawbacks I'd be glad to use it. AFAIU compiler can't
operate with the string literal symbols, thus the symbols extraction
like "1.01a"[0] will be performed on each statement execution which isn't
that performant comparing to a simple two integers comparison.
BTW note the DWC3 macros implicitly depend on having a local variable
with dwc name which violates the kernel coding style.
-Sergey
>
>
> >
> > > >
> > > > > + }
> > >
> >
> > > Have you seen this, btw?
> > >
> > > https://elixir.bootlin.com/linux/latest/source/drivers/
> > tty/serial/8250/8250_dwlib.c#L93
> >
> > It doesn't utilized version ID for something functional, but just
> > prints it to the console. So it isn't that good reference in this
> > case.
> >
> > -Sergey
> >
> > >
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-13 0:19 ` Serge Semin
@ 2021-11-13 13:15 ` Andy Shevchenko
2021-11-13 21:30 ` Serge Semin
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2021-11-13 13:15 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Sat, Nov 13, 2021 at 2:19 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote:
> > On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote:
> > > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote:
> > > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
> > > > > <Sergey.Semin@baikalelectronics.ru> wrote:
...
> > > > > > + /*
> > > > > > + * Retrieve the Synopsys component version if it hasn't been
> > > specified
> > > > > > + * by the platform. Note the CoreKit version ID is encoded
> > > as a 4bytes
> > > > > > + * ASCII string enclosed with '*' symbol.
> > > > > > + */
> > > > > > + if (!dws->ver) {
> > > > > > + u32 comp;
> > > > > > +
> > > > > > + comp = dw_readl(dws, DW_SPI_VERSION);
> > > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> > > > > > +
> > > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> > > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : "
> > > APB ",
> > > > > > + dws->ver / 100, dws->ver % 100);
> > > > >
> > >
> > > > > Oh lą lą, first you multiply then you divide in the same piece of code!
> > > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
> > > > > we have %p4cc)
> > >
> > > Please note that's just a dev_DBG() print. So division has been used
> > > in there to check whether the conversion was correct. The whole idea
> > > behind the code above it was to retrieve the Component version as a
> > > single number so then it could be used by the driver code in a simple
> > > statement with a normal integer operation. For instance in case if we
> > > need to check whether DW SSI IP-core version is greater than 1.01 we'd
> > > have something like this: if (dws->ver > 101). Here 101 looks at least
> > > close to the original 1.01. How would the statement look with four
> > > chars? Of course we could add an another macro which would look like
> > > this:
> > > #define DW_SSI_VER(_maj, _mid, _min) \
> > > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*')
> > > and use it with raw version ID, like this
> > > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look
> > > better if not worse.
> > > Alternatively we could split the version ID into two parts with
> > > major and minor numbers. But normally one doesn't make much sense
> > > without another so each statement would need to check both of them
> > > at once anyway. So I decided to stick with a simplest solution and
> > > combined them into a single storage. Have you got a better idea of
> > > how to implement this functionality?
>
> > Then check DWC3 driver which relies on IZp version a lot.
>
> I'm still not convinced that the DWC3 solution would be better in this case.
> (I had a similar approach in mind though.) Although it might be suitable
> here seeing we could take the IP generation into account in a single
> macro. But at the same time having macros defined for each version may
> eventually turn into a clumsy set of macros space as it happened in DWC3.
>
> I don't understand what do you see wrong in the suggested here
> solution except a math in the debug string?
The transformation ruins the fourcc [1] representation. We know that
this is an ASCII. We know the position and meaning of each from 4
characters.
[1]: https://en.wikipedia.org/wiki/FourCC
> Why would you prefer the
> DWC3 approach better than the one implemented in my patch?
You asked what is _better_ implementation than yours. It doesn't mean
the DWC3 approach fully fits here, but
- SPI DW has the same issue as DWC3 solves, i.e. different version
lines for two or more IPs of the same kind (see DWC3, DWC31, DWC32)
- it doesn't ruins 4cc
In case if we need to print it, I would rather see something in %p4cc
implementation than customized 100500 approaches spreaded over the
entire kernel.
> I don't really see much benefits in it:
> if (dws->ver > 101)
> or
> if (DW_SPI_VER_AFTER(dws, 101))
> In both cases version ID isn't represented in the original
> Vendor-defined structure, like "1.01". The only part which could be
> considered as better in DWC3 approach is having a macro name, which gives
> a bit better notion about the operation. But does it really worth
> introducing a new abstraction in the driver?
>
> On the other hand we could intermix the approaches. For instance decode
> the Component version as I suggested in this patch and implement a set of
> version checking macro. Thus we won't need so many additional macro
> encoding the SSI_COMP_VERSION content.
>
> If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no
> performance drawbacks I'd be glad to use it. AFAIU compiler can't
> operate with the string literal symbols, thus the symbols extraction
> like "1.01a"[0] will be performed on each statement execution which isn't
> that performant comparing to a simple two integers comparison.
>
> BTW note the DWC3 macros implicitly depend on having a local variable
> with dwc name which violates the kernel coding style.
> > > > > > + }
> > >
> > > > Have you seen this, btw?
> > > >
> > > > https://elixir.bootlin.com/linux/latest/source/drivers/
> > > tty/serial/8250/8250_dwlib.c#L93
> > >
> > > It doesn't utilized version ID for something functional, but just
> > > prints it to the console. So it isn't that good reference in this
> > > case.
Depends what you would like to do with this. If it's only for
information to the developer, then it fits, if you need to compare,
then see above.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing
2021-11-13 13:15 ` Andy Shevchenko
@ 2021-11-13 21:30 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2021-11-13 21:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Mark Brown, Nandhini Srikandan, Andy Shevchenko,
Andy Shevchenko, linux-spi, Linux Kernel Mailing List
On Sat, Nov 13, 2021 at 03:15:41PM +0200, Andy Shevchenko wrote:
> On Sat, Nov 13, 2021 at 2:19 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> > On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote:
> > > On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote:
> > > > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote:
> > > > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin
> > > > > > <Sergey.Semin@baikalelectronics.ru> wrote:
>
> ...
>
> > > > > > > + /*
> > > > > > > + * Retrieve the Synopsys component version if it hasn't been
> > > > specified
> > > > > > > + * by the platform. Note the CoreKit version ID is encoded
> > > > as a 4bytes
> > > > > > > + * ASCII string enclosed with '*' symbol.
> > > > > > > + */
> > > > > > > + if (!dws->ver) {
> > > > > > > + u32 comp;
> > > > > > > +
> > > > > > > + comp = dw_readl(dws, DW_SPI_VERSION);
> > > > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100;
> > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10;
> > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0');
> > > > > > > +
> > > > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n",
> > > > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : "
> > > > APB ",
> > > > > > > + dws->ver / 100, dws->ver % 100);
> > > > > >
> > > >
> > > > > > Oh lą lą, first you multiply then you divide in the same piece of code!
> > > > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also
> > > > > > we have %p4cc)
> > > >
> > > > Please note that's just a dev_DBG() print. So division has been used
> > > > in there to check whether the conversion was correct. The whole idea
> > > > behind the code above it was to retrieve the Component version as a
> > > > single number so then it could be used by the driver code in a simple
> > > > statement with a normal integer operation. For instance in case if we
> > > > need to check whether DW SSI IP-core version is greater than 1.01 we'd
> > > > have something like this: if (dws->ver > 101). Here 101 looks at least
> > > > close to the original 1.01. How would the statement look with four
> > > > chars? Of course we could add an another macro which would look like
> > > > this:
> > > > #define DW_SSI_VER(_maj, _mid, _min) \
> > > > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*')
> > > > and use it with raw version ID, like this
> > > > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look
> > > > better if not worse.
> > > > Alternatively we could split the version ID into two parts with
> > > > major and minor numbers. But normally one doesn't make much sense
> > > > without another so each statement would need to check both of them
> > > > at once anyway. So I decided to stick with a simplest solution and
> > > > combined them into a single storage. Have you got a better idea of
> > > > how to implement this functionality?
> >
> > > Then check DWC3 driver which relies on IZp version a lot.
> >
> > I'm still not convinced that the DWC3 solution would be better in this case.
> > (I had a similar approach in mind though.) Although it might be suitable
> > here seeing we could take the IP generation into account in a single
> > macro. But at the same time having macros defined for each version may
> > eventually turn into a clumsy set of macros space as it happened in DWC3.
> >
> > I don't understand what do you see wrong in the suggested here
> > solution except a math in the debug string?
>
> The transformation ruins the fourcc [1] representation. We know that
> this is an ASCII. We know the position and meaning of each from 4
> characters.
>
> [1]: https://en.wikipedia.org/wiki/FourCC
So that four-CC thing wasn't Synopsys invention after all, but a sort
of a relatively common approach. Your point finally starts making
sense to me.
>
> > Why would you prefer the
> > DWC3 approach better than the one implemented in my patch?
>
> You asked what is _better_ implementation than yours. It doesn't mean
> the DWC3 approach fully fits here, but
> - SPI DW has the same issue as DWC3 solves, i.e. different version
> lines for two or more IPs of the same kind (see DWC3, DWC31, DWC32)
> - it doesn't ruins 4cc
>
> In case if we need to print it, I would rather see something in %p4cc
> implementation than customized 100500 approaches spreaded over the
> entire kernel.
Yep, these are the main pros of the DWC3 approach. Just to note
in fact AFAICS Synopsys doesn't utilize the denoted components
versioning for all its IP-cores. At least DW (G|X-G|XL-G)?MACs define
either a normal one-byte component version ID (for instance version
3.3 looks as 0x33) or two-bytes with pair - IP-core and version IDs.
But that likely is an exception, while the most of DWCs are indeed
identified by the ASCII tuple.
In our case we don't have the IP-core version explicitly encoded in
the Component version register, so it is determined by the
platform-specific code. With a minor effort I'll be able to just
convert the DW_SPI_CAP_DWC_HSSI capability into two macros with virtual
IP-core ID thus we'd have a more-or-less coherent versioning API. In
this case we can retain the ASCII Version ID. Settled then. I'll send
v2 with this patch reworked as you suggest.
-Sergey
>
> > I don't really see much benefits in it:
> > if (dws->ver > 101)
> > or
> > if (DW_SPI_VER_AFTER(dws, 101))
> > In both cases version ID isn't represented in the original
> > Vendor-defined structure, like "1.01". The only part which could be
> > considered as better in DWC3 approach is having a macro name, which gives
> > a bit better notion about the operation. But does it really worth
> > introducing a new abstraction in the driver?
> >
> > On the other hand we could intermix the approaches. For instance decode
> > the Component version as I suggested in this patch and implement a set of
> > version checking macro. Thus we won't need so many additional macro
> > encoding the SSI_COMP_VERSION content.
> >
> > If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no
> > performance drawbacks I'd be glad to use it. AFAIU compiler can't
> > operate with the string literal symbols, thus the symbols extraction
> > like "1.01a"[0] will be performed on each statement execution which isn't
> > that performant comparing to a simple two integers comparison.
> >
> > BTW note the DWC3 macros implicitly depend on having a local variable
> > with dwc name which violates the kernel coding style.
>
> > > > > > > + }
> > > >
> > > > > Have you seen this, btw?
> > > > >
> > > > > https://elixir.bootlin.com/linux/latest/source/drivers/
> > > > tty/serial/8250/8250_dwlib.c#L93
> > > >
> > > > It doesn't utilized version ID for something functional, but just
> > > > prints it to the console. So it isn't that good reference in this
> > > > case.
>
> Depends what you would like to do with this. If it's only for
> information to the developer, then it fits, if you need to compare,
> then see above.
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-11-13 21:31 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 20:49 [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Serge Semin
2021-11-12 20:49 ` [PATCH 1/4] spi: dw: Discard redundant DW SSI Frame Formats enumeration Serge Semin
2021-11-12 20:49 ` [PATCH 2/4] spi: dw: Put the driver entities naming in order Serge Semin
2021-11-12 21:22 ` Andy Shevchenko
2021-11-12 21:30 ` Serge Semin
2021-11-12 20:49 ` [PATCH 3/4] spi: dw: Convert to using the Bitfield access macros Serge Semin
2021-11-12 21:23 ` Andy Shevchenko
2021-11-12 21:43 ` Serge Semin
2021-11-12 20:49 ` [PATCH 4/4] spi: dw: Add Synopsys Component version reading and parsing Serge Semin
2021-11-12 21:27 ` Andy Shevchenko
2021-11-12 21:37 ` Andy Shevchenko
2021-11-12 22:05 ` Serge Semin
[not found] ` <CAHp75VdP6WL=cFn2eaDC8VH7C+fd11xKxp5_qrFuArubD4KgEQ@mail.gmail.com>
2021-11-13 0:19 ` Serge Semin
2021-11-13 13:15 ` Andy Shevchenko
2021-11-13 21:30 ` Serge Semin
2021-11-12 21:30 ` [PATCH 0/4] spi: dw: Cleanup macros/funcs naming and add IP-core version support Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).