Re: [PATCH v2] spmi: Fix controller->node != parent->node breakage

[Stephen Boyd <sboyd@kernel.org>]

Quoting Konrad Dybcio (2025-01-13 05:02:58)
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> 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:
> 
> arbiter@c400000
>         spmi@c42d000
>                 pmic@0
> 
>         spmi@c432000
>                 pmic@0
> 
> 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).
> 
> 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 = 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?

> 
> Fixes: 821b07853e32 ("spmi: hisi-spmi-controller: manage the OF node reference 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.