| Message ID | 20240210164006.208149-7-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | de4af897ddf242aea18ee90d3ad2e21b4e64359c |
| Headers | show |
| Series | spi: ppc4xx: Various fixes | expand |
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
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.
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.
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
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 --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 */
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(-)