[PATCH 0/2] Cleanup chip info in spi-dw driver

[PATCH 0/2] Cleanup chip info in spi-dw driver

[Clement Leger]

Some mechanisms have no more user, and as such code paths are unused.
Remove these code paths and associated structs members.

Clement Leger (2):
  spi: dw: remove unused dw_spi_chip handling
  spi: dw: remove cs_control and poll_mode members from chip_data

 drivers/spi/spi-dw.c | 57 +-------------------------------------------
 drivers/spi/spi-dw.h | 12 ----------
 2 files changed, 1 insertion(+), 68 deletions(-)

-- 
2.17.1

[PATCH 1/2] spi: dw: remove unused dw_spi_chip handling

[Clement Leger]

The path of code using this struct is unused since there is no more user
of this. Remove code and struct definition.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 drivers/spi/spi-dw.c | 16 ----------------
 drivers/spi/spi-dw.h | 12 ------------
 2 files changed, 28 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 31e3f866d11a..fda8d433a769 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -399,7 +399,6 @@ static void dw_spi_handle_err(struct spi_controller *master,
 /* This may be called twice for each spi dev */
 static int dw_spi_setup(struct spi_device *spi)
 {
-	struct dw_spi_chip *chip_info = NULL;
 	struct chip_data *chip;
 
 	/* Only alloc on first setup */
@@ -411,21 +410,6 @@ static int dw_spi_setup(struct spi_device *spi)
 		spi_set_ctldata(spi, chip);
 	}
 
-	/*
-	 * Protocol drivers may change the chip settings, so...
-	 * if chip_info exists, use it
-	 */
-	chip_info = spi->controller_data;
-
-	/* chip_info doesn't always exist */
-	if (chip_info) {
-		if (chip_info->cs_control)
-			chip->cs_control = chip_info->cs_control;
-
-		chip->poll_mode = chip_info->poll_mode;
-		chip->type = chip_info->type;
-	}
-
 	chip->tmode = SPI_TMOD_TR;
 
 	return 0;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 1bf5713e047d..44ef18187c15 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -235,18 +235,6 @@ static inline void spi_shutdown_chip(struct dw_spi *dws)
 	spi_set_clk(dws, 0);
 }
 
-/*
- * Each SPI slave device to work with dw_api controller should
- * has such a structure claiming its working mode (poll or PIO/DMA),
- * which can be save in the "controller_data" member of the
- * struct spi_device.
- */
-struct dw_spi_chip {
-	u8 poll_mode;	/* 1 for controller polling mode */
-	u8 type;	/* SPI/SSP/MicroWire */
-	void (*cs_control)(u32 command);
-};
-
 extern void dw_spi_set_cs(struct spi_device *spi, bool enable);
 extern int dw_spi_add_host(struct device *dev, struct dw_spi *dws);
 extern void dw_spi_remove_host(struct dw_spi *dws);
-- 
2.17.1

[PATCH 2/2] spi: dw: remove cs_control and poll_mode members from chip_data

[Clement Leger]

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(-)

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;
 }
 
-- 
2.17.1

Re: [PATCH 2/2] spi: dw: remove cs_control and poll_mode members from chip_data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 455 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] spi: dw: remove cs_control and poll_mode members from chip_data

[Clément Leger]

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.

Re: [PATCH 2/2] spi: dw: remove cs_control and poll_mode members from chip_data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] spi: dw: remove cs_control and poll_mode members from chip_data

[Clément Leger]

----- 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

Re: [PATCH 0/2] Cleanup chip info in spi-dw driver

[Mark Brown]

On Thu, 16 Apr 2020 13:08:23 +0200, Clement Leger wrote:
> Some mechanisms have no more user, and as such code paths are unused.
> Remove these code paths and associated structs members.
> 
> Clement Leger (2):
>   spi: dw: remove unused dw_spi_chip handling
>   spi: dw: remove cs_control and poll_mode members from chip_data
> 
> [...]

Applied, thanks!

[1/2] spi: dw: remove unused dw_spi_chip handling
      commit: ae9e6ac4d8542d1b16fad4bd3c3e447632437623
[2/2] spi: dw: remove cs_control and poll_mode members from chip_data
      commit: 33e8fd4bfbd7eb0921eb55caceed54e51912a4e6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark