Resolve compile issues with msp71xx configuration

There have been a number of compile problems with the msp71xx configuration ever since it was included in the linux-mips.org repository. This patch resolves these problems:
- proper files are included when using a squashfs rootfs
- resetting the board no longer uses non-existent GPIO routines
- create the required plat_timer_setup function

This patch has been compile-tested against the current HEAD.

Signed-off-by Shane McDonald <[email protected]>

---
 arch/mips/pmc-sierra/msp71xx/msp_prom.c  |    3 ++-
 arch/mips/pmc-sierra/msp71xx/msp_setup.c |    8 ++------
 arch/mips/pmc-sierra/msp71xx/msp_time.c  |    7 ++-----
 3 files changed, 6 insertions(+), 12 deletions(-)

arch/mips/pmc-sierra/msp71xx/msp_prom.c +0 -0

index e5bd548..1e2d984 100644

arch/mips/pmc-sierra/msp71xx/msp_prom.c +0 -0

 #endif
 
 #include <asm/addrspace.h>

arch/mips/pmc-sierra/msp71xx/msp_setup.c +2 -6

index c936756..a54e85b 100644
@@ -21,7 +21,6 @@
 
 #if defined(CONFIG_PMC_MSP7120_GW)
 #include <msp_regops.h>
-#include <msp_gpio.h>
 #define MSP_BOARD_RESET_GPIO	9
 #endif
 

@@ -88,11 +87,8 @@ void msp7120_reset(void)
 	 * as GPIO char driver may not be enabled and it would look up
 	 * data inRAM!
 	 */
-	set_value_reg32(GPIO_CFG3_REG,
-			basic_mode_mask(MSP_BOARD_RESET_GPIO),
-			basic_mode(MSP_GPIO_OUTPUT, MSP_BOARD_RESET_GPIO));
-	set_reg32(GPIO_DATA3_REG,
-			basic_data_mask(MSP_BOARD_RESET_GPIO));
+	set_value_reg32(GPIO_CFG3_REG, 0xf000, 0x8000);
+	set_reg32(GPIO_DATA3_REG, 8);
 
 	/*
 	 * In case GPIO9 doesn't reset the board (jumper configurable!)

arch/mips/pmc-sierra/msp71xx/msp_time.c +0 -0

index 7cfeda5..cca64e1 100644
@@ -81,10 +81,7 @@ void __init plat_time_init(void)
 	mips_hpt_frequency = cpu_rate/2;
 }

Quoted 6 lines

There have been a number of compile problems with the msp71xx configuration ever since it was included in the linux-mips.org repository. This patch resolves these problems:
- proper files are included when using a squashfs rootfs
- resetting the board no longer uses non-existent GPIO routines
- create the required plat_timer_setup function

Quoted 38 lines

This patch has been compile-tested against the current HEAD.

Signed-off-by Shane McDonald <[email protected]>

arch/mips/pmc-sierra/msp71xx/msp_setup.c +2 -6

index c936756..a54e85b 100644
@@ -21,7 +21,6 @@
 
 #if defined(CONFIG_PMC_MSP7120_GW)
 #include <msp_regops.h>
-#include <msp_gpio.h>
 #define MSP_BOARD_RESET_GPIO	9
 #endif
 

@@ -88,11 +87,8 @@ void msp7120_reset(void)
 	 * as GPIO char driver may not be enabled and it would look up
 	 * data inRAM!
 	 */
-	set_value_reg32(GPIO_CFG3_REG,
-			basic_mode_mask(MSP_BOARD_RESET_GPIO),
-			basic_mode(MSP_GPIO_OUTPUT, MSP_BOARD_RESET_GPIO));
-	set_reg32(GPIO_DATA3_REG,
-			basic_data_mask(MSP_BOARD_RESET_GPIO));
+	set_value_reg32(GPIO_CFG3_REG, 0xf000, 0x8000);
+	set_reg32(GPIO_DATA3_REG, 8);
 
 	/*
 	 * In case GPIO9 doesn't reset the board (jumper configurable!)

arch/mips/pmc-sierra/msp71xx/msp_time.c +0 -0

index 7cfeda5..cca64e1 100644
@@ -81,10 +81,7 @@ void __init plat_time_init(void)
 	mips_hpt_frequency = cpu_rate/2;
 }
 

arch/mips/pmc-sierra/msp71xx/msp_time.c void __init plat_time_init(void) · line 87

Quoted 9 lines

arch/mips/pmc-sierra/msp71xx/msp_time.c +2 -5

-void __init plat_timer_setup(struct irqaction *irq)
+unsigned int __init get_c0_compare_int(void)
 {
-#ifdef CONFIG_IRQ_MSP_CIC
-	/* we are using the vpe0 counter for timer interrupts */
-	setup_irq(MSP_INT_VPE0_TIMER, irq);
-#endif
+	return MSP_INT_VPE0_TIMER;
 }

The rest seems ok. Can you fix the issue above and send a new patch?

Thanks!

Ralf

Hello.

Ralf Baechle wrote:

There have been a number of compile problems with the msp71xx

Quoted 7 lines

Quoted 6 lines

configuration ever since it was included in the linux-mips.org repository. This patch resolves these problems:
- proper files are included when using a squashfs rootfs
- resetting the board no longer uses non-existent GPIO routines
- create the required plat_timer_setup function

get_c0_compare_int(), you mean?

This patch has been compile-tested against the current HEAD.

Quoted 1 line

Quoted 1 line

Quoted 36 lines

Quoted 36 lines

b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
index c936756..a54e85b 100644

arch/mips/pmc-sierra/msp71xx/msp_setup.c +2 -6

@@ -21,7 +21,6 @@
 
 #if defined(CONFIG_PMC_MSP7120_GW)
 #include <msp_regops.h>
-#include <msp_gpio.h>
 #define MSP_BOARD_RESET_GPIO    9
 #endif
 

@@ -88,11 +87,8 @@ void msp7120_reset(void)
        * as GPIO char driver may not be enabled and it would look up
        * data inRAM!
        */
-       set_value_reg32(GPIO_CFG3_REG,
-                       basic_mode_mask(MSP_BOARD_RESET_GPIO),
-                       basic_mode(MSP_GPIO_OUTPUT,
 MSP_BOARD_RESET_GPIO));
-       set_reg32(GPIO_DATA3_REG,
-                       basic_data_mask(MSP_BOARD_RESET_GPIO));
+       set_value_reg32(GPIO_CFG3_REG, 0xf000, 0x8000);
+       set_reg32(GPIO_DATA3_REG, 8);
 
       /*

* In case GPIO9 doesn't reset the board (jumper configurable!)

arch/mips/pmc-sierra/msp71xx/msp_time.c +0 -0

index 7cfeda5..cca64e1 100644
@@ -81,10 +81,7 @@ void __init plat_time_init(void)
       mips_hpt_frequency = cpu_rate/2;
 }
 

Quoted 11 lines

arch/mips/pmc-sierra/msp71xx/msp_time.c void __init plat_time_init(void) · line 87

Quoted 10 lines

arch/mips/pmc-sierra/msp71xx/msp_time.c +2 -5

-void __init plat_timer_setup(struct irqaction *irq)
+unsigned int __init get_c0_compare_int(void)
 {
-#ifdef CONFIG_IRQ_MSP_CIC
-       /* we are using the vpe0 counter for timer interrupts */
-       setup_irq(MSP_INT_VPE0_TIMER, irq);
-#endif
+       return MSP_INT_VPE0_TIMER;
 }

The rest seems ok. Can you fix the issue above and send a new patch?

Quoted 1 line

Better yet 3 patches as the 3 issues seem totally unrelated.

Quoted 1 line

Ralf

Quoted 1 line

WBR, Sergei

Quoted 16 lines · Shane McDonald

There have been a number of compile problems with the msp71xx configuration ever since it was included in the linux-mips.org repository.  This patch resolves these problems:
 - proper files are included when using a squashfs rootfs
 - resetting the board no longer uses non-existent GPIO routines
 - create the required plat_timer_setup function

This patch has been compile-tested against the current HEAD.

Signed-off-by Shane McDonald <[email protected]>

---
 arch/mips/pmc-sierra/msp71xx/msp_prom.c  |    3 ++- arch/mips/pmc-sierra/msp71xx/msp_setup.c |    8 ++------ arch/mips/pmc-sierra/msp71xx/msp_time.c  |    7 ++-----

 3 files changed, 6 insertions(+), 12 deletions(-)

arch/mips/pmc-sierra/msp71xx/msp_prom.c line 48

Quoted 11 lines

arch/mips/pmc-sierra/msp71xx/msp_prom.c +2 -1

index e5bd548..1e2d984 100644
@@ -44,7 +44,8 @@
  #include <linux/cramfs_fs.h>
  #endif
  #ifdef CONFIG_SQUASHFS
-#include <linux/squashfs_fs.h>
+#include <linux/magic.h>
+#include "../../../../fs/squashfs/squashfs_fs.h"

No way.  You're reaching deep into the internals of squashfs for no good reason.  The only use of anything from squashfs_fs.h is a cast and casting to void * would work just as well.

Quoted 11 lines · Ralf Baechle

Quoted 9 lines · Shane McDonald

There have been a number of compile problems with the msp71xx configuration ever since it was included in the linux-mips.org repository. This patch resolves these problems:
- proper files are included when using a squashfs rootfs
- resetting the board no longer uses non-existent GPIO routines
- create the required plat_timer_setup function

This patch has been compile-tested against the current HEAD.

Quoted 8 lines

Review

Quoted 8 lines

Signed-off-by Shane McDonald <[email protected]>

---
 arch/mips/pmc-sierra/msp71xx/msp_prom.c  |    3 ++-
 arch/mips/pmc-sierra/msp71xx/msp_setup.c |    8 ++------
 arch/mips/pmc-sierra/msp71xx/msp_time.c  |    7 ++-----
 3 files changed, 6 insertions(+), 12 deletions(-)

(quoted diff) +0 -0

b/arch/mips/pmc-sierra/msp71xx/msp_prom.c

Quoted 5 lines

Quoted 5 lines

index e5bd548..1e2d984 100644

arch/mips/pmc-sierra/msp71xx/msp_prom.c +0 -0

@@ -44,7 +44,8 @@
 #include <linux/cramfs_fs.h>

Quoted 8 lines

arch/mips/pmc-sierra/msp71xx/msp_prom.c line 48

Quoted 5 lines

arch/mips/pmc-sierra/msp71xx/msp_prom.c +2 -1

 #endif
 #ifdef CONFIG_SQUASHFS
-#include <linux/squashfs_fs.h>
+#include <linux/magic.h>
+#include "../../../../fs/squashfs/squashfs_fs.h"

No way. You're reaching deep into the internals of squashfs for no good reason. The only use of anything from squashfs_fs.h is a cast and

casting

Quoted 1 line

to void * would work just as well.

He needs the definition of struct squashfs_super_block to access the .bytes_used
field. Alternatively, the offset of that field must be hardcoded.

BTW, the magic is __le32, and bytes_used is __le64, so there are some le{32,64}_to_cpu()
missing.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus
Torvalds

On Mon, Apr 27, 2009 at 7:22 AM, Geert Uytterhoeven <[email protected]>

Quoted 37 lines · Ralf Baechle

Quoted 27 lines · Shane McDonald

Quoted 26 lines

There have been a number of compile problems with the msp71xx configuration ever since it was included in the linux-mips.org repository.  This patch resolves these problems:
 - proper files are included when using a squashfs rootfs
 - resetting the board no longer uses non-existent GPIO routines
 - create the required plat_timer_setup function

This patch has been compile-tested against the current HEAD.

Signed-off-by Shane McDonald <[email protected]>

---
 arch/mips/pmc-sierra/msp71xx/msp_prom.c  |    3 ++- arch/mips/pmc-sierra/msp71xx/msp_setup.c |    8 ++------ arch/mips/pmc-sierra/msp71xx/msp_time.c  |    7 ++-----

 3 files changed, 6 insertions(+), 12 deletions(-)

arch/mips/pmc-sierra/msp71xx/msp_prom.c +0 -0

index e5bd548..1e2d984 100644
@@ -44,7 +44,8 @@
  #include <linux/cramfs_fs.h>
  #endif
  #ifdef CONFIG_SQUASHFS

Quoted 8 lines

arch/mips/pmc-sierra/msp71xx/msp_prom.c line 48

Quoted 3 lines

arch/mips/pmc-sierra/msp71xx/msp_prom.c +2 -1

-#include <linux/squashfs_fs.h>
+#include <linux/magic.h>
+#include "../../../../fs/squashfs/squashfs_fs.h"

No way.  You're reaching deep into the internals of squashfs for no good reason.  The only use of anything from squashfs_fs.h is a cast and casting
to void * would work just as well.

Quoted 7 lines

He needs the definition of struct squashfs_super_block to access the .bytes_used
field. Alternatively, the offset of that field must be hardcoded.

BTW, the magic is __le32, and bytes_used is __le64, so there are some le{32,64}_to_cpu()
missing.

I am not sure how to proceed here.  My main purpose in providing this patch was to get the msp71xx platform compiling again, but I stumbled into some pre-existing code ugliness, which I must confess I was ultimately responsible for.

As Geert said, I need to reach deep into the squashfs internals to access the bytes_used field of struct squashfs_super_block; the code just previous to this line works similar for the cramfs filesystem, where it accesses the size field of the struct cramfs_super field.  The cramfs code doesn't look as bad, because its superblock structure is defined in an easier-to-access file (linux/cramfs_fs.h).

I can see a number of possible paths here:

1. Fix up the existing patch with le{32,64}_to_cpu() as Geert suggested, and leave everything else as-is.
2. Approach the squashfs maintainers to move the squashfs_fs.h file to a more public location, and still do 1.
3. Pull the squashfs code entirely.
4. Remove the entire get_ramroot() code, both squashfs and cramfs, as Christoph has suggested.  I am hesitant to do this as it also affects code in the MTD subsystem (file maps/pmcmsp-ramroot.c), and it also loses some functionality on the PMC boards (putting the rootfs in RAM immediately following the kernel).  Perhaps there's a better way to handle this?

I am open to suggestions on how to proceed.

4. Remove the entire get_ramroot() code, both squashfs and cramfs, as Christoph has suggested. I am hesitant to do this as it also affects code in the MTD subsystem (file maps/pmcmsp-ramroot.c), and it also loses some functionality on the PMC boards (putting the rootfs in RAM immediately following the kernel). Perhaps there's a better way to handle this?

Quoted 6 lines · Shane McDonald

4. Remove the entire get_ramroot() code, both squashfs and cramfs, as Christoph has suggested. I am hesitant to do this as it also affects code in the MTD subsystem (file maps/pmcmsp-ramroot.c), and it also loses some functionality on the PMC boards (putting the rootfs in RAM immediately following the kernel). Perhaps there's a better way to handle this?

If the rootfs really is in ram only (and thus you discard any changes to it) you can just use an initramfs which is a lot simpler than any of the cramfs and squashfs hacks and supported by platform-independent code.

He needs the definition of struct squashfs_super_block to access the .bytes_used field. Alternatively, the offset of that field must be hardcoded.

Quoted 1 line · Geert Uytterhoeven

He needs the definition of struct squashfs_super_block to access the

.bytes_used

Quoted 1 line

field. Alternatively, the offset of that field must be hardcoded.

No, that whole crap needs to go. FS code has no business poking into fs internal structures. BTW, this whole setup is really, really gross, it's mtd map driver calling arch code to get base + size for mapping, poking into fs internal structures. I really wonder what people have been smoking to come up with crap like that.

We should just leave it uncompilable as a sign for future generations not to such stupid stuff.

If the rootfs really is in ram only (and thus you discard any changes to it) you can just use an initramfs which is a lot simpler than any of the cramfs and squashfs hacks and supported by platform-independent code.

Hello:

Quoted 7 lines · Christoph Hellwig

Quoted 1 line · Geert Uytterhoeven

He needs the definition of struct squashfs_super_block to access the

.bytes_used

Quoted 1 line

field. Alternatively, the offset of that field must be hardcoded.

Quoted 8 lines

No, that whole crap needs to go. FS code has no business poking into fs internal structures. BTW, this whole setup is really, really gross, it's mtd map driver calling arch code to get base + size for mapping, poking into fs internal structures. I really wonder what people have been smoking to come up with crap like that.

We should just leave it uncompilable as a sign for future generations not to such stupid stuff.

So, just so I'm clear, you prefer option 4 of removing the entire get_ramroot() code? :-)

Quoted 3 lines

If the rootfs really is in ram only (and thus you discard any changes to it) you can just use an initramfs which is a lot simpler than any of the cramfs and squashfs hacks and supported by platform-independent code.

The rootfs is ram only with a union mount of a jffs2 filesystem to retain changes. The target system is a resource-constrained router board, and we were trying to keep everything as small as possible. If I remember correctly, this code originally came over from an internal 2.4 port on an even more resource-constrained platform; perhaps there are better options in today's world.

I will look into a better solution to this problem. In the meantime, I'm hesitant to remove the existing code -- I think I prefer to leave it uncompilable until that solution is found.

Hi Shane,

Le Tuesday 28 April 2009 16:48:52 Shane McDonald, vous avez écrit :

Quoted 3 lines · Christoph Hellwig

Quoted 1 line

If the rootfs really is in ram only (and thus you discard any changes

to

Quoted 1 line

Quoted 1 line

it) you can just use an initramfs which is a lot simpler than any of

the

Quoted 4 lines

Quoted 1 line

cramfs and squashfs hacks and supported by platform-independent code.

The rootfs is ram only with a union mount of a jffs2 filesystem to retain changes. The target system is a resource-constrained router board, and

we

Quoted 4 lines

were trying to keep everything as small as possible. If I remember correctly, this code originally came over from an internal 2.4 port on an even more resource-constrained platform; perhaps there are better options in today's world.

Initramfs is supposed to address that kind of issue, coupled to the use of mini_fo/unionfs with a jffs2 partition for instance.

If you want to compress initramfs even more you may want to have a look at the
patch we maintain here:

https://dev.openwrt.org/browser/trunk/target/linux/brcm47xx/patches-2.6.28/500-lzma_initramfs.patch

Quoted 3 lines

I will look into a better solution to this problem. In the meantime, I'm hesitant to remove the existing code -- I think I prefer to leave it uncompilable until that solution is found.

It is likely to confuse people that may want to try get it compiling again, removing sounds like a safe bet to me.

Quoted 3 lines

If the rootfs really is in ram only (and thus you discard any changes to it) you can just use an initramfs which is a lot simpler than any of the cramfs and squashfs hacks and supported by platform-independent code.

The rootfs is ram only with a union mount of a jffs2 filesystem to retain changes. The target system is a resource-constrained router board, and we were trying to keep everything as small as possible. If I remember correctly, this code originally came over from an internal 2.4 port on an even more resource-constrained platform; perhaps there are better options in today's world.

I will look into a better solution to this problem. In the meantime, I'm hesitant to remove the existing code -- I think I prefer to leave it uncompilable until that solution is found.