From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> To: Andre Przywara <andre.przywara@arm.com> 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: Thu, 16 Feb 2023 09:15:56 +0100 [thread overview] Message-ID: <fa013fbc-dc54-6d20-7935-2ce01afc37be@linaro.org> (raw) In-Reply-To: <20230214181138.4a8f4beb@donnerap.cambridge.arm.com> On 14/02/2023 19:11, Andre Przywara wrote: > 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. I would say it is quite obvious. Otherwise you could remove entire binding and still submit the driver, right? Isn't the entire point of the binding to match what the driver is doing, as it is the description of interface used by driver towards DTS? > > 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. Sure. To me it tells - this patch is a no-go and driver should be fixed. Best regards, Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> To: Andre Przywara <andre.przywara@arm.com> 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: Thu, 16 Feb 2023 09:15:56 +0100 [thread overview] Message-ID: <fa013fbc-dc54-6d20-7935-2ce01afc37be@linaro.org> (raw) In-Reply-To: <20230214181138.4a8f4beb@donnerap.cambridge.arm.com> On 14/02/2023 19:11, Andre Przywara wrote: > 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. I would say it is quite obvious. Otherwise you could remove entire binding and still submit the driver, right? Isn't the entire point of the binding to match what the driver is doing, as it is the description of interface used by driver towards DTS? > > 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. Sure. To me it tells - this patch is a no-go and driver should be fixed. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-02-16 8:16 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-13 23:19 [PATCH 0/3] Enable hwlock on Allwinner A64 Bastian Germann 2023-02-13 23:19 ` Bastian Germann 2023-02-13 23:19 ` [PATCH 1/3] dt-bindings: hwlock: sun6i: Add missing #hwlock-cells Bastian Germann 2023-02-13 23:19 ` Bastian Germann 2023-02-14 0:04 ` Andre Przywara 2023-02-14 0:04 ` Andre Przywara 2023-02-14 8:08 ` Krzysztof Kozlowski 2023-02-14 8:08 ` Krzysztof Kozlowski 2023-02-14 16:12 ` Rob Herring 2023-02-14 16:12 ` Rob Herring 2023-02-15 14:07 ` Rob Herring 2023-02-15 14:07 ` Rob Herring 2023-02-16 8:39 ` Bastian Germann 2023-02-16 8:39 ` Bastian Germann 2023-02-13 23:19 ` [PATCH 2/3] dt-bindings: hwlock: sun6i: Add missing names Bastian Germann 2023-02-13 23:19 ` Bastian Germann 2023-02-14 0:07 ` Andre Przywara 2023-02-14 0:07 ` Andre Przywara 2023-02-14 8:09 ` Krzysztof Kozlowski 2023-02-14 8:09 ` Krzysztof Kozlowski 2023-02-14 9:45 ` Wilken Gottwalt 2023-02-14 9:45 ` Wilken Gottwalt 2023-02-14 12:12 ` Andre Przywara 2023-02-14 12:12 ` Andre Przywara 2023-02-14 12:39 ` Krzysztof Kozlowski 2023-02-14 12:39 ` Krzysztof Kozlowski 2023-02-14 12:37 ` Krzysztof Kozlowski 2023-02-14 12:37 ` Krzysztof Kozlowski 2023-02-14 18:11 ` Andre Przywara 2023-02-14 18:11 ` Andre Przywara 2023-02-16 8:15 ` Krzysztof Kozlowski [this message] 2023-02-16 8:15 ` Krzysztof Kozlowski 2023-02-14 12:38 ` Krzysztof Kozlowski 2023-02-14 12:38 ` Krzysztof Kozlowski 2023-02-13 23:19 ` [PATCH 3/3] arm64: dts: allwinner: a64: Add hwspinlock node Bastian Germann 2023-02-13 23:19 ` Bastian Germann 2023-02-14 0:12 ` Andre Przywara 2023-02-14 0:12 ` Andre Przywara 2023-02-14 0:18 ` [PATCH 0/3] Enable hwlock on Allwinner A64 Andre Przywara 2023-02-14 0:18 ` Andre Przywara 2023-02-14 21:55 ` Bastian Germann 2023-02-14 21:55 ` Bastian Germann 2023-02-15 5:01 ` Wilken Gottwalt 2023-02-15 5:01 ` Wilken Gottwalt 2023-02-14 20:57 ` (subset) " Bjorn Andersson 2023-02-14 20:57 ` Bjorn Andersson 2023-02-15 22:31 ` Andre Przywara 2023-02-15 22:31 ` Andre Przywara
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fa013fbc-dc54-6d20-7935-2ce01afc37be@linaro.org \ --to=krzysztof.kozlowski@linaro.org \ --cc=andersson@kernel.org \ --cc=andre.przywara@arm.com \ --cc=bage@debian.org \ --cc=baolin.wang@linux.alibaba.com \ --cc=devicetree@vger.kernel.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-remoteproc@vger.kernel.org \ --cc=linux-sunxi@lists.linux.dev \ --cc=ohad@wizery.com \ --cc=robh+dt@kernel.org \ --cc=samuel@sholland.org \ --cc=wens@csie.org \ --cc=wilken.gottwalt@posteo.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.