Re: [PATCH] mtd: nand: Convert to platform remove callback returning void

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

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

Hello Miquel,

On Tue, Apr 11, 2023 at 10:20:25AM +0200, Miquel Raynal wrote:
> > On Fri, Apr 07, 2023 at 10:10:43AM +0200, Miquel Raynal wrote:
> > > I've looked at the different patches, they look good to me but as they
> > > are all trivial and exactly identical, would you mind sending this
> > > again all squashed in a single patch? A subsystem-wide conversion seems
> > > appropriate. In all cases I plan to take this for the next merge
> > > window.  
> > 
> > I slightly prefer them separately, because I like small patches and
> > because the Acks and Reviews only apply to the individual drivers.
> > But I don't mind seriously, so here comes the series squashed into one.
> 
> For any non trivial change, I would definitely do that as well.

A patch is only trivial as long as it doesn't do anything wrong.
I don't assume any problem, but it wouldn't be the first. The first was
bcm2835_spi_remove() being used in bcm2835_spi_shutdown().

> The thing is, by collecting the tags with b4, you lost all the Acks and
> Reviews targets, while we could prevent this, see below.
> 
> > While going through the changed, probably the s3c24xx driver (which
> > isn't exactly identical to the other changes) could benefit from an
> > additional change throwing out the early exit (which---I guess---cannot
> > be hit).
> 
> Yes, I believe the 'info == NULL' condition is useless, feel free to
> drop it in a second patch if you wish.

Yeah.

> > BTW, I constructed the lists of acks/reviews myself and found the same
> > set. However b4 wailed about each patch claiming:
> > 
> > 	    ✗ BADSIG: DKIM/infradead.org
> 
> No idea what this means, any pointer?

lists.infradead.org signs all outgoing mail and something between these
outgoing mails and b4 consuming them breaks the signature. I didn't
debug, but depending on where the problem is, either
postmaster@lists.infradead.org or the admins of lore.kernel.org or the
maintainer of b4 might want to know and fix.

> > And it didn't like you producing the tags, saying:
> > 
> > 	NOTE: some trailers ignored due to from/email mismatches:
> > 	    ! Trailer: Acked-by: Roger Quadros <rogerq@kernel.org>
> > 	     Msg From: Miquel Raynal <miquel.raynal@bootlin.com>
> > 	    [...]
> 
> Well, yes, I don't expect b4 to read plain english when I say "I
> collected them for you" ^^ But at least my list had a '# <area>' suffix
> for each of the Acked and Reviewed changes, which is now missing. I
> don't know  how useful they actually are, but it seems to me that the
> information was lost between v1 and v2?

That's right. I will resend in a separate thread with the annotations
fixed. Sorry for the noise.

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 --]