Re: [PATCH] mtd: nand: Convert to platform remove callback returning void - Uwe Kleine-König

From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Valentin Korenblit" <vkorenblit@sequans.com>,
	"Lucas Stach" <dev@lynxeye.de>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
	"Stefan Agner" <stefan@agner.ch>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Liang Yang" <liang.yang@amlogic.com>,
	"Naga Sureshkumar Relli" <nagasure@xilinx.com>,
	"Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"Han Xu" <han.xu@nxp.com>, "Chuanhong Guo" <gch981213@gmail.com>,
	linux-stm32@st-md-mailman.stormreply.com,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Marek Behún" <kabel@kernel.org>,
	"Samuel Holland" <samuel@sholland.org>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	linux-rockchip@lists.infradead.org,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Miaoqian Lin" <linmq006@gmail.com>,
	"Yang Yingliang" <yangyingliang@huawei.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Jack Wang" <jinpu.wang@ionos.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	linux-sunxi@lists.linux.dev,
	"ye xingchen" <ye.xingchen@zte.com.cn>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	linux-arm-msm@vger.kernel.org,
	"Vladimir Zapolskiy" <vz@mleia.com>,
	"Roger Quadros" <rogerq@kernel.org>,
	"Wang Weiyang" <wangweiyang2@huawei.com>,
	"Harvey Hunt" <harveyhuntnexus@gmail.com>,
	linux-mediatek@lists.infradead.org,
	linux-mtd@lists.infradead.org, linux-tegra@vger.kernel.org,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Christophe Kerello" <christophe.kerello@foss.st.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	linux-mips@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	kernel@pengutronix.de, "Richard Weinberger" <richard@nod.at>,
	"Christophe JAILLET" <christophe.jaillet@wanadoo.fr>,
	linux-oxnas@groups.io, "Pali Rohár" <pali@kernel.org>,
	"Claudiu Beznea" <claudiu.beznea@microchip.com>
Subject: Re: [PATCH] mtd: nand: Convert to platform remove callback returning void
Date: Tue, 11 Apr 2023 13:11:48 +0200	[thread overview]
Message-ID: <20230411111148.yefmzohsvk3facq3@pengutronix.de> (raw)
In-Reply-To: <20230411102025.6b2fdc9e@xps-13>

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