Re: [PATCH] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul <vkoul@xxxxxxxxxx>]

On 03-12-21, 16:45, David Collins wrote:
> On 12/2/21 7:13 PM, Stephen Boyd wrote:
> > Quoting David Collins (2021-12-02 15:51:18)
> >> On 12/2/21 3:06 PM, Stephen Boyd wrote:
> >>> Quoting Vinod Koul (2021-11-30 23:27:18)

> > It feels like we should make a parent node that holds the core, chnls,
> > obsrvr reg properties with a new compatible string and then have two
> > child nodes for each bus. Then the various PMICs can hang off those two
> > different bus nodes. Of course, it needs DT review to make sure nothing
> > else goes wrong.
> 
> We considered this alternative DT layout when implementing PMIC arbiter
> v7 support downstream.  The benefit is allowing common register ranges
> to be specified once instead of in both SPMI bus nodes.  However, this
> approach has several downsides.
> 
> It results in substantially more complex device tree binding
> documentation that requires a different layout for SPMI buses for PMIC
> arbiter v7 (and above) vs early versions even though support can be
> provided with only a minimal modification (i.e. "qcom,bus-id").
> Complexity is also increased inside of the spmi-pmic-arb driver.  There,
> special data structures and logic would be needed to handle the shared
> vs independent register addresses and data.
> 
> The SPMI framework currently uses a one-to-one mapping between SPMI
> buses and struct device pointers.  This would not work if the new
> top-level node represents the true struct device and the per-bus
> subnodes are not true devices.  Also, platform_get_resource_byname()
> (used in the spmi-pmic-arb probe function) needs a struct
> platform_device pointer to read address ranges using "reg" +
> "reg-names".  That wouldn't work for the subnodes.
> 
> I suppose that "compatible" could be specified for the top-level node
> and the per bus subnodes so that all three get probed as struct devices.
>  However, that makes things even more complicated and convoluted in the
> driver (and DT).
> 
> I would prefer to stay with the approach of the two bus instances being
> specified independently in DT.
> 
> Note that the clk-imx8qxp-lpcg driver has a similar situation where
> multiple drivers need to map addresses in the same region.  Commit [1]
> documents the requirement.  The details of the problem where
> devm_platform_ioremap_resource() cannot be used in place of
> devm_ioremap() were discussed in this thread [2].

Steve, David,

Is this the way we are recommending for this to be move forward with?

-- 
~Vinod