[PATCH 1/2] spi/tegra20-slink: Crude refactoring to use core message parsing

[PATCH 1/2] spi/tegra20-slink: Crude refactoring to use core message parsing

[Mark Brown]

From: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

This is a half done conversion with minimal code reorganisation provided
for bisection purposes. A further patch will move the first transfer
preparation into tegra_slink_prepare_message().

Signed-off-by: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-tegra20-slink.c | 88 +++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index f8d4ef5..bb28033 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -196,6 +196,7 @@ struct tegra_slink_data {
 	u32					rx_status;
 	u32					status_reg;
 	bool					is_packed;
+	bool                                    is_first_msg;
 	unsigned long				packed_size;
 
 	u32					command_reg;
@@ -823,55 +824,56 @@ static int tegra_slink_setup(struct spi_device *spi)
 	return 0;
 }
 
-static int tegra_slink_transfer_one_message(struct spi_master *master,
-			struct spi_message *msg)
+static int tegra_slink_prepare_message(struct spi_master *master,
+				       struct spi_message *msg)
 {
-	bool is_first_msg = true;
 	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
-	struct spi_transfer *xfer;
-	struct spi_device *spi = msg->spi;
-	int ret;
 
-	msg->status = 0;
-	msg->actual_length = 0;
+	tspi->is_first_msg = true;
 
-	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
-		INIT_COMPLETION(tspi->xfer_completion);
-		ret = tegra_slink_start_transfer_one(spi, xfer, is_first_msg);
-		if (ret < 0) {
-			dev_err(tspi->dev,
-				"spi can not start transfer, err %d\n", ret);
-			goto exit;
-		}
-		is_first_msg = false;
-		ret = wait_for_completion_timeout(&tspi->xfer_completion,
-						SLINK_DMA_TIMEOUT);
-		if (WARN_ON(ret == 0)) {
-			dev_err(tspi->dev,
-				"spi trasfer timeout, err %d\n", ret);
-			ret = -EIO;
-			goto exit;
-		}
+	return 0;
+}
 
-		if (tspi->tx_status ||  tspi->rx_status) {
-			dev_err(tspi->dev, "Error in Transfer\n");
-			ret = -EIO;
-			goto exit;
-		}
-		msg->actual_length += xfer->len;
-		if (xfer->cs_change && xfer->delay_usecs) {
-			tegra_slink_writel(tspi, tspi->def_command_reg,
-					SLINK_COMMAND);
-			udelay(xfer->delay_usecs);
-		}
+static int tegra_slink_transfer_one(struct spi_master *master,
+				    struct spi_device *spi,
+				    struct spi_transfer *xfer)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+	int ret;
+
+	INIT_COMPLETION(tspi->xfer_completion);
+	ret = tegra_slink_start_transfer_one(spi, xfer, tspi->is_first_msg);
+	if (ret < 0) {
+		dev_err(tspi->dev,
+			"spi can not start transfer, err %d\n", ret);
+		return ret;
 	}
-	ret = 0;
-exit:
+	tspi->is_first_msg = false;
+	ret = wait_for_completion_timeout(&tspi->xfer_completion,
+					  SLINK_DMA_TIMEOUT);
+	if (WARN_ON(ret == 0)) {
+		dev_err(tspi->dev,
+			"spi trasfer timeout, err %d\n", ret);
+		return -EIO;
+	}
+
+	if (tspi->tx_status)
+		return tspi->tx_status;
+	if (tspi->rx_status)
+		return tspi->rx_status;
+
+	return 0;
+}
+
+static int tegra_slink_unprepare_message(struct spi_master *master,
+					 struct spi_message *msg)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
 	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
 	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
-	msg->status = ret;
-	spi_finalize_current_message(master);
-	return ret;
+
+	return 0;
 }
 
 static irqreturn_t handle_cpu_based_xfer(struct tegra_slink_data *tspi)
@@ -1074,7 +1076,9 @@ static int tegra_slink_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->setup = tegra_slink_setup;
-	master->transfer_one_message = tegra_slink_transfer_one_message;
+	master->prepare_message = tegra_slink_prepare_message;
+	master->transfer_one = tegra_slink_transfer_one;
+	master->unprepare_message = tegra_slink_unprepare_message;
 	master->auto_runtime_pm = true;
 	master->num_chipselect = MAX_CHIP_SELECT;
 	master->bus_num = -1;
-- 
1.8.4.rc3

[PATCH 2/2] spi/tegra20-slink: Move first transfer preparation to prepare_message

[Mark Brown]

From: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

This is more idiomatic for the factored out message processing and gives a
small simplification of the code since we always set the per-transfer
parameters in the same fashion.

Signed-off-by: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-tegra20-slink.c | 55 +++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 30 deletions(-)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index bb28033..175a2ff 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -196,7 +196,6 @@ struct tegra_slink_data {
 	u32					rx_status;
 	u32					status_reg;
 	bool					is_packed;
-	bool                                    is_first_msg;
 	unsigned long				packed_size;
 
 	u32					command_reg;
@@ -708,7 +707,7 @@ static void tegra_slink_deinit_dma_param(struct tegra_slink_data *tspi,
 }
 
 static int tegra_slink_start_transfer_one(struct spi_device *spi,
-		struct spi_transfer *t, bool is_first_of_msg)
+		struct spi_transfer *t)
 {
 	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
 	u32 speed;
@@ -732,32 +731,12 @@ static int tegra_slink_start_transfer_one(struct spi_device *spi,
 	tspi->curr_xfer = t;
 	total_fifo_words = tegra_slink_calculate_curr_xfer_param(spi, tspi, t);
 
-	if (is_first_of_msg) {
-		tegra_slink_clear_status(tspi);
+	command = tspi->command_reg;
+	command &= ~SLINK_BIT_LENGTH(~0);
+	command |= SLINK_BIT_LENGTH(bits_per_word - 1);
 
-		command = tspi->def_command_reg;
-		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
-		command |= SLINK_CS_SW | SLINK_CS_VALUE;
-
-		command2 = tspi->def_command2_reg;
-		command2 |= SLINK_SS_EN_CS(spi->chip_select);
-
-		command &= ~SLINK_MODES;
-		if (spi->mode & SPI_CPHA)
-			command |= SLINK_CK_SDA;
-
-		if (spi->mode & SPI_CPOL)
-			command |= SLINK_IDLE_SCLK_DRIVE_HIGH;
-		else
-			command |= SLINK_IDLE_SCLK_DRIVE_LOW;
-	} else {
-		command = tspi->command_reg;
-		command &= ~SLINK_BIT_LENGTH(~0);
-		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
-
-		command2 = tspi->command2_reg;
-		command2 &= ~(SLINK_RXEN | SLINK_TXEN);
-	}
+	command2 = tspi->command2_reg;
+	command2 &= ~(SLINK_RXEN | SLINK_TXEN);
 
 	tegra_slink_writel(tspi, command, SLINK_COMMAND);
 	tspi->command_reg = command;
@@ -828,8 +807,24 @@ static int tegra_slink_prepare_message(struct spi_master *master,
 				       struct spi_message *msg)
 {
 	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+	struct spi_device *spi = msg->spi;
 
-	tspi->is_first_msg = true;
+	tegra_slink_clear_status(tspi);
+
+	tspi->command_reg = tspi->def_command_reg;
+	tspi->command_reg |= SLINK_CS_SW | SLINK_CS_VALUE;
+
+	tspi->command2_reg = tspi->def_command2_reg;
+	tspi->command2_reg |= SLINK_SS_EN_CS(spi->chip_select);
+
+	tspi->command_reg &= ~SLINK_MODES;
+	if (spi->mode & SPI_CPHA)
+		tspi->command_reg |= SLINK_CK_SDA;
+
+	if (spi->mode & SPI_CPOL)
+		tspi->command_reg |= SLINK_IDLE_SCLK_DRIVE_HIGH;
+	else
+		tspi->command_reg |= SLINK_IDLE_SCLK_DRIVE_LOW;
 
 	return 0;
 }
@@ -842,13 +837,13 @@ static int tegra_slink_transfer_one(struct spi_master *master,
 	int ret;
 
 	INIT_COMPLETION(tspi->xfer_completion);
-	ret = tegra_slink_start_transfer_one(spi, xfer, tspi->is_first_msg);
+	ret = tegra_slink_start_transfer_one(spi, xfer);
 	if (ret < 0) {
 		dev_err(tspi->dev,
 			"spi can not start transfer, err %d\n", ret);
 		return ret;
 	}
-	tspi->is_first_msg = false;
+
 	ret = wait_for_completion_timeout(&tspi->xfer_completion,
 					  SLINK_DMA_TIMEOUT);
 	if (WARN_ON(ret == 0)) {
-- 
1.8.4.rc3

Re: [PATCH 1/2] spi/tegra20-slink: Crude refactoring to use core message parsing

[Stephen Warren]

On 10/16/2013 03:00 PM, Mark Brown wrote:
> From: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> This is a half done conversion with minimal code reorganisation provided
> for bisection purposes. A further patch will move the first transfer
> preparation into tegra_slink_prepare_message().

> diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c

> -static int tegra_slink_transfer_one_message(struct spi_master *master,
> -			struct spi_message *msg)
...
> -		msg->actual_length += xfer->len;
> -		if (xfer->cs_change && xfer->delay_usecs) {
> -			tegra_slink_writel(tspi, tspi->def_command_reg,
> -					SLINK_COMMAND);
> -			udelay(xfer->delay_usecs);
> -		}

That chunk didn't seem to end up anywhere else. Is that expected?

Re: [PATCH 1/2] spi/tegra20-slink: Crude refactoring to use core message parsing

[Mark Brown]

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

On Wed, Oct 16, 2013 at 03:41:51PM -0600, Stephen Warren wrote:
> On 10/16/2013 03:00 PM, Mark Brown wrote:

> > -static int tegra_slink_transfer_one_message(struct spi_master *master,
> > -			struct spi_message *msg)
> ...
> > -		msg->actual_length += xfer->len;
> > -		if (xfer->cs_change && xfer->delay_usecs) {
> > -			tegra_slink_writel(tspi, tspi->def_command_reg,
> > -					SLINK_COMMAND);
> > -			udelay(xfer->delay_usecs);
> > -		}

> That chunk didn't seem to end up anywhere else. Is that expected?

It should be handled by framework code and it's buggy anyway, the two
fields are not connected and cs_change looks awfully like it'd at best
only work the first time it's used.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] spi/tegra20-slink: Crude refactoring to use core message parsing

[Stephen Warren]

On 10/16/2013 03:00 PM, Mark Brown wrote:
> From: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> This is a half done conversion with minimal code reorganisation provided
> for bisection purposes. A further patch will move the first transfer
> preparation into tegra_slink_prepare_message().

The series,
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Trimslice, Beaver, and Dalmore. (Tegra20/30/114).