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

[Scott Wood <scottwood@freescale.com>]

On Wed, 2015-12-09 at 03:05 -0600, Lu Yangbo-B47093 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, December 09, 2015 12:33 PM
> > 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 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
> 
> [Lu Yangbo-B47093] Ok, I'd like to try. But can you suggest where we should
> put the guts driver in kernel?
> Thanks.

drivers/soc/fsl/

-Scott