From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 90899C5CB for ; Tue, 14 Feb 2023 18:11:43 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 68B7E1042; Tue, 14 Feb 2023 10:12:25 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 049FA3F663; Tue, 14 Feb 2023 10:11:40 -0800 (PST) Date: Tue, 14 Feb 2023 18:11:38 +0000 From: Andre Przywara To: Krzysztof Kozlowski Cc: Wilken Gottwalt , Bastian Germann , Ohad Ben-Cohen , Bjorn Andersson , Baolin Wang , Rob Herring , Krzysztof Kozlowski , Chen-Yu Tsai , Samuel Holland , 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 Message-ID: <20230214181138.4a8f4beb@donnerap.cambridge.arm.com> In-Reply-To: <43336959-643d-4041-d11a-b0c030fbf050@linaro.org> References: <20230213231931.6546-1-bage@debian.org> <20230213231931.6546-3-bage@debian.org> <20230214104554.35338faa@posteo.net> <43336959-643d-4041-d11a-b0c030fbf050@linaro.org> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 14 Feb 2023 13:37:20 +0100 Krzysztof Kozlowski wrote: Hi Krzysztof, > On 14/02/2023 10:45, Wilken Gottwalt wrote: > > On Tue, 14 Feb 2023 00:19:29 +0100 > > Bastian Germann 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6B7B9C61DA4 for ; Tue, 14 Feb 2023 18:12:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=13w7jmBPH4QVhNkO2idoIyN9SztNjIkzU4iZ03SC3DM=; b=NuvK0KrHKWiGO0 7jpSqrXmF6br0ZsMlPux4IaTQ3W0oFrZtL44zr6ZJuwO1rt/GRZvJJ/HvbCitd+QrJXWL0Ts6wYxo lbu9kvAK3ldBx/JPQN3HF7QBOcoj3ZElw4rZF6qT6B8NajMRDyibTE0UyNKTggjN4UzlTwykjczJK 0A64m6iVIMy5FzeZ/YuF7Z9xtnxbz3LKHw2VMQYY1iQtTWLyc7l/51p2nAS1PcLaVxqMGzVdUhQIS koaNmYK+/lCMdrlgSIxoNyYxr+co0GFvoqVIAk8a0cgpgM9aXh4QiBgzNO6yJrPpW8blXp0L3wS52 96QVckkqZgPsD7sKxL8w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pRzmQ-0037QT-8A; Tue, 14 Feb 2023 18:11:50 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pRzmM-0037Ow-KK for linux-arm-kernel@lists.infradead.org; Tue, 14 Feb 2023 18:11:48 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 68B7E1042; Tue, 14 Feb 2023 10:12:25 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 049FA3F663; Tue, 14 Feb 2023 10:11:40 -0800 (PST) Date: Tue, 14 Feb 2023 18:11:38 +0000 From: Andre Przywara To: Krzysztof Kozlowski Cc: Wilken Gottwalt , Bastian Germann , Ohad Ben-Cohen , Bjorn Andersson , Baolin Wang , Rob Herring , Krzysztof Kozlowski , Chen-Yu Tsai , Samuel Holland , 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 Message-ID: <20230214181138.4a8f4beb@donnerap.cambridge.arm.com> In-Reply-To: <43336959-643d-4041-d11a-b0c030fbf050@linaro.org> References: <20230213231931.6546-1-bage@debian.org> <20230213231931.6546-3-bage@debian.org> <20230214104554.35338faa@posteo.net> <43336959-643d-4041-d11a-b0c030fbf050@linaro.org> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230214_101146_750778_B18FE71D X-CRM114-Status: GOOD ( 20.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 14 Feb 2023 13:37:20 +0100 Krzysztof Kozlowski wrote: Hi Krzysztof, > On 14/02/2023 10:45, Wilken Gottwalt wrote: > > On Tue, 14 Feb 2023 00:19:29 +0100 > > Bastian Germann 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel