Re: [PATCH 2/3] dt-bindings: hwlock: sun6i: Add missing names

From: Andre Przywara <andre.przywara@arm.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Wilken Gottwalt <wilken.gottwalt@posteo.net>,
	Bastian Germann <bage@debian.org>,
	Ohad Ben-Cohen <ohad@wizery.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Chen-Yu Tsai <wens@csie.org>,
	Samuel Holland <samuel@sholland.org>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: hwlock: sun6i: Add missing names
Date: Tue, 14 Feb 2023 18:11:38 +0000	[thread overview]
Message-ID: <20230214181138.4a8f4beb@donnerap.cambridge.arm.com> (raw)
In-Reply-To: <43336959-643d-4041-d11a-b0c030fbf050@linaro.org>

On Tue, 14 Feb 2023 13:37:20 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi Krzysztof,

> On 14/02/2023 10:45, Wilken Gottwalt wrote:
> > On Tue, 14 Feb 2023 00:19:29 +0100
> > Bastian Germann <bage@debian.org> wrote:
> >   
> >> The allwinner,sun6i-a31-hwspinlock.yaml binding needs clock-names
> >> and reset-names set to "ahb" as required by the driver.  
> > 
> > Hmm, this one is a bit odd. If you look into my earlier versions of the
> > patchset, you may notice, that I actually included these bindings and they
> > were refused. I think the argumentation was like
> > "there is only one bus = no need for it".
> > 
> > If it gets accepted now, I really like to know why. (It was some trouble
> > back then to get the documentation properly done and accepted.)  
> 
> The clock names and resent names are not correct. They should have never
> been added. If you got comments about this and did not update driver,
> that's not nice. You just shoved incomplete bindings. :(
> 
> So indeed to avoid precedence - people pushing fake bindings and
> avoiding review - NAK on this.

Maybe it's just me, but I don't think this tone is necessary.

Wilken's original submission was correct. Later there was a comment just
on the binding patch, to remove the not needed clock-names and reset-names
properties. But there was not a word in there that the driver requires
changing as well, and I don't think it's fair to blame Wilken on this, or
somewhat even implying intention. There were several patch revisions after
this was raised, and this just slipped through review. But surely no one
wanted this or pushed for that.

If anything, it tells us that we should be more careful when merging
drivers without users: if there would have been a DT patch, possibly even
a consumer, this would have been flagged by dtbs_check.

Cheers,
Andre