[PATCH 0/3] spi: ppc4xx: Various fixes

[PATCH 0/3] spi: ppc4xx: Various fixes

[Uwe Kleine-König]

Hello,

this series fixes three problems of the spi-ppc4xx driver. One of them
was introduced by myself, the other two are already older. I guess they
were unhandled before because the driver isn't enabled in any
allmodconfig build.

Now that I have this series, I found the first patch to be a duplicate
of
https://lore.kernel.org/linux-spi/3eb3f9c4407ba99d1cd275662081e46b9e839173.1707490664.git.chunkeey@gmail.com/
. Mark claimed to have applied this patch to his for-next branch, but
it's not included there yet. So I kept my version of it to please the
build bots. (Also my patch has a Fixes: line, which Christian's doesn't.
Up to Mark what to do with that.)

Best regards
Uwe

Uwe Kleine-König (3):
  spi: ppc4xx: Fix fallout from include cleanup
  spi: ppc4xx: Fix fallout from rename in struct spi_bitbang
  spi: ppc4xx: Drop write-only variable

 drivers/spi/spi-ppc4xx.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

base-commit: 65b3e71d024d553764e67e5916a41675afd0d142
-- 
2.43.0

[PATCH 1/3] spi: ppc4xx: Fix fallout from include cleanup

[Uwe Kleine-König]

The driver uses several symbols declared in <linux/platform_device.h>,
e.g module_platform_driver(). Include this header explicitly now that
<linux/of_platform.h> doesn't include <linux/platform_device.h> any
more.

Fixes: ef175b29a242 ("of: Stop circularly including of_device.h and of_platform.h")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi-ppc4xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 412d6e678224..2290b40459ef 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -31,6 +31,7 @@
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/platform_device.h>
 
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
-- 
2.43.0

[PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Uwe Kleine-König]

I failed to adapt this driver because it's not enabled in a powerpc
allmodconfig build and also wasn't hit by my grep expertise. Fix
accordingly.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202402100815.XQXw9XCF-lkp@intel.com/
Fixes: 2259233110d9 ("spi: bitbang: Follow renaming of SPI "master" to "controller"")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi-ppc4xx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 2290b40459ef..2d5536efa9f8 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -364,22 +364,22 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
 
 	/* Setup the state for the bitbang driver */
 	bbp = &hw->bitbang;
-	bbp->master = hw->host;
+	bbp->ctlr = hw->host;
 	bbp->setup_transfer = spi_ppc4xx_setupxfer;
 	bbp->txrx_bufs = spi_ppc4xx_txrx;
 	bbp->use_dma = 0;
-	bbp->master->setup = spi_ppc4xx_setup;
-	bbp->master->cleanup = spi_ppc4xx_cleanup;
-	bbp->master->bits_per_word_mask = SPI_BPW_MASK(8);
-	bbp->master->use_gpio_descriptors = true;
+	bbp->ctlr->setup = spi_ppc4xx_setup;
+	bbp->ctlr->cleanup = spi_ppc4xx_cleanup;
+	bbp->ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
+	bbp->ctlr->use_gpio_descriptors = true;
 	/*
 	 * The SPI core will count the number of GPIO descriptors to figure
 	 * out the number of chip selects available on the platform.
 	 */
-	bbp->master->num_chipselect = 0;
+	bbp->ctlr->num_chipselect = 0;
 
 	/* the spi->mode bits understood by this driver: */
-	bbp->master->mode_bits =
+	bbp->ctlr->mode_bits =
 		SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | SPI_LSB_FIRST;
 
 	/* Get the clock for the OPB */
-- 
2.43.0

[PATCH 3/3] spi: ppc4xx: Drop write-only variable

[Uwe Kleine-König]

Since commit 24778be20f87 ("spi: convert drivers to use
bits_per_word_mask") the bits_per_word variable is only written to. The
check that was there before isn't needed any more as the spi core
ensures that only 8 bit transfers are used, so the variable can go away
together with all assignments to it.

Fixes: 24778be20f87 ("spi: convert drivers to use bits_per_word_mask")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi-ppc4xx.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index 2d5536efa9f8..942c3117ab3a 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -168,10 +168,8 @@ static int spi_ppc4xx_setupxfer(struct spi_device *spi, struct spi_transfer *t)
 	int scr;
 	u8 cdm = 0;
 	u32 speed;
-	u8 bits_per_word;
 
 	/* Start with the generic configuration for this device. */
-	bits_per_word = spi->bits_per_word;
 	speed = spi->max_speed_hz;
 
 	/*
@@ -179,9 +177,6 @@ static int spi_ppc4xx_setupxfer(struct spi_device *spi, struct spi_transfer *t)
 	 * the transfer to overwrite the generic configuration with zeros.
 	 */
 	if (t) {
-		if (t->bits_per_word)
-			bits_per_word = t->bits_per_word;
-
 		if (t->speed_hz)
 			speed = min(t->speed_hz, spi->max_speed_hz);
 	}
-- 
2.43.0

Re: [PATCH 0/3] spi: ppc4xx: Various fixes

[Mark Brown]

On Sat, 10 Feb 2024 17:40:05 +0100, Uwe Kleine-König wrote:
> this series fixes three problems of the spi-ppc4xx driver. One of them
> was introduced by myself, the other two are already older. I guess they
> were unhandled before because the driver isn't enabled in any
> allmodconfig build.
> 
> Now that I have this series, I found the first patch to be a duplicate
> of
> https://lore.kernel.org/linux-spi/3eb3f9c4407ba99d1cd275662081e46b9e839173.1707490664.git.chunkeey@gmail.com/
> . Mark claimed to have applied this patch to his for-next branch, but
> it's not included there yet. So I kept my version of it to please the
> build bots. (Also my patch has a Fixes: line, which Christian's doesn't.
> Up to Mark what to do with that.)
> 
> [...]

Applied to

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

Thanks!

[1/3] spi: ppc4xx: Fix fallout from include cleanup
      commit: 6f98e44984d5eff599906c0376c027df985bf38b
[2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang
      commit: de4af897ddf242aea18ee90d3ad2e21b4e64359c
[3/3] spi: ppc4xx: Drop write-only variable
      commit: b3aa619a8b4706f35cb62f780c14e68796b37f3f

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

Re: [PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Uwe Kleine-König]

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

Hello Mark,

On Sat, Feb 10, 2024 at 05:40:07PM +0100, Uwe Kleine-König wrote:
> I failed to adapt this driver because it's not enabled in a powerpc
> allmodconfig build and also wasn't hit by my grep expertise. Fix
> accordingly.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202402100815.XQXw9XCF-lkp@intel.com/
> Fixes: 2259233110d9 ("spi: bitbang: Follow renaming of SPI "master" to "controller"")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This patch made it into v6.8-rc5 as commit
de4af897ddf242aea18ee90d3ad2e21b4e64359c. However 2259233110d9 (i.e. the
commit that renamed "master" to "ctlr" and missed to adapt spi-ppc4xx.c)
isn't in v6.8-rc5 but still waits in next for the merge window.

This patch applied to v6.8-rc5 made the driver fail to build.

Assuming we don't want to have this problem in v6.8, I suggest to revert
de4af897ddf2 and reapply it on top of your next branch.

If you're interested to prevent such problems in the future, there is a
tool in https://github.com/krzk/tools that can catch this type of
problem:

	linux$ ~/gsrc/tools/linux/verify_fixes.sh . v6.8-rc1..de4af897ddf2
	Commit: de4af897ddf2 ("spi: ppc4xx: Fix fallout from rename in struct spi_bitbang")
		Fixes tag: Fixes: 2259233110d9 ("spi: bitbang: Follow renaming of SPI "master" to "controller"")
		Has these problem(s):
			- Target SHA should be an ancestor of your tree

Thanks to Anthony for reporting this problem (by private mail, so I only
added him to Bcc).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Mark Brown]

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

On Tue, Feb 27, 2024 at 08:23:06AM +0100, Uwe Kleine-König wrote:

> If you're interested to prevent such problems in the future, there is a
> tool in https://github.com/krzk/tools that can catch this type of
> problem:

These tools are *far* too finicky about the precise formatting of the
fixes line already, I do actually run them but routinely have to turn
them off since they're just causing noise.

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

Re: [PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Mark Brown]

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

On Tue, Feb 27, 2024 at 08:23:06AM +0100, Uwe Kleine-König wrote:

> Assuming we don't want to have this problem in v6.8, I suggest to revert
> de4af897ddf2 and reapply it on top of your next branch.

BTW the issue here is that you sent this without comment in the middle
of a series of fixes the other two of which *do* apply to mainline,
ideally it would have just been sent separately since it needs to go
separately but if you *are* going to send a single series like this
things that are -next only should go after any fixes that are for
mainline.  My automation does look at where fixes are targeted this
catches it out.

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

Re: [PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Uwe Kleine-König]

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

Hello,

On Tue, Feb 27, 2024 at 12:50:15PM +0000, Mark Brown wrote:
> On Tue, Feb 27, 2024 at 08:23:06AM +0100, Uwe Kleine-König wrote:
> 
> > Assuming we don't want to have this problem in v6.8, I suggest to revert
> > de4af897ddf2 and reapply it on top of your next branch.
> 
> BTW the issue here is that you sent this without comment in the middle
> of a series of fixes the other two of which *do* apply to mainline,
> ideally it would have just been sent separately since it needs to go
> separately but if you *are* going to send a single series like this
> things that are -next only should go after any fixes that are for
> mainline.

I expected that adding Fixes lines is enough documentation but I agree
that in retrospect it would have been a good idea to mention the
expected target branch for each patch. I'm willing to take half of the
blame you assigned me as in retrospect double checking the Fixes lines
or doing a compile test of the ppc4xx driver would also have been a good
idea for you as maintainer applying the patches. Sorry for my
contribution to this problem. I only looked at next when I sent the
patches and wasn't aware of the trip wire that git applies patch 2 just
fine to mainline though it's not right to put it there.

I ordered patch 3 at the end because I didn't consider this an urgent
fix as it only addresses a W=1 warning that we lived with for over 10
years since v3.11-rc1.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang

[Mark Brown]

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

On Tue, Feb 27, 2024 at 02:45:05PM +0100, Uwe Kleine-König wrote:
> On Tue, Feb 27, 2024 at 12:50:15PM +0000, Mark Brown wrote:
> > On Tue, Feb 27, 2024 at 08:23:06AM +0100, Uwe Kleine-König wrote:

> > BTW the issue here is that you sent this without comment in the middle
> > of a series of fixes the other two of which *do* apply to mainline,
> > ideally it would have just been sent separately since it needs to go
> > separately but if you *are* going to send a single series like this
> > things that are -next only should go after any fixes that are for
> > mainline.

> I expected that adding Fixes lines is enough documentation but I agree
> that in retrospect it would have been a good idea to mention the
> expected target branch for each patch. I'm willing to take half of the

The Fixes would have done the right thing if the ordering was what I
expected or if it had been sent separately - basically I wasn't
expecting to find -next material after a mainline fix in the series so
I'll not check back further in the series. 

> blame you assigned me as in retrospect double checking the Fixes lines
> or doing a compile test of the ppc4xx driver would also have been a good
> idea for you as maintainer applying the patches. Sorry for my

I have a standard set of checks I do but as you pointed out the driver
doesn't even build with a PowerPC defconfig, let alone cross
architecture - at some point it gets to be the same situation as with
if drivers for hardware I don't have works.  The coverage in CI systems
is generally good enough for long tail configs like this, but AFAICT
even 0day didn't notice here.

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

Re: (subset) [PATCH 0/3] spi: ppc4xx: Various fixes

[Mark Brown]

On Sat, 10 Feb 2024 17:40:05 +0100, Uwe Kleine-König wrote:
> this series fixes three problems of the spi-ppc4xx driver. One of them
> was introduced by myself, the other two are already older. I guess they
> were unhandled before because the driver isn't enabled in any
> allmodconfig build.
> 
> Now that I have this series, I found the first patch to be a duplicate
> of
> https://lore.kernel.org/linux-spi/3eb3f9c4407ba99d1cd275662081e46b9e839173.1707490664.git.chunkeey@gmail.com/
> . Mark claimed to have applied this patch to his for-next branch, but
> it's not included there yet. So I kept my version of it to please the
> build bots. (Also my patch has a Fixes: line, which Christian's doesn't.
> Up to Mark what to do with that.)
> 
> [...]

Applied to

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

Thanks!

[2/3] spi: ppc4xx: Fix fallout from rename in struct spi_bitbang
      commit: d748b48eeba8e1a10c822109252b75ca30288ca0

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