Re: [PATCH v6 2/8] regulator: dt-bindings: Document max8997-pmic nodes

From: Timon Baetz <timon.baetz@protonmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Lee Jones <lee.jones@linaro.org>,
	Sebastian Reichel <sre@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org,
	~postmarketos/upstreaming@lists.sr.ht
Subject: Re: [PATCH v6 2/8] regulator: dt-bindings: Document max8997-pmic nodes
Date: Fri, 08 Jan 2021 15:16:48 +0000	[thread overview]
Message-ID: <20210108161635.1b9303c8.timon.baetz@protonmail.com> (raw)
In-Reply-To: <20210106145931.GE4752@sirena.org.uk>

On Wed, 6 Jan 2021 14:59:31 +0000, Mark Brown wrote:
> On Tue, Jan 05, 2021 at 05:55:29PM +0100, Krzysztof Kozlowski wrote:
> > On Mon, Jan 04, 2021 at 09:24:49PM +0000, Mark Brown wrote:  
> 
> > > I'm not sure I follow, sorry?  Either the core driver can parse the
> > > bindings enough to know what children it has or (probably better) it can
> > > instantiate the children unconditionally and then the function drivers
> > > can figure out if they need to do anything.  
> 
> > Currently the MFD parent/core driver will instantiate children
> > unconditionally.  It would have to be adapted. With proposed bindings -
> > nothing to change.  MFD core already does the thing.  
> 
> We're not talking massive amounts of code here, but we are talking ABI
> for a DT update.
> 
> > The point is that function drivers should not be even bound, should not
> > start to probe. Otherwise if they probe and fail, they will pollute the
> > dmesg/probe log with failure. With the failure coming from looking for
> > missing of_node or any other condition from parent/core driver.  
> 
> There will only be an error message if one is printed, if we can do a
> definitive -ENODEV there should be no need to print an error.
> 
> > > > Another point, is that this reflects the real hardware. The same as we
> > > > model entire SoC as multiple children of soc node (with their own
> > > > properties), here we represent smaller chip which also has
> > > > sub-components.  
> 
> > > Components we're calling things like "extcon"...  
> 
> > I am rather thinking about charger, but yes, extcon as well. Either you
> > have USB socket (and want to use associated logic) or not.  
> 
> Right, I'm just saying we don't need to add new device nodes reflecting
> implementation details into the DT to do that.

I'm not sure I can contribute that much to this discussion (this is my
first proper kernel patch, also I don't really understand the argument).
FWIW I looked at other MFD devices while implementing this like max77836, 
max77693, max77650, max77843 (just to name a few). 
Assigning of_node to sub-devices using sub-nodes with compatible strings 
seemed to be a common pattern for MFD devices.
Muic needs a node to be used with extcon_get_edev_by_phandle().
Charger needs a node to reference a regulator.