diff mbox series

spi: mpc52xx-psc: Switch to using core message queue

Message ID 20220613121946.136193-1-broonie@kernel.org (mailing list archive)
State Accepted
Commit 145cfc3840e5931a789a8e2e76af841ab4cad44b
Headers show
Series spi: mpc52xx-psc: Switch to using core message queue | expand

Commit Message

Mark Brown June 13, 2022, 12:19 p.m. UTC
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 mpc52xx-psc 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.

The driver seems like a good candidate for transfer_one() but the chip
select function is actually doing rather more than just updating the chip
select and both transfer_one() and transfer_one_message() are current APIs
so leave that refactoring for another day, ideally by someone with the
hardware.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-mpc52xx-psc.c | 116 ++++++++++------------------------
 1 file changed, 34 insertions(+), 82 deletions(-)

Comments

Mark Brown June 27, 2022, 8:33 p.m. UTC | #1
On Mon, 13 Jun 2022 13:19:46 +0100, Mark Brown wrote:
> 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 mpc52xx-psc 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: mpc52xx-psc: Switch to using core message queue
      commit: 145cfc3840e5931a789a8e2e76af841ab4cad44b

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
diff mbox series

Patch

diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index 7654736c2c0e..609311231e64 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -37,12 +37,6 @@  struct mpc52xx_psc_spi {
 	struct mpc52xx_psc_fifo __iomem *fifo;
 	unsigned int irq;
 	u8 bits_per_word;
-	u8 busy;
-
-	struct work_struct work;
-
-	struct list_head queue;
-	spinlock_t lock;
 
 	struct completion done;
 };
@@ -198,69 +192,53 @@  static int mpc52xx_psc_spi_transfer_rxtx(struct spi_device *spi,
 	return 0;
 }
 
-static void mpc52xx_psc_spi_work(struct work_struct *work)
+int mpc52xx_psc_spi_transfer_one_message(struct spi_controller *ctlr,
+					 struct spi_message *m)
 {
-	struct mpc52xx_psc_spi *mps =
-		container_of(work, struct mpc52xx_psc_spi, work);
-
-	spin_lock_irq(&mps->lock);
-	mps->busy = 1;
-	while (!list_empty(&mps->queue)) {
-		struct spi_message *m;
-		struct spi_device *spi;
-		struct spi_transfer *t = NULL;
-		unsigned cs_change;
-		int status;
-
-		m = container_of(mps->queue.next, struct spi_message, queue);
-		list_del_init(&m->queue);
-		spin_unlock_irq(&mps->lock);
-
-		spi = m->spi;
-		cs_change = 1;
-		status = 0;
-		list_for_each_entry (t, &m->transfers, transfer_list) {
-			if (t->bits_per_word || t->speed_hz) {
-				status = mpc52xx_psc_spi_transfer_setup(spi, t);
-				if (status < 0)
-					break;
-			}
-
-			if (cs_change)
-				mpc52xx_psc_spi_activate_cs(spi);
-			cs_change = t->cs_change;
-
-			status = mpc52xx_psc_spi_transfer_rxtx(spi, t);
-			if (status)
+	struct spi_device *spi;
+	struct spi_transfer *t = NULL;
+	unsigned cs_change;
+	int status;
+
+	spi = m->spi;
+	cs_change = 1;
+	status = 0;
+	list_for_each_entry (t, &m->transfers, transfer_list) {
+		if (t->bits_per_word || t->speed_hz) {
+			status = mpc52xx_psc_spi_transfer_setup(spi, t);
+			if (status < 0)
 				break;
-			m->actual_length += t->len;
+		}
 
-			spi_transfer_delay_exec(t);
+		if (cs_change)
+			mpc52xx_psc_spi_activate_cs(spi);
+		cs_change = t->cs_change;
 
-			if (cs_change)
-				mpc52xx_psc_spi_deactivate_cs(spi);
-		}
+		status = mpc52xx_psc_spi_transfer_rxtx(spi, t);
+		if (status)
+			break;
+		m->actual_length += t->len;
 
-		m->status = status;
-		if (m->complete)
-			m->complete(m->context);
+		spi_transfer_delay_exec(t);
 
-		if (status || !cs_change)
+		if (cs_change)
 			mpc52xx_psc_spi_deactivate_cs(spi);
+	}
 
-		mpc52xx_psc_spi_transfer_setup(spi, NULL);
+	m->status = status;
+	if (status || !cs_change)
+		mpc52xx_psc_spi_deactivate_cs(spi);
 
-		spin_lock_irq(&mps->lock);
-	}
-	mps->busy = 0;
-	spin_unlock_irq(&mps->lock);
+	mpc52xx_psc_spi_transfer_setup(spi, NULL);
+
+	spi_finalize_current_message(ctlr);
+
+	return 0;
 }
 
 static int mpc52xx_psc_spi_setup(struct spi_device *spi)
 {
-	struct mpc52xx_psc_spi *mps = spi_master_get_devdata(spi->master);
 	struct mpc52xx_psc_spi_cs *cs = spi->controller_state;
-	unsigned long flags;
 
 	if (spi->bits_per_word%8)
 		return -EINVAL;
@@ -275,28 +253,6 @@  static int mpc52xx_psc_spi_setup(struct spi_device *spi)
 	cs->bits_per_word = spi->bits_per_word;
 	cs->speed_hz = spi->max_speed_hz;
 
-	spin_lock_irqsave(&mps->lock, flags);
-	if (!mps->busy)
-		mpc52xx_psc_spi_deactivate_cs(spi);
-	spin_unlock_irqrestore(&mps->lock, flags);
-
-	return 0;
-}
-
-static int mpc52xx_psc_spi_transfer(struct spi_device *spi,
-		struct spi_message *m)
-{
-	struct mpc52xx_psc_spi *mps = spi_master_get_devdata(spi->master);
-	unsigned long flags;
-
-	m->actual_length = 0;
-	m->status = -EINPROGRESS;
-
-	spin_lock_irqsave(&mps->lock, flags);
-	list_add_tail(&m->queue, &mps->queue);
-	schedule_work(&mps->work);
-	spin_unlock_irqrestore(&mps->lock, flags);
-
 	return 0;
 }
 
@@ -391,7 +347,7 @@  static int mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 		master->num_chipselect = pdata->max_chipselect;
 	}
 	master->setup = mpc52xx_psc_spi_setup;
-	master->transfer = mpc52xx_psc_spi_transfer;
+	master->transfer_one_message = mpc52xx_psc_spi_transfer_one_message;
 	master->cleanup = mpc52xx_psc_spi_cleanup;
 	master->dev.of_node = dev->of_node;
 
@@ -415,10 +371,7 @@  static int mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 		goto free_irq;
 	}
 
-	spin_lock_init(&mps->lock);
 	init_completion(&mps->done);
-	INIT_WORK(&mps->work, mpc52xx_psc_spi_work);
-	INIT_LIST_HEAD(&mps->queue);
 
 	ret = spi_register_master(master);
 	if (ret < 0)
@@ -470,7 +423,6 @@  static int mpc52xx_psc_spi_of_remove(struct platform_device *op)
 	struct spi_master *master = spi_master_get(platform_get_drvdata(op));
 	struct mpc52xx_psc_spi *mps = spi_master_get_devdata(master);
 
-	flush_work(&mps->work);
 	spi_unregister_master(master);
 	free_irq(mps->irq, mps);
 	if (mps->psc)