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:29 -0600, Lu Yangbo-B47093 wrote:
> > -----Original Message-----
> > From: Lu Yangbo-B47093
> > Sent: Wednesday, December 09, 2015 5:05 PM
> > To: Wood Scott-B07421; 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
> > 
> > > -----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.
> > 
> [Lu Yangbo-B47093] Actually, most platforms would need to be added 'syscon'
> compatible in DFCG node later if we use SYSCON in eSDHC.
> Because most of eSDHC errata only could be identified by SVR.
> But for this patchset, for this T4240 eSDHC erratum, I only add 'syscon' for
> T4240.
> 
> What about adding 'syscon' in all platforms although there is some workload?

Please just add a guts driver.

-Scott