Re: [v3, 4/5] powerpc: dts: t4240: add syscon support for DCFG node

[Scott Wood <scottwood@freescale.com>]

On Tue, 2015-12-08 at 22:30 -0600, Lu Yangbo-B47093 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, December 09, 2015 1:46 AM
> > To: Lu Yangbo-B47093; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org
> > Cc: Xie Xiaobo-R63061; Li Yang-Leo-R58472
> > Subject: Re: [v3, 4/5] powerpc: dts: t4240: add syscon support for DCFG
> > node
> > 
> > On Fri, 2015-11-27 at 11:20 +0800, Yangbo Lu wrote:
> > > Add syscon support for DCFG node, so that the driver could use syscon
> > > regmap interface to access the device config module registers.
> > > And the CONFIG_MFD_SYSCON should be enabled for this.
> > > 
> > > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com>
> > > ---
> > > Changes for v2:
> > > 	- None
> > > Changes for v3:
> > > 	- Added this patch
> > > ---
> > >  arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> > > b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> > > index 68c4ead..5f148b2 100644
> > > --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> > > +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> > > @@ -941,7 +941,9 @@
> > >  /include/ "qoriq-mpic4.3.dtsi"
> > > 
> > >  	guts: global-utilities@e0000 {
> > > -		compatible = "fsl,t4240-device-config", "fsl,qoriq
> > > -device
> > > -config-2.0";
> > > +		compatible = "fsl,t4240-device-config",
> > > +				"fsl,qoriq-device-config-2.0",
> > > +				"syscon";
> > 
> > 
> > I really don't like changing the device tree based on Linux internals.
> > It also means that the workaround wouldn't work for users that don't
> > upgrade their device tree.  I definitely don't like one QorIQ chip having
> > "syscon" on the dcfg node but others not having it.
> > 
> > The guts driver should just maintain a list of compatibles to match.  Why
> > do we need to use syscon, rather than a guts driver that exports an
> > fsl_get_svr() function?
> > 
> > -Scott
> 
> [Lu Yangbo-B47093] I think the only difference between fsl_get_svr() and
> syscon is that we don’t need to change dts with fsl_get_svr(), right?

That is one difference.  It also would simplify callers, and avoid the need to
depend on the overcomplicated and difficult-to-follow regmap code in order to
accomplish something very simple.

> The syscon has considered the endianess, and we don’t need to add more code
> to use it and only add 'syscon' in the node.

Dealing with the endianess in a common guts driver would be trivial.

> This patchset only enables syscon in T4240 DFCG node since it has an erratum
> to use it.

The device tree describes the hardware, not what you want to use it for.  The
existence of an erratum in the esdhc block says nothing about what the dcfg
node is.

> The fsl_get_svr() you suggested is also an idea for this.
> Then, may I know is there a guts driver in kernel?
> Thanks a lot. 

There isn't, but there should be.

-Scott