Re: [PATCH v5 1/6] dt-bindings: thermal: mediatek: Rename thermal zone definitions for MT8186 and MT8188

From: Conor Dooley <conor@kernel.org>
To: Julien Panis <jpanis@baylibre.com>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Nicolas Pitre <npitre@baylibre.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH v5 1/6] dt-bindings: thermal: mediatek: Rename thermal zone definitions for MT8186 and MT8188
Date: Mon, 27 May 2024 18:28:56 +0100	[thread overview]
Message-ID: <20240527-wife-dress-3ecaf1509506@spud> (raw)
In-Reply-To: <61a1be10-7df3-4ba7-b7b4-ccc7f0379656@baylibre.com>

[-- Attachment #1.1: Type: text/plain, Size: 1747 bytes --]

On Mon, May 27, 2024 at 05:25:35PM +0200, Julien Panis wrote:
> On 5/24/24 20:27, Conor Dooley wrote:
> > On Fri, May 24, 2024 at 07:24:47PM +0100, Conor Dooley wrote:
> > > On Fri, May 24, 2024 at 11:04:34AM +0200, Julien Panis wrote:
> > > > Use thermal zone names that make more sense.
> > > > 
> > > > Signed-off-by: Julien Panis <jpanis@baylibre.com>
> > > Removing the defines is an ABI break. If these are all the same devices,
> > > but with more accurate naming, then keep the old defines and add new
> > > ones. However, the GPU1 define changes in the course of this patch which
> > > is more problematic.
> > > > [RFC] When PATCH 1/6 and 2/6 are squashed, checkpatch raises this WARNING:
> > > > "DT binding docs and includes should be a separate patch." That's why I
> > > > split them in this v5. The problem is that the driver can't be compiled
> > > > any more at PATCH 1/6. It needs PATCH 2/6 to be compiled. Should the
> > > > checkpatch warning be ignored here ? Should I finally squash PATCH 1/6
> > > > and PATCH 2/6 ?
> > Heh, and there's just one of the issues caused by your ABI break...
> 
> Conor,
> 
> Would Russell's suggestion be acceptable for you ?
> I mean, this one:
> https://lore.kernel.org/all/ZlDMNkdE2jmFgD8B@shell.armlinux.org.uk/
> 
> I could implement it, but before submitting it I would like to make
> sure that it suits everyone.

How's that going to work? MT8188_AP_GPU1 currently means 1, after your
series it means 2.
You're gonna need to pick a different naming for the new defines to
avoid that. Additionally, why even delete the old ones? Just define
new names with the same numbering and you don't need to worry about
any compatibility issues.

Thanks,
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel