diff mbox

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

Message ID 1448594417-22515-5-git-send-email-yangbo.lu@freescale.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

yangbo lu Nov. 27, 2015, 3:20 a.m. UTC
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(-)

Comments

Scott Wood Dec. 8, 2015, 5:46 p.m. UTC | #1
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

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Dec. 9, 2015, 4:33 a.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Dec. 9, 2015, 5 p.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Dec. 9, 2015, 5:01 p.m. UTC | #4
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

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Dec. 11, 2015, 8:26 a.m. UTC | #5
[...]

>> > > 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/
>

Okay, go ahead and have a try.

BTW, there is actually also a corresponding include directory to use

/include/linux/soc/..

Please keep me posted.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Dec. 11, 2015, 8:29 a.m. UTC | #6
On Fri, 2015-12-11 at 09:26 +0100, Ulf Hansson wrote:
> [...]
> 
> > > > > 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/
> > 
> 
> Okay, go ahead and have a try.
> 
> BTW, there is actually also a corresponding include directory to use
> 
> /include/linux/soc/..

OK, but we already have an include/linux/fsl and not an include/linux/soc/fsl.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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";
 		reg = <0xe0000 0xe00>;
 		fsl,has-rstcr;
 		fsl,liodn-bits = <12>;