diff mbox series

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

Message ID 20240210164006.208149-7-u.kleine-koenig@pengutronix.de
State New
Headers
Series spi: ppc4xx: Various fixes |

Commit Message

Uwe Kleine-König Feb. 10, 2024, 4:40 p.m. UTC 
  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(-)

Comments

Uwe Kleine-König Feb. 27, 2024, 7:23 a.m. UTC | #1 
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
Mark Brown Feb. 27, 2024, 12:23 p.m. UTC | #2 
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.
Mark Brown Feb. 27, 2024, 12:50 p.m. UTC | #3 
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.
Uwe Kleine-König Feb. 27, 2024, 1:45 p.m. UTC | #4 
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
Mark Brown Feb. 27, 2024, 2:17 p.m. UTC | #5 
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.
diff mbox series

Patch

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 */