| Message ID | 20200416110916.22633-2-cleger@kalray.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 33e8fd4bfbd7eb0921eb55caceed54e51912a4e6 |
| Headers | show |
| Series | Cleanup chip info in spi-dw driver | expand |
On Thu, Apr 16, 2020 at 01:09:16PM +0200, Clement Leger wrote: > Since these members were initialized only with previous dw_spi_chip > struct members and that there is no user anymore, remove them. Along > this removal, remove code path which were using these members. This means that your first patch will break the build, to keep things bisectable please do some combination of moving this before the first patch and squashing it into the first patch.
Hi Mark, Sorry, maybe the commit message was not clear enough but actually these fields were not initialized so it does not break anything (ie, the default values were always used). Clément ----- On 16 Apr, 2020, at 13:35, Mark Brown broonie@kernel.org wrote: > On Thu, Apr 16, 2020 at 01:09:16PM +0200, Clement Leger wrote: >> Since these members were initialized only with previous dw_spi_chip >> struct members and that there is no user anymore, remove them. Along >> this removal, remove code path which were using these members. > > This means that your first patch will break the build, to keep things > bisectable please do some combination of moving this before the first > patch and squashing it into the first patch.
On Thu, Apr 16, 2020 at 01:50:55PM +0200, Clément Leger wrote: > Hi Mark, Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed. > Sorry, maybe the commit message was not clear enough but actually > these fields were not initialized so it does not break anything > (ie, the default values were always used). When you remove the definition of the struct and declaration of the variable pointing to it then the code won't compile. > > This means that your first patch will break the build, to keep things
----- On 16 Apr, 2020, at 14:47, Mark Brown broonie@kernel.org wrote: > On Thu, Apr 16, 2020 at 01:50:55PM +0200, Clément Leger wrote: >> Hi Mark, > > Please don't top post, reply in line with needed context. This allows > readers to readily follow the flow of conversation and understand what > you are talking about and also helps ensure that everything in the > discussion is being addressed. Sorry, I'll take care of that. > >> Sorry, maybe the commit message was not clear enough but actually >> these fields were not initialized so it does not break anything >> (ie, the default values were always used). > > When you remove the definition of the struct and declaration of the > variable pointing to it then the code won't compile. Yes I know and I took care of that but there is probably a misunderstanding. There is actually two structs with the same members (dw_chip_info and chip_data). The first one (chip_info) if it exists, is copied into the second (chip_data). First patch [1/2] removes the part of code that did copy these fields. And then second patchs removes the usage of chip_data fields. Please let me know if I missed something. I tried to recompile it and there is no build breakage. Clément > > > > This means that your first patch will break the build, to keep things
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c index fda8d433a769..2b79c5a983c0 100644 --- a/drivers/spi/spi-dw.c +++ b/drivers/spi/spi-dw.c @@ -24,11 +24,8 @@ struct chip_data { u8 tmode; /* TR/TO/RO/EEPROM */ u8 type; /* SPI/SSP/MicroWire */ - u8 poll_mode; /* 1 means use poll mode */ - u16 clk_div; /* baud rate divider */ u32 speed_hz; /* baud rate */ - void (*cs_control)(u32 command); }; #ifdef CONFIG_DEBUG_FS @@ -127,11 +124,6 @@ static inline void dw_spi_debugfs_remove(struct dw_spi *dws) void dw_spi_set_cs(struct spi_device *spi, bool enable) { struct dw_spi *dws = spi_controller_get_devdata(spi->controller); - struct chip_data *chip = spi_get_ctldata(spi); - - /* Chip select logic is inverted from spi_set_cs() */ - if (chip && chip->cs_control) - chip->cs_control(!enable); if (!enable) dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select)); @@ -265,18 +257,6 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id) return dws->transfer_handler(dws); } -/* Must be called inside pump_transfers() */ -static int poll_transfer(struct dw_spi *dws) -{ - do { - dw_writer(dws); - dw_reader(dws); - cpu_relax(); - } while (dws->rx_end > dws->rx); - - return 0; -} - static int dw_spi_transfer_one(struct spi_controller *master, struct spi_device *spi, struct spi_transfer *transfer) { @@ -324,22 +304,6 @@ static int dw_spi_transfer_one(struct spi_controller *master, (((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET)) | (chip->tmode << SPI_TMOD_OFFSET); - /* - * Adjust transfer mode if necessary. Requires platform dependent - * chipselect mechanism. - */ - if (chip->cs_control) { - if (dws->rx && dws->tx) - chip->tmode = SPI_TMOD_TR; - else if (dws->rx) - chip->tmode = SPI_TMOD_RO; - else - chip->tmode = SPI_TMOD_TO; - - cr0 &= ~SPI_TMOD_MASK; - cr0 |= (chip->tmode << SPI_TMOD_OFFSET); - } - dw_writel(dws, DW_SPI_CTRL0, cr0); /* Check if current transfer is a DMA transaction */ @@ -359,7 +323,7 @@ static int dw_spi_transfer_one(struct spi_controller *master, spi_enable_chip(dws, 1); return ret; } - } else if (!chip->poll_mode) { + } else { txlevel = min_t(u16, dws->fifo_len / 2, dws->len / dws->n_bytes); dw_writel(dws, DW_SPI_TXFLTR, txlevel); @@ -379,9 +343,6 @@ static int dw_spi_transfer_one(struct spi_controller *master, return ret; } - if (chip->poll_mode) - return poll_transfer(dws); - return 1; }
Since these members were initialized only with previous dw_spi_chip struct members and that there is no user anymore, remove them. Along this removal, remove code path which were using these members. Signed-off-by: Clement Leger <cleger@kalray.eu> --- drivers/spi/spi-dw.c | 41 +---------------------------------------- 1 file changed, 1 insertion(+), 40 deletions(-)