Patchwork [1/2] ARM: mach-imx: Remove board entries in dt_board_compat

login
register
mail settings
Submitter Fabio Estevam
Date June 17, 2012, 2:04 p.m.
Message ID <1339941856-2836-1-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/165344/
State New
Headers show

Comments

Fabio Estevam - June 17, 2012, 2:04 p.m.
From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need for adding board related entries into dt_board_compat.

Leave only the SoC entry.

This way we do not need to patch a C file when adding dt support for a new board.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/mach-imx/imx51-dt.c   |    1 -
 arch/arm/mach-imx/imx53-dt.c   |    4 ----
 arch/arm/mach-imx/mach-imx6q.c |    3 ---
 3 files changed, 0 insertions(+), 8 deletions(-)
Sascha Hauer - July 5, 2012, 7:13 a.m.
Shawn,

Is this ok with you?

Sascha

On Sun, Jun 17, 2012 at 11:04:15AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There is no need for adding board related entries into dt_board_compat.
> 
> Leave only the SoC entry.
> 
> This way we do not need to patch a C file when adding dt support for a new board.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/mach-imx/imx51-dt.c   |    1 -
>  arch/arm/mach-imx/imx53-dt.c   |    4 ----
>  arch/arm/mach-imx/mach-imx6q.c |    3 ---
>  3 files changed, 0 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
> index 18e78db..5f10812 100644
> --- a/arch/arm/mach-imx/imx51-dt.c
> +++ b/arch/arm/mach-imx/imx51-dt.c
> @@ -106,7 +106,6 @@ static struct sys_timer imx51_timer = {
>  };
>  
>  static const char *imx51_dt_board_compat[] __initdata = {
> -	"fsl,imx51-babbage",
>  	"fsl,imx51",
>  	NULL
>  };
> diff --git a/arch/arm/mach-imx/imx53-dt.c b/arch/arm/mach-imx/imx53-dt.c
> index eb04b62..75535f9 100644
> --- a/arch/arm/mach-imx/imx53-dt.c
> +++ b/arch/arm/mach-imx/imx53-dt.c
> @@ -132,10 +132,6 @@ static struct sys_timer imx53_timer = {
>  };
>  
>  static const char *imx53_dt_board_compat[] __initdata = {
> -	"fsl,imx53-ard",
> -	"fsl,imx53-evk",
> -	"fsl,imx53-qsb",
> -	"fsl,imx53-smd",
>  	"fsl,imx53",
>  	NULL
>  };
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index b47e98b..1796be4 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -173,9 +173,6 @@ static struct sys_timer imx6q_timer = {
>  };
>  
>  static const char *imx6q_dt_compat[] __initdata = {
> -	"fsl,imx6q-arm2",
> -	"fsl,imx6q-sabrelite",
> -	"fsl,imx6q-sabresd",
>  	"fsl,imx6q",
>  	NULL,
>  };
> -- 
> 1.7.1
> 
>
Shawn Guo - July 5, 2012, 7:46 a.m.
On Thu, Jul 05, 2012 at 09:13:18AM +0200, Sascha Hauer wrote:
> Shawn,
> 
> Is this ok with you?
> 
No.

http://thread.gmane.org/gmane.linux.ports.arm.kernel/172558/focus=172703
Shawn Guo - Aug. 13, 2012, 3:15 p.m.
On Thu, Jul 05, 2012 at 03:46:23PM +0800, Shawn Guo wrote:
> On Thu, Jul 05, 2012 at 09:13:18AM +0200, Sascha Hauer wrote:
> > Shawn,
> > 
> > Is this ok with you?
> > 
> No.
> 
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/172558/focus=172703
> 
I change my mind.  Though it's really a pity to lose a concentrated
place maintaining a full list of compatible strings of all supported
board, I'm more concerned by the dt_board_compat matching efficiency
when the table gets longer.

So, Fabio, can you please resend the patches against v3.6-rc?
Marek Vasut - Aug. 13, 2012, 3:18 p.m.
Dear Shawn Guo,

> On Thu, Jul 05, 2012 at 03:46:23PM +0800, Shawn Guo wrote:
> > On Thu, Jul 05, 2012 at 09:13:18AM +0200, Sascha Hauer wrote:
> > > Shawn,
> > > 
> > > Is this ok with you?
> > 
> > No.
> > 
> > http://thread.gmane.org/gmane.linux.ports.arm.kernel/172558/focus=172703
> 
> I change my mind.  Though it's really a pity to lose a concentrated
> place maintaining a full list of compatible strings of all supported
> board, I'm more concerned by the dt_board_compat matching efficiency
> when the table gets longer.
> 
> So, Fabio, can you please resend the patches against v3.6-rc?

What about the board quirks that are present there?

Best regards,
Marek Vasut
Fabio Estevam - Aug. 13, 2012, 4:56 p.m.
On Mon, Aug 13, 2012 at 12:15 PM, Shawn Guo <shawn.guo@linaro.org> wrote:

> So, Fabio, can you please resend the patches against v3.6-rc?

Which branch at git.linaro.org should I use it to rebase?

Regards,

Fabio Estevam
Matt Sealey - Aug. 13, 2012, 9:37 p.m.
On Mon, Aug 13, 2012 at 10:18 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Shawn Guo,
>
>> On Thu, Jul 05, 2012 at 03:46:23PM +0800, Shawn Guo wrote:
>> > On Thu, Jul 05, 2012 at 09:13:18AM +0200, Sascha Hauer wrote:
>> > > Shawn,
>> > >
>> > > Is this ok with you?
>> >
>> > No.
>> >
>> > http://thread.gmane.org/gmane.linux.ports.arm.kernel/172558/focus=172703
>>
>> I change my mind.  Though it's really a pity to lose a concentrated
>> place maintaining a full list of compatible strings of all supported
>> board, I'm more concerned by the dt_board_compat matching efficiency
>> when the table gets longer.
>>
>> So, Fabio, can you please resend the patches against v3.6-rc?
>
> What about the board quirks that are present there?

They don't need to be done since they're usually driver-specific or
unit-specific; DTs should include these at the appropriate places. For
the MX51 Babbage entry all it did was set up all the iomux which is
now done via pinctrl and DT, so it can go. The rest, well, this is
more a quirk of the chip, and what should happen is, if it can be
moved to a bootloader, do it, and set up a quirk as a property in that
device like fsl,mc13xxx-uses-rtc - although in that particular example
I dare say that should be an rtc node under the pmic entry rather than
a property, at least for now it shows you can pick up a property that
changes the behavior of a device without it being in a mach-specific
file.

There may actually be some boards that need some specific, low-level
hacks to make work that will need entering but, for now, this doesn't
need to be. They can be removed and when those specific hacks appear,
their compatibles and new code to support the hacks can be added back
in.
Rob Herring - Aug. 13, 2012, 9:48 p.m.
On 08/13/2012 10:15 AM, Shawn Guo wrote:
> On Thu, Jul 05, 2012 at 03:46:23PM +0800, Shawn Guo wrote:
>> On Thu, Jul 05, 2012 at 09:13:18AM +0200, Sascha Hauer wrote:
>>> Shawn,
>>>
>>> Is this ok with you?
>>>
>> No.
>>
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/172558/focus=172703
>>
> I change my mind.  Though it's really a pity to lose a concentrated
> place maintaining a full list of compatible strings of all supported
> board, I'm more concerned by the dt_board_compat matching efficiency
> when the table gets longer.
> 

I think this is needless churn. You can't say board entries are never
needed. Perhaps we've been overly active in adding all compatible
strings before they are needed. When you add a new board, it is fine to
match against the soc string without changing the kernel, but the dtb
should still have a more specific string. Then if you need to add
something board specific later, you can add the match entry and
corresponding board specific code.

Rob
Shawn Guo - Aug. 14, 2012, 1:26 a.m.
On Mon, Aug 13, 2012 at 04:48:11PM -0500, Rob Herring wrote:
> I think this is needless churn. You can't say board entries are never
> needed. Perhaps we've been overly active in adding all compatible
> strings before they are needed. When you add a new board, it is fine to
> match against the soc string without changing the kernel, but the dtb
> should still have a more specific string. Then if you need to add
> something board specific later, you can add the match entry and
> corresponding board specific code.
> 
I probably haven't made myself clear.  What I'm asking for is merely
removing the board compatible string from dt_board_compat, not from
dtb.  Every single board will still have its specific compatible string
defined in its dts.
Shawn Guo - Aug. 14, 2012, 1:28 a.m.
On Mon, Aug 13, 2012 at 01:56:40PM -0300, Fabio Estevam wrote:
> On Mon, Aug 13, 2012 at 12:15 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 
> > So, Fabio, can you please resend the patches against v3.6-rc?
> 
> Which branch at git.linaro.org should I use it to rebase?
> 
Just against mainline v3.6-rc.

Patch

diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
index 18e78db..5f10812 100644
--- a/arch/arm/mach-imx/imx51-dt.c
+++ b/arch/arm/mach-imx/imx51-dt.c
@@ -106,7 +106,6 @@  static struct sys_timer imx51_timer = {
 };
 
 static const char *imx51_dt_board_compat[] __initdata = {
-	"fsl,imx51-babbage",
 	"fsl,imx51",
 	NULL
 };
diff --git a/arch/arm/mach-imx/imx53-dt.c b/arch/arm/mach-imx/imx53-dt.c
index eb04b62..75535f9 100644
--- a/arch/arm/mach-imx/imx53-dt.c
+++ b/arch/arm/mach-imx/imx53-dt.c
@@ -132,10 +132,6 @@  static struct sys_timer imx53_timer = {
 };
 
 static const char *imx53_dt_board_compat[] __initdata = {
-	"fsl,imx53-ard",
-	"fsl,imx53-evk",
-	"fsl,imx53-qsb",
-	"fsl,imx53-smd",
 	"fsl,imx53",
 	NULL
 };
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index b47e98b..1796be4 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -173,9 +173,6 @@  static struct sys_timer imx6q_timer = {
 };
 
 static const char *imx6q_dt_compat[] __initdata = {
-	"fsl,imx6q-arm2",
-	"fsl,imx6q-sabrelite",
-	"fsl,imx6q-sabresd",
 	"fsl,imx6q",
 	NULL,
 };