All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>
Subject: [PATCH] spi: sh: Switch to using core message queue
Date: Fri, 10 Jun 2022 16:46:49 +0100	[thread overview]
Message-ID: <20220610154649.1707851-1-broonie@kernel.org> (raw)

We deprecated open coding of the transfer queue back in 2017 so it's high
time we finished up converting drivers to use the standard message queue
code. The SH driver is fairly straightforward so convert to use
transfer_one_message(), it looks like the driver would be a good fit for
transfer_one() with a little bit of updating but this smaller change seems
safer.

I'm not actually clear how the driver worked robustly previously, it
clears SSA and CR1 when queueing a transfer which looks like it would
interfere with any running transfer. This clearing has been moved to the
start of the message transfer function.

I'm also unclear how exactly the chip select is managed with this driver.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh.c | 94 +++++++++++++-------------------------------
 1 file changed, 28 insertions(+), 66 deletions(-)

diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c
index 45f304935332..3e72fad99adf 100644
--- a/drivers/spi/spi-sh.c
+++ b/drivers/spi/spi-sh.c
@@ -73,11 +73,8 @@ struct spi_sh_data {
 	void __iomem *addr;
 	int irq;
 	struct spi_master *master;
-	struct list_head queue;
-	struct work_struct ws;
 	unsigned long cr1;
 	wait_queue_head_t wait;
-	spinlock_t lock;
 	int width;
 };
 
@@ -271,47 +268,39 @@ static int spi_sh_receive(struct spi_sh_data *ss, struct spi_message *mesg,
 	return 0;
 }
 
-static void spi_sh_work(struct work_struct *work)
+static int spi_sh_transfer_one_message(struct spi_controller *ctlr,
+					struct spi_message *mesg)
 {
-	struct spi_sh_data *ss = container_of(work, struct spi_sh_data, ws);
-	struct spi_message *mesg;
+	struct spi_sh_data *ss = spi_controller_get_devdata(ctlr);
 	struct spi_transfer *t;
-	unsigned long flags;
 	int ret;
 
 	pr_debug("%s: enter\n", __func__);
 
-	spin_lock_irqsave(&ss->lock, flags);
-	while (!list_empty(&ss->queue)) {
-		mesg = list_entry(ss->queue.next, struct spi_message, queue);
-		list_del_init(&mesg->queue);
-
-		spin_unlock_irqrestore(&ss->lock, flags);
-		list_for_each_entry(t, &mesg->transfers, transfer_list) {
-			pr_debug("tx_buf = %p, rx_buf = %p\n",
-					t->tx_buf, t->rx_buf);
-			pr_debug("len = %d, delay.value = %d\n",
-					t->len, t->delay.value);
-
-			if (t->tx_buf) {
-				ret = spi_sh_send(ss, mesg, t);
-				if (ret < 0)
-					goto error;
-			}
-			if (t->rx_buf) {
-				ret = spi_sh_receive(ss, mesg, t);
-				if (ret < 0)
-					goto error;
-			}
-			mesg->actual_length += t->len;
-		}
-		spin_lock_irqsave(&ss->lock, flags);
+	spi_sh_clear_bit(ss, SPI_SH_SSA, SPI_SH_CR1);
 
-		mesg->status = 0;
-		if (mesg->complete)
-			mesg->complete(mesg->context);
+	list_for_each_entry(t, &mesg->transfers, transfer_list) {
+		pr_debug("tx_buf = %p, rx_buf = %p\n",
+			 t->tx_buf, t->rx_buf);
+		pr_debug("len = %d, delay.value = %d\n",
+			 t->len, t->delay.value);
+
+		if (t->tx_buf) {
+			ret = spi_sh_send(ss, mesg, t);
+			if (ret < 0)
+				goto error;
+		}
+		if (t->rx_buf) {
+			ret = spi_sh_receive(ss, mesg, t);
+			if (ret < 0)
+				goto error;
+		}
+		mesg->actual_length += t->len;
 	}
 
+	mesg->status = 0;
+	spi_finalize_current_message(ctlr);
+
 	clear_fifo(ss);
 	spi_sh_set_bit(ss, SPI_SH_SSD, SPI_SH_CR1);
 	udelay(100);
@@ -321,12 +310,11 @@ static void spi_sh_work(struct work_struct *work)
 
 	clear_fifo(ss);
 
-	spin_unlock_irqrestore(&ss->lock, flags);
-
-	return;
+	return 0;
 
  error:
 	mesg->status = ret;
+	spi_finalize_current_message(ctlr);
 	if (mesg->complete)
 		mesg->complete(mesg->context);
 
@@ -334,6 +322,7 @@ static void spi_sh_work(struct work_struct *work)
 			 SPI_SH_CR1);
 	clear_fifo(ss);
 
+	return ret;
 }
 
 static int spi_sh_setup(struct spi_device *spi)
@@ -355,29 +344,6 @@ static int spi_sh_setup(struct spi_device *spi)
 	return 0;
 }
 
-static int spi_sh_transfer(struct spi_device *spi, struct spi_message *mesg)
-{
-	struct spi_sh_data *ss = spi_master_get_devdata(spi->master);
-	unsigned long flags;
-
-	pr_debug("%s: enter\n", __func__);
-	pr_debug("\tmode = %02x\n", spi->mode);
-
-	spin_lock_irqsave(&ss->lock, flags);
-
-	mesg->actual_length = 0;
-	mesg->status = -EINPROGRESS;
-
-	spi_sh_clear_bit(ss, SPI_SH_SSA, SPI_SH_CR1);
-
-	list_add_tail(&mesg->queue, &ss->queue);
-	schedule_work(&ss->ws);
-
-	spin_unlock_irqrestore(&ss->lock, flags);
-
-	return 0;
-}
-
 static void spi_sh_cleanup(struct spi_device *spi)
 {
 	struct spi_sh_data *ss = spi_master_get_devdata(spi->master);
@@ -416,7 +382,6 @@ static int spi_sh_remove(struct platform_device *pdev)
 	struct spi_sh_data *ss = platform_get_drvdata(pdev);
 
 	spi_unregister_master(ss->master);
-	flush_work(&ss->ws);
 	free_irq(ss->irq, ss);
 
 	return 0;
@@ -467,9 +432,6 @@ static int spi_sh_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "ioremap error.\n");
 		return -ENOMEM;
 	}
-	INIT_LIST_HEAD(&ss->queue);
-	spin_lock_init(&ss->lock);
-	INIT_WORK(&ss->ws, spi_sh_work);
 	init_waitqueue_head(&ss->wait);
 
 	ret = request_irq(irq, spi_sh_irq, 0, "spi_sh", ss);
@@ -481,7 +443,7 @@ static int spi_sh_probe(struct platform_device *pdev)
 	master->num_chipselect = 2;
 	master->bus_num = pdev->id;
 	master->setup = spi_sh_setup;
-	master->transfer = spi_sh_transfer;
+	master->transfer_one_message = spi_sh_transfer_one_message;
 	master->cleanup = spi_sh_cleanup;
 
 	ret = spi_register_master(master);
-- 
2.30.2


             reply	other threads:[~2022-06-10 15:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-10 15:46 Mark Brown [this message]
2022-06-27 20:33 ` [PATCH] spi: sh: Switch to using core message queue Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220610154649.1707851-1-broonie@kernel.org \
    --to=broonie@kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.