From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 925001F940B for ; Mon, 13 Jan 2025 21:52:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736805171; cv=none; b=cYz5rTl5AkhcwE7ZswQfrjl5wxqdbfmv0vqqXP6ocqZ1wezqqjB6Gct2z5ETtzoHBaVwGKbA1NJmXpMJtCUW4aqHhi8vRRsiv/h5nK1lZZ3wvrMpOg5tUAI02pW31erBIrdxxqslUqxbp89SSpFXpfdVBh60knnEIiVyapWOzl0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736805171; c=relaxed/simple; bh=lqRXAoXBKW4gFS0Jtek0gypvT9cjk5SylvVOg20JUA4=; h=Message-ID:Content-Type:MIME-Version:In-Reply-To:References: Subject:From:Cc:To:Date; b=Qkl5YDGci+j4He+cD/6fXVm077mjho8vaZ2pN744l46gaYSOfiz7hdRU3vBOiHvY4qWizkGxeDrCBWiTlQSmPrWJizpbZTIbnhTVA5Yv/numUXj+spHUnXSUDGNxsu20KfTfeHvPVcBAljKoON3kWW2AvT1OC/eGgDGdzBRyP9A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=iVCJ5jPo; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="iVCJ5jPo" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EE709C4CED6; Mon, 13 Jan 2025 21:52:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736805171; bh=lqRXAoXBKW4gFS0Jtek0gypvT9cjk5SylvVOg20JUA4=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=iVCJ5jPoWdQ8U4RVlbfiLnCP+Kmg2OM0YPEifRYUCun7oHE5IpE6zApYDve5HBkYe CRqAene4BGmE5Mw2wi7fd59nJvan4sQ0NhNbj3+1KLFsaLHkJCsiW2A42r70ES3lNp ch3x0z7PEwKxCfk4JLY+qBdgxwkSKacBUTFD0VkxPWnADdmKIMxTXeKUTX7HiA7g3G s9hK/NzNNRYyS4gkKtpveiNl3GVklBxd3IaLN15jP2ZAOfM5CtSK4AwkNNj6XRgAm5 4l66FKAqVaAYmv5ZAvpgySwy5OHXWjELUklX1YogkNhZWRdQC5sSl5gAfzkftF0Ogv a3kNXM3/OJyvQ== Message-ID: <5b139af8d639e20a14c8bd00e8e03a4a.sboyd@kernel.org> Content-Type: text/plain; charset="utf-8" Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20250113-topic-spmi_node_breakage-v2-1-dd35a3a6daa6@oss.qualcomm.com> References: <20250113-topic-spmi_node_breakage-v2-1-dd35a3a6daa6@oss.qualcomm.com> Subject: Re: [PATCH v2] spmi: Fix controller->node != parent->node breakage From: Stephen Boyd Cc: Marijn Suijten , linux-kernel@vger.kernel.org, Bjorn Andersson , Abel Vesa , Johan Hovold , linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, Konrad Dybcio To: AngeloGioacchino Del Regno , Joe Hattori , Konrad Dybcio , Matthias Brugger , Mauro Carvalho Chehab Date: Mon, 13 Jan 2025 13:52:48 -0800 User-Agent: alot/0.12.dev1+gaa8c22fdeedb Quoting Konrad Dybcio (2025-01-13 05:02:58) > From: Konrad Dybcio >=20 > On some platforms, like recent Qualcomm SoCs with multi-bus SPMI > arbiters, controller->node must be assigned to the individual buses' > subnodes, as the slave devices are children of these, like so: >=20 > arbiter@c400000 > spmi@c42d000 > pmic@0 >=20 > spmi@c432000 > pmic@0 >=20 > The commit referenced in Fixes changed that assignment, such that > spmi_controller_alloc() always assumes the PMICs come directly under > the arbiter node (which is true when there's only a single bus per > controller). >=20 > Make controller->node specifiable to both benefit from Joe's refcount > improvements and un-break the aforementioned platforms. How is it broken? I see spmi_pmic_arb_bus_init() calls devm_spmi_controller_alloc() which sets the of_node to the parent device and then spmi_pmic_arb_bus_init() overwrites that with 'ctrl->dev.of_node =3D node' later on in the same function. That will cause one more of_node_put() than is expected. I don't see that removed in this patch though, so the leak is still there? >=20 > Fixes: 821b07853e32 ("spmi: hisi-spmi-controller: manage the OF node refe= rence in device initialization and cleanup") I've dropped this patch from my queue. I don't know if we're really doing anything better by managing the of_node lifetime in that function vs. letting the callers assign the node they want and manage the lifetime themselves. Maybe we don't need to do anything? Presumably the parent device driver will unregister the controller anyway, so the lifetime of the of_node will be ensured regardless. For subnodes like qcom SPMI, the subnodes are child nodes of the parent device so they won't be removed. If they are dynamic nodes, then the caller can manage the lifetime.