[PATCH] 86xx: mark functions static, other minor cleanups

[PATCH] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
incudes: mark a bunch of functions static; add vendor prefix to the
compat node check for uniqueness, add in missing of_node_put(), delete
unused DBG macros.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/boot/dts/mpc8641_hpcn.dts     |    2 +-
 arch/powerpc/boot/dts/sbc8641d.dts         |    2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    8 ++++----
 arch/powerpc/platforms/86xx/sbc8641d.c     |   17 +++++------------
 5 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 79385bc..d5c2da4 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "MPC8641HPCN";
-	compatible = "mpc86xx";
+	compatible = "fsl,mpc86xx";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/boot/dts/sbc8641d.dts b/arch/powerpc/boot/dts/sbc8641d.dts
index 27b5a3d..33dcf6e 100644
--- a/arch/powerpc/boot/dts/sbc8641d.dts
+++ b/arch/powerpc/boot/dts/sbc8641d.dts
@@ -17,7 +17,7 @@
 
 / {
 	model = "SBC8641D";
-	compatible = "mpc86xx";
+	compatible = "wrs,sbc8641";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 0b07485..18b8ebe 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -52,7 +52,7 @@ static int __init mpc8610_declare_of_platform_devices(void)
 }
 machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);
 
-void __init
+static void __init
 mpc86xx_hpcd_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -200,7 +200,7 @@ static int __init mpc86xx_hpcd_probe(void)
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index cfbe8c5..600bbf3 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -55,7 +55,7 @@ static void mpc86xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
 }
 #endif	/* CONFIG_PCI */
 
-void __init
+static void __init
 mpc86xx_hpcn_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -162,7 +162,7 @@ mpc86xx_hpcn_setup_arch(void)
 }
 
 
-void
+static void
 mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -190,13 +190,13 @@ static int __init mpc86xx_hpcn_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	if (of_flat_dt_is_compatible(root, "fsl,mpc86xx"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/sbc8641d.c b/arch/powerpc/platforms/86xx/sbc8641d.c
index c7516be..0863cca 100644
--- a/arch/powerpc/platforms/86xx/sbc8641d.c
+++ b/arch/powerpc/platforms/86xx/sbc8641d.c
@@ -37,15 +37,7 @@
 
 #include "mpc86xx.h"
 
-#undef DEBUG
-
-#ifdef DEBUG
-#define DBG(fmt...) do { printk(KERN_ERR fmt); } while(0)
-#else
-#define DBG(fmt...) do { } while(0)
-#endif
-
-void __init
+static void __init
 sbc8641_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -62,6 +54,7 @@ sbc8641_init_irq(void)
 	mpic1 = mpic_alloc(np, res.start,
 			MPIC_PRIMARY | MPIC_WANTS_RESET | MPIC_BIG_ENDIAN,
 			0, 256, " MPIC     ");
+	of_node_put(np);
 	BUG_ON(mpic1 == NULL);
 
 	mpic_init(mpic1);
@@ -90,7 +83,7 @@ sbc8641_setup_arch(void)
 }
 
 
-void
+static void
 sbc8641_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -118,13 +111,13 @@ static int __init sbc8641_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	if (of_flat_dt_is_compatible(root, "wrs,sbc8641"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
-- 
1.5.4.3

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Scott Wood]

On Fri, Apr 11, 2008 at 12:59:46PM -0400, Paul Gortmaker wrote:
> diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> index 79385bc..d5c2da4 100644
> --- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> +++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> @@ -13,7 +13,7 @@
>  
>  / {
>  	model = "MPC8641HPCN";
> -	compatible = "mpc86xx";
> +	compatible = "fsl,mpc86xx";
>  	#address-cells = <1>;
>  	#size-cells = <1>;

If we're changing the compatible, change it to something proper (e.g.
fsl,mpc8641hpcn).

-Scott

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

In message: Re: [PATCH] 86xx: mark functions static, other minor cleanups
on 11/04/2008 Scott Wood wrote:

> >  	model = "MPC8641HPCN";
> > -	compatible = "mpc86xx";
> > +	compatible = "fsl,mpc86xx";
> >  	#address-cells = <1>;
> >  	#size-cells = <1>;
> 
> If we're changing the compatible, change it to something proper (e.g.
> fsl,mpc8641hpcn).
> 

Updated as per above, and with tickerized prefixes for sbc8641.

Paul.
---

>From 8278c7ec25c1aadc92c71ca4e4b8e51b2c02a49a Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 11 Apr 2008 12:51:04 -0400
Subject: [PATCH] 86xx: mark functions static, other minor cleanups

Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
incudes: mark a bunch of functions static; add vendor prefix to the
compat node check for uniqueness, add in missing of_node_put(), delete
unused DBG macros.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/boot/dts/mpc8641_hpcn.dts     |    2 +-
 arch/powerpc/boot/dts/sbc8641d.dts         |    2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    8 ++++----
 arch/powerpc/platforms/86xx/sbc8641d.c     |   17 +++++------------
 5 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 79385bc..7f9b999 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "MPC8641HPCN";
-	compatible = "mpc86xx";
+	compatible = "fsl,mpc8641hpcn";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/boot/dts/sbc8641d.dts b/arch/powerpc/boot/dts/sbc8641d.dts
index 27b5a3d..f2d85b3 100644
--- a/arch/powerpc/boot/dts/sbc8641d.dts
+++ b/arch/powerpc/boot/dts/sbc8641d.dts
@@ -17,7 +17,7 @@
 
 / {
 	model = "SBC8641D";
-	compatible = "mpc86xx";
+	compatible = "wind,sbc8641";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 0b07485..18b8ebe 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -52,7 +52,7 @@ static int __init mpc8610_declare_of_platform_devices(void)
 }
 machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);
 
-void __init
+static void __init
 mpc86xx_hpcd_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -200,7 +200,7 @@ static int __init mpc86xx_hpcd_probe(void)
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index cfbe8c5..0764032 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -55,7 +55,7 @@ static void mpc86xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
 }
 #endif	/* CONFIG_PCI */
 
-void __init
+static void __init
 mpc86xx_hpcn_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -162,7 +162,7 @@ mpc86xx_hpcn_setup_arch(void)
 }
 
 
-void
+static void
 mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -190,13 +190,13 @@ static int __init mpc86xx_hpcn_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	if (of_flat_dt_is_compatible(root, "fsl,mpc8641hpcn"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/sbc8641d.c b/arch/powerpc/platforms/86xx/sbc8641d.c
index c7516be..510a06e 100644
--- a/arch/powerpc/platforms/86xx/sbc8641d.c
+++ b/arch/powerpc/platforms/86xx/sbc8641d.c
@@ -37,15 +37,7 @@
 
 #include "mpc86xx.h"
 
-#undef DEBUG
-
-#ifdef DEBUG
-#define DBG(fmt...) do { printk(KERN_ERR fmt); } while(0)
-#else
-#define DBG(fmt...) do { } while(0)
-#endif
-
-void __init
+static void __init
 sbc8641_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -62,6 +54,7 @@ sbc8641_init_irq(void)
 	mpic1 = mpic_alloc(np, res.start,
 			MPIC_PRIMARY | MPIC_WANTS_RESET | MPIC_BIG_ENDIAN,
 			0, 256, " MPIC     ");
+	of_node_put(np);
 	BUG_ON(mpic1 == NULL);
 
 	mpic_init(mpic1);
@@ -90,7 +83,7 @@ sbc8641_setup_arch(void)
 }
 
 
-void
+static void
 sbc8641_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -118,13 +111,13 @@ static int __init sbc8641_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	if (of_flat_dt_is_compatible(root, "wind,sbc8641"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
-- 
1.5.4.3

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Segher Boessenkool]

> Updated as per above, and with tickerized prefixes for sbc8641.

Care to try once more?  It's only "tickerized" if it's in all
uppercase.

> +	compatible = "wind,sbc8641";


Segher

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

On Fri, Apr 11, 2008 at 3:11 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> > Updated as per above, and with tickerized prefixes for sbc8641.
> >
>
>  Care to try once more?  It's only "tickerized" if it's in all
>  uppercase.

I'm looking at what exists in arch/powerpc/boot/dts/* and I'm
not seeing too much uppercase - here is a sample:

ebony.dts:        compatible = "ibm,ebony";
ep405.dts:        compatible = "ibm,uic";
ep8248e.dts:    compatible = "fsl,ep8248e";
bamboo.dts:     compatible = "amcc,bamboo";
cm5200.dts:     compatible = "schindler,cm5200";
ep88xc.dts:     compatible = "fsl,ep88xc";
haleakala.dts:  compatible = "amcc,kilauea";
holly.dts:         compatible = "ibm,holly";
katmai.dts:     compatible = "amcc,katmai";
kilauea.dts:    compatible = "amcc,kilauea";
lite5200b.dts:  compatible = "fsl,lite5200b";
motionpro.dts:  compatible = "promess,motionpro";
mpc8272ads.dts: compatible = "fsl,mpc8272ads";
mpc866ads.dts:  compatible = "fsl,mpc866ads";

> > +       compatible = "wind,sbc8641";

To me this looks in keeping with the rest.  And I prefer
with the lower case, actually.  (Apparently so do a lot of
other people...)

Paul.

> >
>
>
>  Segher
>
>
>
>  _______________________________________________
>  Linuxppc-dev mailing list
>  Linuxppc-dev@ozlabs.org
>  https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[David Gibson]

On Mon, Apr 14, 2008 at 09:32:44PM -0400, Paul Gortmaker wrote:
> On Fri, Apr 11, 2008 at 3:11 PM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > > Updated as per above, and with tickerized prefixes for sbc8641.
> > >
> >
> >  Care to try once more?  It's only "tickerized" if it's in all
> >  uppercase.
> 
> I'm looking at what exists in arch/powerpc/boot/dts/* and I'm
> not seeing too much uppercase - here is a sample:
> 
> ebony.dts:        compatible = "ibm,ebony";
> ep405.dts:        compatible = "ibm,uic";
> ep8248e.dts:    compatible = "fsl,ep8248e";
> bamboo.dts:     compatible = "amcc,bamboo";
> cm5200.dts:     compatible = "schindler,cm5200";
> ep88xc.dts:     compatible = "fsl,ep88xc";
> haleakala.dts:  compatible = "amcc,kilauea";
> holly.dts:         compatible = "ibm,holly";
> katmai.dts:     compatible = "amcc,katmai";
> kilauea.dts:    compatible = "amcc,kilauea";
> lite5200b.dts:  compatible = "fsl,lite5200b";
> motionpro.dts:  compatible = "promess,motionpro";
> mpc8272ads.dts: compatible = "fsl,mpc8272ads";
> mpc866ads.dts:  compatible = "fsl,mpc866ads";
> 
> > > +       compatible = "wind,sbc8641";
> 
> To me this looks in keeping with the rest.  And I prefer
> with the lower case, actually.  (Apparently so do a lot of
> other people...)

The confusion arises due to a difference between historical OF
practice, and current flattened DT practice.

Historically, as Segher says, uppercase names are stock tickers (and
thereby a centrally registered, guaranteed-unique namespace).
Lowercase names are a free-for-all, no central management, put pick a
reasonable name and it will probably be ok.

Use of the formal uppercase names in OF practice isn't particularly
common - there certainly are AAPL, and IBM, names out there but
they're rather outnumbered by the informal lowercase prefixes.  Which
is why most of us doing flattened tree work didn't realise the
distinction.  So, the flattened tree Linux community independently
came up with the convention of using stock tickers as a way of
uniqueifying the names - but used lowercase names.

So, it's a bit of a mess.  Here's my recommended procedure:

	* If you can find a single dominant existing practice for the
vendor in question, use that.  (Consider both OF and flattened tree
practice). i.e. existing practice trumps all.

	* If you can't find any existing practice and need to make a
new prefix, use the stock ticker.

	* If you find more than one existing practice (and none is
clearly dominant), take it to the list and we can argue about it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Segher Boessenkool]

> add vendor prefix to the compat node check for uniqueness,

> -	compatible = "mpc86xx";
> +	compatible = "wrs,sbc8641";

Either use the ticker thing (uppercase, "WIND,xxxx"), or, if you
prefer a lowercase name without that little extra namespace protection,
please use something more descriptive -- there are only so many
three-letter acronyms: "windriver,xxxx"?


Segher

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Timur Tabi]

Paul Gortmaker wrote:

> -void
> +static void
>  mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
>  {
>  	struct device_node *root;
> @@ -190,13 +190,13 @@ static int __init mpc86xx_hpcn_probe(void)
>  {
>  	unsigned long root = of_get_flat_dt_root();
>  
> -	if (of_flat_dt_is_compatible(root, "mpc86xx"))
> +	if (of_flat_dt_is_compatible(root, "fsl,mpc86xx"))
>  		return 1;	/* Looks good */

This breaks compatibility with older device trees.  You still need to look for
"mpc86xx".

A lot of people have been doing this recently, and it needs to stop.  You need
to wait at least one whole kernel version before you can remove support for an
older device tree.

> -void
> +static void
>  sbc8641_show_cpuinfo(struct seq_file *m)
>  {
>  	struct device_node *root;
> @@ -118,13 +111,13 @@ static int __init sbc8641_probe(void)
>  {
>  	unsigned long root = of_get_flat_dt_root();
>  
> -	if (of_flat_dt_is_compatible(root, "mpc86xx"))
> +	if (of_flat_dt_is_compatible(root, "wrs,sbc8641"))
>  		return 1;	/* Looks good */

Same here.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

Timur Tabi wrote:
> Paul Gortmaker wrote:
>
>   
>> -void
>> +static void
>>  mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
>>  {
>>  	struct device_node *root;
>> @@ -190,13 +190,13 @@ static int __init mpc86xx_hpcn_probe(void)
>>  {
>>  	unsigned long root = of_get_flat_dt_root();
>>  
>> -	if (of_flat_dt_is_compatible(root, "mpc86xx"))
>> +	if (of_flat_dt_is_compatible(root, "fsl,mpc86xx"))
>>  		return 1;	/* Looks good */
>>     
>
> This breaks compatibility with older device trees.  You still need to look for
> "mpc86xx".
>
> A lot of people have been doing this recently, and it needs to stop.  You need
> to wait at least one whole kernel version before you can remove support for an
> older device tree.
>   

Valid point.  Is there a precedent here -- like a printk indicating
that the old ID matched, to let the user know?

>   
>> -void
>> +static void
>>  sbc8641_show_cpuinfo(struct seq_file *m)
>>  {
>>  	struct device_node *root;
>> @@ -118,13 +111,13 @@ static int __init sbc8641_probe(void)
>>  {
>>  	unsigned long root = of_get_flat_dt_root();
>>  
>> -	if (of_flat_dt_is_compatible(root, "mpc86xx"))
>> +	if (of_flat_dt_is_compatible(root, "wrs,sbc8641"))
>>  		return 1;	/* Looks good */
>>     
>
> Same here.
>   

Actually on this one, we are OK, since the board support didn't exist
in the default kernel until I'd just sent it last week.

Thanks,
Paul.

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Timur Tabi]

Paul Gortmaker wrote:

> Valid point.  Is there a precedent here -- like a printk indicating
> that the old ID matched, to let the user know?

Not really, but a pr_warning() would be nice.

> Actually on this one, we are OK, since the board support didn't exist
> in the default kernel until I'd just sent it last week.

No problem, then.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

In message: Re: [PATCH] 86xx: mark functions static, other minor cleanups
on 15/04/2008 Timur Tabi wrote:

> Paul Gortmaker wrote:
> 
> > Valid point.  Is there a precedent here -- like a printk indicating
> > that the old ID matched, to let the user know?
> 
> Not really, but a pr_warning() would be nice.

Done.

I've also removed the sbc8641 content from this patch, and rolled it
into the resend of those board specific patches.  This is now just for
incorporating that same feedback into existing boards.

Thanks,
Paul.
---

 From aa2d1dd871c7eb440b3947cf8952d28249acf218 Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 11 Apr 2008 12:51:04 -0400
Subject: [PATCH] 86xx: mark functions static, other minor cleanups

Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
incudes marking a bunch of functions static and add a vendor prefix to
the compat node check for uniqueness.  We match on the old compat node
ID for one version and warn accordingly, so as to not plunge people
into silent boot death, as suggested by Timur Tabi.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/boot/dts/mpc8641_hpcn.dts     |    2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |   14 ++++++++++----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 79385bc..7f9b999 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "MPC8641HPCN";
-	compatible = "mpc86xx";
+	compatible = "fsl,mpc8641hpcn";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 0b07485..18b8ebe 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -52,7 +52,7 @@ static int __init mpc8610_declare_of_platform_devices(void)
 }
 machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);
 
-void __init
+static void __init
 mpc86xx_hpcd_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -200,7 +200,7 @@ static int __init mpc86xx_hpcd_probe(void)
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index cfbe8c5..9d32223 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -55,7 +55,7 @@ static void mpc86xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
 }
 #endif	/* CONFIG_PCI */
 
-void __init
+static void __init
 mpc86xx_hpcn_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -162,7 +162,7 @@ mpc86xx_hpcn_setup_arch(void)
 }
 
 
-void
+static void
 mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -190,13 +190,19 @@ static int __init mpc86xx_hpcn_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	/* Delete this in 2.6.27 */
+	if (of_flat_dt_is_compatible(root, "mpc86xx")) {
+		pr_warning("WARNING: your dts/dtb is old. You must update before the next kernel release\n");
+		return 1;
+	}
+
+	if (of_flat_dt_is_compatible(root, "fsl,mpc8641hpcn"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
-- 
1.5.4.3

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Kumar Gala]

>
> -void
> +static void
> mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
> {
> 	struct device_node *root;
> @@ -190,13 +190,19 @@ static int __init mpc86xx_hpcn_probe(void)
> {
> 	unsigned long root = of_get_flat_dt_root();
>
> -	if (of_flat_dt_is_compatible(root, "mpc86xx"))
> +	/* Delete this in 2.6.27 */
> +	if (of_flat_dt_is_compatible(root, "mpc86xx")) {
> +		pr_warning("WARNING: your dts/dtb is old. You must update before  
> the next kernel release\n");
> +		return 1;
> +	}
> +
> +	if (of_flat_dt_is_compatible(root, "fsl,mpc8641hpcn"))
> 		return 1;	/* Looks good */
>

how about reversing the order of the checks.

- k


> 	return 0;
> }

Re: [PATCH] 86xx: mark functions static, other minor cleanups

[Kumar Gala]

On Apr 15, 2008, at 5:46 PM, Paul Gortmaker wrote:
> In message: Re: [PATCH] 86xx: mark functions static, other minor  
> cleanups
> on 15/04/2008 Timur Tabi wrote:
>
>> Paul Gortmaker wrote:
>>
>>> Valid point.  Is there a precedent here -- like a printk indicating
>>> that the old ID matched, to let the user know?
>>
>> Not really, but a pr_warning() would be nice.
>
> Done.
>
> I've also removed the sbc8641 content from this patch, and rolled it
> into the resend of those board specific patches.  This is now just for
> incorporating that same feedback into existing boards.
>
> Thanks,
> Paul.
> ---
>
> From aa2d1dd871c7eb440b3947cf8952d28249acf218 Mon Sep 17 00:00:00 2001
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Fri, 11 Apr 2008 12:51:04 -0400
> Subject: [PATCH] 86xx: mark functions static, other minor cleanups
>
> Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
> incudes marking a bunch of functions static and add a vendor prefix to
> the compat node check for uniqueness.  We match on the old compat node
> ID for one version and warn accordingly, so as to not plunge people
> into silent boot death, as suggested by Timur Tabi.

can you add a commit about the change to the root compatible and its  
eventual removal for backward compatibility.

- k

[PATCH 1/2] 86xx: mark functions static, other minor cleanups

[Paul Gortmaker]

Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
incudes marking a bunch of functions static and add a vendor prefix to
the compat node check for uniqueness.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/boot/dts/mpc8641_hpcn.dts     |    2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 79385bc..7f9b999 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "MPC8641HPCN";
-	compatible = "mpc86xx";
+	compatible = "fsl,mpc8641hpcn";
 	#address-cells = <1>;
 	#size-cells = <1>;
 
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 0b07485..18b8ebe 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -52,7 +52,7 @@ static int __init mpc8610_declare_of_platform_devices(void)
 }
 machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);
 
-void __init
+static void __init
 mpc86xx_hpcd_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -200,7 +200,7 @@ static int __init mpc86xx_hpcd_probe(void)
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index cfbe8c5..0764032 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -55,7 +55,7 @@ static void mpc86xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
 }
 #endif	/* CONFIG_PCI */
 
-void __init
+static void __init
 mpc86xx_hpcn_init_irq(void)
 {
 	struct mpic *mpic1;
@@ -162,7 +162,7 @@ mpc86xx_hpcn_setup_arch(void)
 }
 
 
-void
+static void
 mpc86xx_hpcn_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -190,13 +190,13 @@ static int __init mpc86xx_hpcn_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (of_flat_dt_is_compatible(root, "mpc86xx"))
+	if (of_flat_dt_is_compatible(root, "fsl,mpc8641hpcn"))
 		return 1;	/* Looks good */
 
 	return 0;
 }
 
-long __init
+static long __init
 mpc86xx_time_init(void)
 {
 	unsigned int temp;
-- 
1.5.4.3

[PATCH 2/2] mpc86xx_hpcn: Temporarily accept old dts node identifier.

[Paul Gortmaker]

As suggested by Timur Tabi, we match on the old compat node ID for one
version and warn accordingly.  If we don't do this, we plunge people who
try to use an old DTB into silent boot death with no clear indication of
what the problem is.

This patch should be removed at the beginning of the 2.6.27 dev cycle.
It is only meant to ease the transition in the short term.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index 0764032..f947f55 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -193,6 +193,12 @@ static int __init mpc86xx_hpcn_probe(void)
 	if (of_flat_dt_is_compatible(root, "fsl,mpc8641hpcn"))
 		return 1;	/* Looks good */
 
+	/* Be nice and don't give silent boot death.  Delete this in 2.6.27 */
+	if (of_flat_dt_is_compatible(root, "mpc86xx")) {
+		pr_warning("WARNING: your dts/dtb is old. You must update before the next kernel release\n");
+		return 1;
+	}
+
 	return 0;
 }
 
-- 
1.5.4.3

Re: [PATCH 2/2] mpc86xx_hpcn: Temporarily accept old dts node identifier.

[Kumar Gala]

On Apr 16, 2008, at 12:53 PM, Paul Gortmaker wrote:
> As suggested by Timur Tabi, we match on the old compat node ID for one
> version and warn accordingly.  If we don't do this, we plunge people  
> who
> try to use an old DTB into silent boot death with no clear  
> indication of
> what the problem is.
>
> This patch should be removed at the beginning of the 2.6.27 dev cycle.
> It is only meant to ease the transition in the short term.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)

applied.

- k

Re: [PATCH 1/2] 86xx: mark functions static, other minor cleanups

[Kumar Gala]

On Apr 16, 2008, at 12:53 PM, Paul Gortmaker wrote:
> Cleanups as suggested by Stephen Rothwell and Dale Farnsworth, which
> incudes marking a bunch of functions static and add a vendor prefix to
> the compat node check for uniqueness.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> arch/powerpc/boot/dts/mpc8641_hpcn.dts     |    2 +-
> arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 ++--
> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    8 ++++----
> 3 files changed, 7 insertions(+), 7 deletions(-)

applied.

- k