Re: [GIT PULL] Disintegrate and kill asm/system.h

Hi all,

On Thu, 29 Mar 2012 13:55:55 +1100 Stephen Rothwell [off-list ref] wrote:

On Thu, 29 Mar 2012 13:50:44 +1100 Michael Neuling [off-list ref] wrote:

quoted

quoted

Here are a bunch of patches to disintegrate asm/system.h into a set of separate
bits to relieve the problem of circular inclusion dependencies.

The following changes since commit 437538267b672f9320833907f1b5acbb2605f4be:

  Merge tag 'fbdev-updates-for-3.4' of git://github.com/schandinat/linux-2.6 (2012-03-22 20:43:40 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-asm_system.git split-asm-system_h

It would have been nice to have this in linux-next for a day or two to
sort out the merge issues,  hence....

Um, what can I say but "yeah, it would have been nice" :-(

The following patch fixes (as much as possible) a powerpc allyesconfig build.

From: Stephen Rothwell <redacted>
Date: Thu, 29 Mar 2012 14:58:10 +1100
Subject: [PATCH] powerpc: fix fallout from system.h split up

Signed-off-by: Stephen Rothwell <redacted>
---
 arch/powerpc/kernel/fadump.c |    2 ++
 arch/powerpc/kernel/kgdb.c   |    1 +
 kernel/irq/irqdomain.c       |    3 +++
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index cfe7a38..18bdf74 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c

@@ -40,6 +40,8 @@
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/fadump.h>
+#include <asm/debug.h>
+#include <asm/setup.h>
 
 static struct fw_dump fw_dump;
 static struct fadump_mem_struct fdm;

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 76a6e40..782bd0a 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c

@@ -24,6 +24,7 @@
 #include <asm/current.h>
 #include <asm/processor.h>
 #include <asm/machdep.h>
+#include <asm/debug.h>
 
 /*
  * This table contains the mapping between PowerPC hardware trap types, and

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index af48e59..1f94ddf 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c

@@ -12,6 +12,9 @@
 #include <linux/slab.h>
 #include <linux/smp.h>
 #include <linux/fs.h>
+#ifdef CONFIG_VIRQ_DEBUG
+#include <asm/debug.h>
+#endif
 
 #define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
 				 * ie. legacy 8259, gets irqs 1..15 */

-- 
1.7.9.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

In message [off-list ref] you wrote
:

--Signature=_Thu__29_Mar_2012_15_15_44_+1100_WbAuFOESkuBxVlHq
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi all,

On Thu, 29 Mar 2012 13:55:55 +1100 Stephen Rothwell [off-list ref] =
wrote:

quoted

On Thu, 29 Mar 2012 13:50:44 +1100 Michael Neuling [off-list ref] wr=

ote:

quoted

quoted

quoted

Here are a bunch of patches to disintegrate asm/system.h into a set o=

f separate

quoted

quoted

quoted

bits to relieve the problem of circular inclusion dependencies.
=20
The following changes since commit 437538267b672f9320833907f1b5acbb26=

05f4be:

quoted

quoted

quoted

=20
  Merge tag 'fbdev-updates-for-3.4' of git://github.com/schandinat/li=

nux-2.6 (2012-03-22 20:43:40 -0700)

quoted

quoted

quoted

=20
are available in the git repository at:
=20
  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-asm_sy=

stem.git split-asm-system_h

quoted

quoted

=20
It would have been nice to have this in linux-next for a day or two to
sort out the merge issues,  hence....

=20
Um, what can I say but "yeah, it would have been nice" :-(

The following patch fixes (as much as possible) a powerpc allyesconfig build.

From: Stephen Rothwell <redacted>
Date: Thu, 29 Mar 2012 14:58:10 +1100
Subject: [PATCH] powerpc: fix fallout from system.h split up

Signed-off-by: Stephen Rothwell <redacted>

Acked-by: Michael Neuling <redacted>

Thanks, this is much better than my crappy fix.

Mikey

On Wed, Mar 28, 2012 at 9:15 PM, Stephen Rothwell [off-list ref] wr=
ote:

The following patch fixes (as much as possible) a powerpc allyesconfig bu=

ild.

Ugh:

Can we please move that abortion into arch/powerpc/? Instead of making
generic code even uglier..

                       Linus

Hi Linus,

On Wed, 28 Mar 2012 21:24:41 -0700 Linus Torvalds [off-list ref] wrote:

On Wed, Mar 28, 2012 at 9:15 PM, Stephen Rothwell [off-list ref] wrote:

quoted

The following patch fixes (as much as possible) a powerpc allyesconfig build.

Ugh:

quoted

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index af48e59..1f94ddf 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c

@@ -12,6 +12,9 @@

 #include <linux/slab.h>
 #include <linux/smp.h>
 #include <linux/fs.h>
+#ifdef CONFIG_VIRQ_DEBUG
+#include <asm/debug.h>
+#endif

Can we please move that abortion into arch/powerpc/? Instead of making
generic code even uglier..

How about this (build tested on powerpc allyesconfig):

From: Stephen Rothwell <redacted>
Date: Thu, 29 Mar 2012 14:58:10 +1100
Subject: [PATCH] powerpc: fix fallout from system.h split up

Signed-off-by: Stephen Rothwell <redacted>
---
 arch/powerpc/kernel/fadump.c |    2 +
 arch/powerpc/kernel/irq.c    |   72 ++++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/kgdb.c   |    1 +
 kernel/irq/irqdomain.c       |   72 ------------------------------------------
 4 files changed, 75 insertions(+), 72 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index cfe7a38..18bdf74 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c

@@ -40,6 +40,8 @@
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/fadump.h>
+#include <asm/debug.h>
+#include <asm/setup.h>
 
 static struct fw_dump fw_dump;
 static struct fadump_mem_struct fdm;

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 243dbab..ff165f5 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c

@@ -623,3 +623,75 @@ static int __init setup_noirqdistrib(char *str)
 
 __setup("noirqdistrib", setup_noirqdistrib);
 #endif /* CONFIG_PPC64 */
+
+#ifdef CONFIG_VIRQ_DEBUG
+static int virq_debug_show(struct seq_file *m, void *private)
+{
+	unsigned long flags;
+	struct irq_desc *desc;
+	const char *p;
+	static const char none[] = "none";
+	void *data;
+	int i;
+
+	seq_printf(m, "%-5s  %-7s  %-15s  %-18s  %s\n", "virq", "hwirq",
+		      "chip name", "chip data", "domain name");
+
+	for (i = 1; i < nr_irqs; i++) {
+		desc = irq_to_desc(i);
+		if (!desc)
+			continue;
+
+		raw_spin_lock_irqsave(&desc->lock, flags);
+
+		if (desc->action && desc->action->handler) {
+			struct irq_chip *chip;
+
+			seq_printf(m, "%5d  ", i);
+			seq_printf(m, "0x%05lx  ", desc->irq_data.hwirq);
+
+			chip = irq_desc_get_chip(desc);
+			if (chip && chip->name)
+				p = chip->name;
+			else
+				p = none;
+			seq_printf(m, "%-15s  ", p);
+
+			data = irq_desc_get_chip_data(desc);
+			seq_printf(m, "0x%16p  ", data);
+
+			if (desc->irq_data.domain->of_node)
+				p = desc->irq_data.domain->of_node->full_name;
+			else
+				p = none;
+			seq_printf(m, "%s\n", p);
+		}
+
+		raw_spin_unlock_irqrestore(&desc->lock, flags);
+	}
+
+	return 0;
+}
+
+static int virq_debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, virq_debug_show, inode->i_private);
+}
+
+static const struct file_operations virq_debug_fops = {
+	.open = virq_debug_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int __init irq_debugfs_init(void)
+{
+	if (debugfs_create_file("virq_mapping", S_IRUGO, powerpc_debugfs_root,
+				 NULL, &virq_debug_fops) == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+__initcall(irq_debugfs_init);
+#endif /* CONFIG_VIRQ_DEBUG */

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 76a6e40..782bd0a 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c

@@ -24,6 +24,7 @@
 #include <asm/current.h>
 #include <asm/processor.h>
 #include <asm/machdep.h>
+#include <asm/debug.h>
 
 /*
  * This table contains the mapping between PowerPC hardware trap types, and

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index af48e59..ae645ab 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c

@@ -632,78 +632,6 @@ unsigned int irq_linear_revmap(struct irq_domain *domain,
 	return revmap[hwirq];
 }
 
-#ifdef CONFIG_VIRQ_DEBUG
-static int virq_debug_show(struct seq_file *m, void *private)
-{
-	unsigned long flags;
-	struct irq_desc *desc;
-	const char *p;
-	static const char none[] = "none";
-	void *data;
-	int i;
-
-	seq_printf(m, "%-5s  %-7s  %-15s  %-18s  %s\n", "virq", "hwirq",
-		      "chip name", "chip data", "domain name");
-
-	for (i = 1; i < nr_irqs; i++) {
-		desc = irq_to_desc(i);
-		if (!desc)
-			continue;
-
-		raw_spin_lock_irqsave(&desc->lock, flags);
-
-		if (desc->action && desc->action->handler) {
-			struct irq_chip *chip;
-
-			seq_printf(m, "%5d  ", i);
-			seq_printf(m, "0x%05lx  ", desc->irq_data.hwirq);
-
-			chip = irq_desc_get_chip(desc);
-			if (chip && chip->name)
-				p = chip->name;
-			else
-				p = none;
-			seq_printf(m, "%-15s  ", p);
-
-			data = irq_desc_get_chip_data(desc);
-			seq_printf(m, "0x%16p  ", data);
-
-			if (desc->irq_data.domain->of_node)
-				p = desc->irq_data.domain->of_node->full_name;
-			else
-				p = none;
-			seq_printf(m, "%s\n", p);
-		}
-
-		raw_spin_unlock_irqrestore(&desc->lock, flags);
-	}
-
-	return 0;
-}
-
-static int virq_debug_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, virq_debug_show, inode->i_private);
-}
-
-static const struct file_operations virq_debug_fops = {
-	.open = virq_debug_open,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
-};
-
-static int __init irq_debugfs_init(void)
-{
-	if (debugfs_create_file("virq_mapping", S_IRUGO, powerpc_debugfs_root,
-				 NULL, &virq_debug_fops) == NULL)
-		return -ENOMEM;
-
-	return 0;
-}
-__initcall(irq_debugfs_init);
-#endif /* CONFIG_VIRQ_DEBUG */
-
 int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
 			  irq_hw_number_t hwirq)
 {

-- 
1.7.9.1


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

quoted

Can we please move that abortion into arch/powerpc/? Instead of making
generic code even uglier..

How about this (build tested on powerpc allyesconfig):

From: Stephen Rothwell <redacted>
Date: Thu, 29 Mar 2012 14:58:10 +1100
Subject: [PATCH] powerpc: fix fallout from system.h split up

Signed-off-by: Stephen Rothwell <redacted>

FWIW.. this works for me too..

Acked-by: Michael Neuling <redacted>

Mikey

;

;

-			else
-				p =3D none;
-			seq_printf(m, "%s\n", p);
-		}
-
-		raw_spin_unlock_irqrestore(&desc->lock, flags);
-	}
-
-	return 0;
-}
-
-static int virq_debug_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, virq_debug_show, inode->i_private);
-}
-
-static const struct file_operations virq_debug_fops =3D {
-	.open =3D virq_debug_open,
-	.read =3D seq_read,
-	.llseek =3D seq_lseek,
-	.release =3D single_release,
-};
-
-static int __init irq_debugfs_init(void)
-{
-	if (debugfs_create_file("virq_mapping", S_IRUGO, powerpc_debugfs_root,
-				 NULL, &virq_debug_fops) =3D=3D NULL)
-		return -ENOMEM;
-
-	return 0;
-}
-__initcall(irq_debugfs_init);
-#endif /* CONFIG_VIRQ_DEBUG */
-
 int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
 			  irq_hw_number_t hwirq)
 {
--=20
1.7.9.1


--=20
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

On Wed, Mar 28, 2012 at 9:42 PM, Stephen Rothwell [off-list ref] wrote:

How about this (build tested on powerpc allyesconfig):

Looks better.

Grant - what's the plan about that CONFIG_VIRQ_DEBUG thing? In theory
something like it could be useful as a generic thing, but at least
right now it is clearly powerpc-specific (not just that the config
option only exists for powerpc: it has that whole
'powerpc_debugfs_root' thing in it)?

Stephen's patch very much looks like the right thing, but if there was
some plan to actually make this generic ...

                    Linus

On Wed, 2012-03-28 at 22:02 -0700, Linus Torvalds wrote:

On Wed, Mar 28, 2012 at 9:42 PM, Stephen Rothwell [off-list ref] wrote:

quoted

How about this (build tested on powerpc allyesconfig):

Looks better.

Grant - what's the plan about that CONFIG_VIRQ_DEBUG thing? In theory
something like it could be useful as a generic thing, but at least
right now it is clearly powerpc-specific (not just that the config
option only exists for powerpc: it has that whole
'powerpc_debugfs_root' thing in it)?

Stephen's patch very much looks like the right thing, but if there was
some plan to actually make this generic ...

Yes, as I said earlier I think Grant should make it generic, there's
no reason to keep that powerpc specific, it's fairly useful and nothing
in userspace relies on the existing location of the debugfs file that I
know about.

Another option is to put the mapping information in /proc/interrupts but
worry that changing anything in there (like adding a column) will break
countless userspace tools.

Cheers,
Ben.
 

On Thu, Mar 29, 2012 at 2:11 AM, Benjamin Herrenschmidt
[off-list ref] wrote:

On Wed, 2012-03-28 at 22:02 -0700, Linus Torvalds wrote:

quoted

On Wed, Mar 28, 2012 at 9:42 PM, Stephen Rothwell [off-list ref] wrote:

quoted

How about this (build tested on powerpc allyesconfig):

Looks better.

Grant - what's the plan about that CONFIG_VIRQ_DEBUG thing? In theory
something like it could be useful as a generic thing, but at least
right now it is clearly powerpc-specific (not just that the config
option only exists for powerpc: it has that whole
'powerpc_debugfs_root' thing in it)?

Stephen's patch very much looks like the right thing, but if there was
some plan to actually make this generic ...

Yes, as I said earlier I think Grant should make it generic, there's
no reason to keep that powerpc specific, it's fairly useful and nothing
in userspace relies on the existing location of the debugfs file that I
know about.

Another option is to put the mapping information in /proc/interrupts but
worry that changing anything in there (like adding a column) will break
countless userspace tools.

I posted an RFC patch that does exactly that, but I'm similarly
nervous for the same reason:

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg56368.html

The rightmost fields of /proc/interrupts are a weird set of
conditional outputs that don't really have any parsable formatting to
them.  It may be safe to apply my patch because it adds another field
in the middle of a section of conditional outputs* anyway so tools
already won't know what those fields mean.  But, regardless, I'm not
going to take responsibility for applying that patch unless encouraged
by several other maintainers to do so.

*It's added to the middle instead of the end purely for aesthetic
reasons.  The rightmost field is a freeform string without a field
width, so if put at the end they would never line up.  Suggestions for
a better layout are welcome.

g.

On Thu, 29 Mar 2012, Grant Likely wrote:

On Thu, Mar 29, 2012 at 2:11 AM, Benjamin Herrenschmidt
The rightmost fields of /proc/interrupts are a weird set of
conditional outputs that don't really have any parsable formatting to
them.  It may be safe to apply my patch because it adds another field
in the middle of a section of conditional outputs* anyway so tools
already won't know what those fields mean.  But, regardless, I'm not
going to take responsibility for applying that patch unless encouraged
by several other maintainers to do so.

Go ahead. We changed that output several times in the last years
without causing the end of the universe.

Thanks,

	tglx

On Thu, Mar 29, 2012 at 3:48 PM, Thomas Gleixner [off-list ref] wrote=
:

On Thu, 29 Mar 2012, Grant Likely wrote:

quoted

On Thu, Mar 29, 2012 at 2:11 AM, Benjamin Herrenschmidt
The rightmost fields of /proc/interrupts are a weird set of
conditional outputs that don't really have any parsable formatting to
them. =A0It may be safe to apply my patch because it adds another field
in the middle of a section of conditional outputs* anyway so tools
already won't know what those fields mean. =A0But, regardless, I'm not
going to take responsibility for applying that patch unless encouraged
by several other maintainers to do so.

Go ahead. We changed that output several times in the last years
without causing the end of the universe.

Okay, RFC patch sent.  I'm okay to wait for v3.5 for this, but it can
go into v3.4 if anyone really wants it faster.

g.