Date:   Tue, 7 Dec 2021 17:22:30 +0530
From:   Vinod Koul <vkoul@kernel.org>
To:     David Collins <quic_collinsd@quicinc.com>
Cc:     Stephen Boyd <sboyd@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-arm-msm@vger.kernel.org,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
Subject: Re: [PATCH] spmi: pmic-arb: Add support for PMIC v7
Message-ID: <Ya9K/sF1YDCYCp2Y@matsya>
References: <20211201072718.3969011-1-vkoul@kernel.org>
 <20211202230624.C05F3C00446@smtp.kernel.org>
 <9161450a-40e0-c84f-f529-c903d6f1d722@quicinc.com>
 <20211203031323.211E5C00446@smtp.kernel.org>
 <1fb0553b-eca5-a537-4dc3-77437feffc69@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1fb0553b-eca5-a537-4dc3-77437feffc69@quicinc.com>
Precedence: bulk

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