* [PATCH] spi: mpc52xx-psc: Switch to using core message queue
@ 2022-06-13 12:19 Mark Brown
2022-06-27 18:36 ` kernel test robot
2022-06-27 20:33 ` Mark Brown
0 siblings, 2 replies; 3+ messages in thread
From: Mark Brown @ 2022-06-13 12:19 UTC (permalink / raw)
To: linux-spi, linuxppc-dev; +Cc: Mark Brown
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(-)
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)
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] spi: mpc52xx-psc: Switch to using core message queue
2022-06-13 12:19 [PATCH] spi: mpc52xx-psc: Switch to using core message queue Mark Brown
@ 2022-06-27 18:36 ` kernel test robot
2022-06-27 20:33 ` Mark Brown
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-06-27 18:36 UTC (permalink / raw)
To: Mark Brown; +Cc: llvm, kbuild-all
Hi Mark,
I love your patch! Perhaps something to improve:
[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on v5.19-rc4 next-20220627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mark-Brown/spi-mpc52xx-psc-Switch-to-using-core-message-queue/20220613-202736
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: powerpc-mpc5200_defconfig (https://download.01.org/0day-ci/archive/20220628/202206280209.YzC8dodM-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 016342e319fd31e41cf5ed16a6140a8ea2de74dd)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/863e7d2f2d8e54cc76aa3824e7a271fdb0987f6a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mark-Brown/spi-mpc52xx-psc-Switch-to-using-core-message-queue/20220613-202736
git checkout 863e7d2f2d8e54cc76aa3824e7a271fdb0987f6a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/spi/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
__do_insb
^
arch/powerpc/include/asm/io.h:578:56: note: expanded from macro '__do_insb'
#define __do_insb(p, b, n) readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/spi/spi-mpc52xx-psc.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:641:
arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:638:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:117:1: note: expanded from here
__do_insw
^
arch/powerpc/include/asm/io.h:579:56: note: expanded from macro '__do_insw'
#define __do_insw(p, b, n) readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/spi/spi-mpc52xx-psc.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:641:
arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:638:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:119:1: note: expanded from here
__do_insl
^
arch/powerpc/include/asm/io.h:580:56: note: expanded from macro '__do_insl'
#define __do_insl(p, b, n) readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/spi/spi-mpc52xx-psc.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:641:
arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:638:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:121:1: note: expanded from here
__do_outsb
^
arch/powerpc/include/asm/io.h:581:58: note: expanded from macro '__do_outsb'
#define __do_outsb(p, b, n) writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/spi/spi-mpc52xx-psc.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:641:
arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:638:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:123:1: note: expanded from here
__do_outsw
^
arch/powerpc/include/asm/io.h:582:58: note: expanded from macro '__do_outsw'
#define __do_outsw(p, b, n) writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/spi/spi-mpc52xx-psc.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:641:
arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:638:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:125:1: note: expanded from here
__do_outsl
^
arch/powerpc/include/asm/io.h:583:58: note: expanded from macro '__do_outsl'
#define __do_outsl(p, b, n) writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
>> drivers/spi/spi-mpc52xx-psc.c:195:5: warning: no previous prototype for function 'mpc52xx_psc_spi_transfer_one_message' [-Wmissing-prototypes]
int mpc52xx_psc_spi_transfer_one_message(struct spi_controller *ctlr,
^
drivers/spi/spi-mpc52xx-psc.c:195:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int mpc52xx_psc_spi_transfer_one_message(struct spi_controller *ctlr,
^
static
7 warnings generated.
vim +/mpc52xx_psc_spi_transfer_one_message +195 drivers/spi/spi-mpc52xx-psc.c
194
> 195 int mpc52xx_psc_spi_transfer_one_message(struct spi_controller *ctlr,
196 struct spi_message *m)
197 {
198 struct spi_device *spi;
199 struct spi_transfer *t = NULL;
200 unsigned cs_change;
201 int status;
202
203 spi = m->spi;
204 cs_change = 1;
205 status = 0;
206 list_for_each_entry (t, &m->transfers, transfer_list) {
207 if (t->bits_per_word || t->speed_hz) {
208 status = mpc52xx_psc_spi_transfer_setup(spi, t);
209 if (status < 0)
210 break;
211 }
212
213 if (cs_change)
214 mpc52xx_psc_spi_activate_cs(spi);
215 cs_change = t->cs_change;
216
217 status = mpc52xx_psc_spi_transfer_rxtx(spi, t);
218 if (status)
219 break;
220 m->actual_length += t->len;
221
222 spi_transfer_delay_exec(t);
223
224 if (cs_change)
225 mpc52xx_psc_spi_deactivate_cs(spi);
226 }
227
228 m->status = status;
229 if (status || !cs_change)
230 mpc52xx_psc_spi_deactivate_cs(spi);
231
232 mpc52xx_psc_spi_transfer_setup(spi, NULL);
233
234 spi_finalize_current_message(ctlr);
235
236 return 0;
237 }
238
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] spi: mpc52xx-psc: Switch to using core message queue
2022-06-13 12:19 [PATCH] spi: mpc52xx-psc: Switch to using core message queue Mark Brown
2022-06-27 18:36 ` kernel test robot
@ 2022-06-27 20:33 ` Mark Brown
1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2022-06-27 20:33 UTC (permalink / raw)
To: linux-spi, broonie, linuxppc-dev
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-27 20:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 12:19 [PATCH] spi: mpc52xx-psc: Switch to using core message queue Mark Brown
2022-06-27 18:36 ` kernel test robot
2022-06-27 20:33 ` Mark Brown
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.