From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johan Hovold Subject: Re: [PATCH 1/4] regulator: qcom_spmi: Fix warning Bad of_node_put() Date: Mon, 16 Jul 2018 16:23:10 +0200 Message-ID: <20180716142310.GQ10204@localhost> References: <20180716113525.9335-1-niklas.cassel@linaro.org> <20180716113525.9335-2-niklas.cassel@linaro.org> <20180716120134.GL10204@localhost> <20180716132931.GA866@centauri.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20180716132931.GA866@centauri.lan> Sender: linux-kernel-owner@vger.kernel.org To: Niklas Cassel Cc: Johan Hovold , broonie@kernel.org, Liam Girdwood , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org On Mon, Jul 16, 2018 at 03:29:31PM +0200, Niklas Cassel wrote: > On Mon, Jul 16, 2018 at 02:01:34PM +0200, Johan Hovold wrote: > > Hi Niklas, > > > > On Mon, Jul 16, 2018 at 01:35:22PM +0200, Niklas Cassel wrote: > > > For of_find_node_by_name(), you typically pass what the previous call > > > returned. Therefore, of_find_node_by_name() increases the refcount of > > > the returned node, and decreases the refcount of the node passed as the > > > first argument. > > > > > > However, in this case we don't pass what the previous call returned, > > > so we have to increase the refcount of the first argument to compensate. > > > > I don't think this is the right fix. of_find_node_by_name() should > > generally not be used by drivers in the first place as it searches the > > entire tree and can end up matching an entirely unrelated node. > > > > I haven't looked at the device-tree binding in question, but you > > probably want to use something like of_get_child_by_name() instead. > > > > Hello Johan, > > of_find_node_by_name() will only search the whole tree if the > first argument is NULL, which isn't the case here. It's searching the entire tree *starting* at its first argument, which means you may end up matching a completely unrelated node (i.e. not a child or even descendant) elsewhere in the tree. > However, of_get_child_by_name() is indeed better suited here. > Will send out a v2. Unless you are doing a tree-wide search, using of_get_child_by_name() is simply wrong. I fixed up most of these bugs a few releases ago, but they keep on creeping in. > Thank you for your feedback, it is much appreciated :) No worries. Johan