[MIPS] Resolve compile issues with msp71xx configuration

[MIPS] Resolve compile issues with msp71xx configuration

[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 <mcdonald.shane@gmail.com>
---
 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(-)

diff --git a/arch/mips/pmc-sierra/msp71xx/msp_prom.c b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
index e5bd548..1e2d984 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
@@ -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"
 #endif
 
 #include <asm/addrspace.h>
diff --git a/arch/mips/pmc-sierra/msp71xx/msp_setup.c b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
index c936756..a54e85b 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
@@ -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!)
diff --git a/arch/mips/pmc-sierra/msp71xx/msp_time.c b/arch/mips/pmc-sierra/msp71xx/msp_time.c
index 7cfeda5..cca64e1 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_time.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_time.c
@@ -81,10 +81,7 @@ void __init plat_time_init(void)
 	mips_hpt_frequency = cpu_rate/2;
 }
 
-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;
 }
-- 
1.5.6.5

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Ralf Baechle]

On Mon, Apr 27, 2009 at 06:59:17AM -0600, Shane McDonald wrote:

> 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 <mcdonald.shane@gmail.com>
> ---
>  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(-)
> 
> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_prom.c b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> index e5bd548..1e2d984 100644
> --- a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> +++ b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> @@ -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.

>  #endif
>  
>  #include <asm/addrspace.h>
> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_setup.c b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
> index c936756..a54e85b 100644
> --- a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
> +++ b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
> @@ -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!)
> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_time.c b/arch/mips/pmc-sierra/msp71xx/msp_time.c
> index 7cfeda5..cca64e1 100644
> --- a/arch/mips/pmc-sierra/msp71xx/msp_time.c
> +++ b/arch/mips/pmc-sierra/msp71xx/msp_time.c
> @@ -81,10 +81,7 @@ void __init plat_time_init(void)
>  	mips_hpt_frequency = cpu_rate/2;
>  }
>  
> -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

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Sergei Shtylyov]

Hello.

Ralf Baechle wrote:

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

    get_c0_compare_int(), you mean?

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

>>Signed-off-by: Shane McDonald <mcdonald.shane@gmail.com>

>>diff --git a/arch/mips/pmc-sierra/msp71xx/msp_setup.c b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>index c936756..a54e85b 100644
>>--- a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>+++ b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>@@ -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!)
>>diff --git a/arch/mips/pmc-sierra/msp71xx/msp_time.c b/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>index 7cfeda5..cca64e1 100644
>>--- a/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>+++ b/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>@@ -81,10 +81,7 @@ void __init plat_time_init(void)
>> 	mips_hpt_frequency = cpu_rate/2;
>> }
>> 
>>-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?

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

> Thanks!

>   Ralf

WBR, Sergei

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Geert Uytterhoeven]

On Mon, Apr 27, 2009 at 15:09, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Mon, Apr 27, 2009 at 06:59:17AM -0600, Shane McDonald wrote:
>
>> 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 <mcdonald.shane@gmail.com>
>> ---
>>  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(-)
>>
>> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_prom.c b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> index e5bd548..1e2d984 100644
>> --- a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> +++ b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> @@ -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.

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 -- geert@linux-m68k.org

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

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Christoph Hellwig]

On Mon, Apr 27, 2009 at 06:59:17AM -0600, Shane McDonald wrote:
>  #ifdef CONFIG_SQUASHFS
> -#include <linux/squashfs_fs.h>
> +#include <linux/magic.h>
> +#include "../../../../fs/squashfs/squashfs_fs.h"
>  #endif

Sorry guys, this has zero business to be there.  Filesystem magic checks
of any sort don't belong into platform code.  Please kill that whole
get_ramroot piece of junk.

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Shane McDonald]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

Hi:

On Mon, Apr 27, 2009 at 7:18 AM, Sergei Shtylyov <sshtylyov@ru.mvista.com>wrote:

> Hello.
>
> Ralf Baechle wrote:
>
>  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
>>>
>>
>   get_c0_compare_int(), you mean?
>

Yes, I should have said, remove the duplicate definition of plat_timer_setup
and add the required definition of get_c0_compare_int.


> This patch has been compile-tested against the current HEAD.
>>
>
 Signed-off-by: Shane McDonald <mcdonald.shane@gmail.com>
>>
>
 diff --git a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>> b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>> index c936756..a54e85b 100644
>>> --- a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>> +++ b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
>>> @@ -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!)
>>> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>> b/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>> index 7cfeda5..cca64e1 100644
>>> --- a/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>> +++ b/arch/mips/pmc-sierra/msp71xx/msp_time.c
>>> @@ -81,10 +81,7 @@ void __init plat_time_init(void)
>>>        mips_hpt_frequency = cpu_rate/2;
>>> }
>>>
>>> -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?
>>
>
>   Better yet 3 patches as the 3 issues seem totally unrelated.


I will respin as three patches.  It looks like the squashfs part is
controversial, but the other two shouldn't be affected.

 Thanks!
>>
>
>   Ralf
>>
>
> WBR, Sergei


Shane

[-- Attachment #2: Type: text/html, Size: 5387 bytes --]

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Shane McDonald]

[-- Attachment #1: Type: text/plain, Size: 3697 bytes --]

Hello:

On Mon, Apr 27, 2009 at 7:22 AM, Geert Uytterhoeven <geert@linux-m68k.org>wrote:

> On Mon, Apr 27, 2009 at 15:09, Ralf Baechle <ralf@linux-mips.org> wrote:
> > On Mon, Apr 27, 2009 at 06:59:17AM -0600, Shane McDonald wrote:
> >
> >> 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 <mcdonald.shane@gmail.com>
> >> ---
> >>  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(-)
> >>
> >> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> >> index e5bd548..1e2d984 100644
> >> --- a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> >> +++ b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
> >> @@ -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.
>
> 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.


>
> Gr{oetje,eeting}s,
>
>                                                Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> 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


Shane

[-- Attachment #2: Type: text/html, Size: 4824 bytes --]

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Geert Uytterhoeven]

On Tue, Apr 28, 2009 at 08:21, Shane McDonald <mcdonald.shane@gmail.com> wrote:
> On Mon, Apr 27, 2009 at 7:22 AM, Geert Uytterhoeven <geert@linux-m68k.org>

>> On Mon, Apr 27, 2009 at 15:09, Ralf Baechle <ralf@linux-mips.org> wrote:
>> > On Mon, Apr 27, 2009 at 06:59:17AM -0600, Shane McDonald wrote:
>> >
>> >> 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 <mcdonald.shane@gmail.com>
>> >> ---
>> >>  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(-)
>> >>
>> >> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> >> b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> >> index e5bd548..1e2d984 100644
>> >> --- a/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> >> +++ b/arch/mips/pmc-sierra/msp71xx/msp_prom.c
>> >> @@ -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.
>>
>> 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.

The only reason you need to know about the file system internals is to
find out the
size of the appended file system, right?

The way other platforms solve this is by e.g. letting the bootloader
pass the location
and size of the ramdisk to the kernel. This is file system agnostic.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Christoph Hellwig]

On Mon, Apr 27, 2009 at 03:22:33PM +0200, Geert Uytterhoeven wrote:
> He needs the definition of struct squashfs_super_block to access the .bytes_used
> 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.

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Christoph Hellwig]

On Tue, Apr 28, 2009 at 12:21:09AM -0600, Shane McDonald wrote:
> 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.

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Kevin D. Kissell]

Christoph Hellwig wrote:
> On Tue, Apr 28, 2009 at 12:21:09AM -0600, Shane McDonald wrote:
>   
>> 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.
>   
I second that emotion.

          Kevin K.

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Shane McDonald]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

Hello:

On Tue, Apr 28, 2009 at 3:20 AM, Christoph Hellwig <hch@lst.de> wrote:

> On Mon, Apr 27, 2009 at 03:22:33PM +0200, Geert Uytterhoeven wrote:
> > He needs the definition of struct squashfs_super_block to access the
> .bytes_used
> > 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.


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

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

Shane

[-- Attachment #2: Type: text/html, Size: 2084 bytes --]

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Florian Fainelli]

Hi Shane,

Le Tuesday 28 April 2009 16:48:52 Shane McDonald, vous avez écrit :
> Hello:
>
> On Tue, Apr 28, 2009 at 3:20 AM, Christoph Hellwig <hch@lst.de> wrote:
> > On Mon, Apr 27, 2009 at 03:22:33PM +0200, Geert Uytterhoeven wrote:
> > > He needs the definition of struct squashfs_super_block to access the
> >
> > .bytes_used
> >
> > > 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.
>
> So, just so I'm clear, you prefer option 4 of removing the entire
> get_ramroot() code? :-)
>
> > 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.

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

>
> 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.
-- 
Best regards, Florian Fainelli
Email : florian@openwrt.org
http://openwrt.org
-------------------------------

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Ralf Baechle]

On Tue, Apr 28, 2009 at 08:48:52AM -0600, Shane McDonald wrote:

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

You may want to chainsaw it into a shape were it's usable in some way -
for example with NFS root until you have a chance to sort this out.

  Ralf

Re: [MIPS] Resolve compile issues with msp71xx configuration

[Shane McDonald]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

Hello Florian:

On Tue, Apr 28, 2009 at 9:05 AM, Florian Fainelli <florian@openwrt.org>wrote:

> Hi Shane,
>
> Le Tuesday 28 April 2009 16:48:52 Shane McDonald, vous avez écrit :
> >
> > On Tue, Apr 28, 2009 at 3:20 AM, Christoph Hellwig <hch@lst.de> wrote:
> > > 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.
>
> 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
>


Excellent -- it looks like I have a good path forward!  Thanks for the
pointer.


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


Based on your and Ralf's request, I'll prepare a patch tonight to get this
compiling again.

Shane

[-- Attachment #2: Type: text/html, Size: 2573 bytes --]