Patchwork [U-Boot,1/5] omap3_beagle: remove JFFS2 support.

login
register
mail settings
Submitter Nishanth Menon
Date July 11, 2013, 9:52 p.m.
Message ID <1373579559-5697-2-git-send-email-nm@ti.com>
Download mbox | patch
Permalink /patch/258628/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Nishanth Menon - July 11, 2013, 9:52 p.m.
We do not use JFFS2 by default and it conflicts with
CONFIG_CMD_FS_GENERIC (ls command is the same). Since most of our
BOOTCMD can be simplified by using the FS_GENERIC, dropping JFFS2

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 include/configs/omap3_beagle.h |    8 --------
 1 file changed, 8 deletions(-)
Joel Fernandes - July 12, 2013, 2:49 a.m.
On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
> We do not use JFFS2 by default and it conflicts with
> CONFIG_CMD_FS_GENERIC (ls command is the same). Since most of our
> BOOTCMD can be simplified by using the FS_GENERIC, dropping JFFS2
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  include/configs/omap3_beagle.h |    8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> index 48ce4c0..9adf4a5 100644
> --- a/include/configs/omap3_beagle.h
> +++ b/include/configs/omap3_beagle.h
> @@ -150,7 +150,6 @@
>  #define CONFIG_CMD_CACHE
>  #define CONFIG_CMD_EXT2                /* EXT2 Support                 */
>  #define CONFIG_CMD_FAT         /* FAT support                  */
> -#define CONFIG_CMD_JFFS2       /* JFFS2 Support                */
>  #define CONFIG_CMD_MTDPARTS    /* Enable MTD parts commands */
>  #define CONFIG_MTD_DEVICE      /* needed for mtdparts commands */
>  #define MTDIDS_DEFAULT                 "nand0=nand"
> @@ -203,13 +202,6 @@
>
>  #define CONFIG_SYS_MAX_NAND_DEVICE     1               /* Max number of NAND */
>                                                         /* devices */
> -#define CONFIG_JFFS2_NAND
> -/* nand device jffs2 lives on */
> -#define CONFIG_JFFS2_DEV               "nand0"
> -/* start of jffs2 partition */
> -#define CONFIG_JFFS2_PART_OFFSET       0x680000
> -#define CONFIG_JFFS2_PART_SIZE         0xf980000       /* size of jffs2 */
>

Minor nit, can we perhaps leave the JFFS configuration and only remove/comment
CONFIG_CMD_JFFS. That way we wont lose these paritition offsets/size for folks
who might need them.

Otherwise,
Acked-by: Joel Fernandes <joelf@ti.com>

Thanks,

-Joel
Nishanth Menon - July 12, 2013, 3:49 a.m.
On 21:49-20130711, Joel Fernandes wrote:
> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
> > We do not use JFFS2 by default and it conflicts with
> > CONFIG_CMD_FS_GENERIC (ls command is the same). Since most of our
> > BOOTCMD can be simplified by using the FS_GENERIC, dropping JFFS2
> >
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  include/configs/omap3_beagle.h |    8 --------
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> > index 48ce4c0..9adf4a5 100644
> > --- a/include/configs/omap3_beagle.h
> > +++ b/include/configs/omap3_beagle.h
> > @@ -150,7 +150,6 @@
> >  #define CONFIG_CMD_CACHE
> >  #define CONFIG_CMD_EXT2                /* EXT2 Support                 */
> >  #define CONFIG_CMD_FAT         /* FAT support                  */
> > -#define CONFIG_CMD_JFFS2       /* JFFS2 Support                */
> >  #define CONFIG_CMD_MTDPARTS    /* Enable MTD parts commands */
> >  #define CONFIG_MTD_DEVICE      /* needed for mtdparts commands */
> >  #define MTDIDS_DEFAULT                 "nand0=nand"
> > @@ -203,13 +202,6 @@
> >
> >  #define CONFIG_SYS_MAX_NAND_DEVICE     1               /* Max number of NAND */
> >                                                         /* devices */
> > -#define CONFIG_JFFS2_NAND
> > -/* nand device jffs2 lives on */
> > -#define CONFIG_JFFS2_DEV               "nand0"
> > -/* start of jffs2 partition */
> > -#define CONFIG_JFFS2_PART_OFFSET       0x680000
> > -#define CONFIG_JFFS2_PART_SIZE         0xf980000       /* size of jffs2 */
> >
> 
> Minor nit, can we perhaps leave the JFFS configuration and only remove/comment
> CONFIG_CMD_JFFS. That way we wont lose these paritition offsets/size for folks
> who might need them.
then we have to drop the usage of FS_GENERIC. code wont build with both.

There is no point in keeping something as an option if it cant be used.
dont you agree?
Joel Fernandes - July 12, 2013, 4:25 a.m.
On Thu, Jul 11, 2013 at 10:49 PM, Nishanth Menon <nm@ti.com> wrote:
> On 21:49-20130711, Joel Fernandes wrote:
>> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
>> > We do not use JFFS2 by default and it conflicts with
>> > CONFIG_CMD_FS_GENERIC (ls command is the same). Since most of our
>> > BOOTCMD can be simplified by using the FS_GENERIC, dropping JFFS2
>> >
>> > Signed-off-by: Nishanth Menon <nm@ti.com>
>> > ---
>> >  include/configs/omap3_beagle.h |    8 --------
>> >  1 file changed, 8 deletions(-)
>> >
>> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
>> > index 48ce4c0..9adf4a5 100644
>> > --- a/include/configs/omap3_beagle.h
>> > +++ b/include/configs/omap3_beagle.h
>> > @@ -150,7 +150,6 @@
>> >  #define CONFIG_CMD_CACHE
>> >  #define CONFIG_CMD_EXT2                /* EXT2 Support                 */
>> >  #define CONFIG_CMD_FAT         /* FAT support                  */
>> > -#define CONFIG_CMD_JFFS2       /* JFFS2 Support                */
>> >  #define CONFIG_CMD_MTDPARTS    /* Enable MTD parts commands */
>> >  #define CONFIG_MTD_DEVICE      /* needed for mtdparts commands */
>> >  #define MTDIDS_DEFAULT                 "nand0=nand"
>> > @@ -203,13 +202,6 @@
>> >
>> >  #define CONFIG_SYS_MAX_NAND_DEVICE     1               /* Max number of NAND */
>> >                                                         /* devices */
>> > -#define CONFIG_JFFS2_NAND
>> > -/* nand device jffs2 lives on */
>> > -#define CONFIG_JFFS2_DEV               "nand0"
>> > -/* start of jffs2 partition */
>> > -#define CONFIG_JFFS2_PART_OFFSET       0x680000
>> > -#define CONFIG_JFFS2_PART_SIZE         0xf980000       /* size of jffs2 */
>> >
>>
>> Minor nit, can we perhaps leave the JFFS configuration and only remove/comment
>> CONFIG_CMD_JFFS. That way we wont lose these paritition offsets/size for folks
>> who might need them.
> then we have to drop the usage of FS_GENERIC. code wont build with both.

No I meant drop CMD_JFFS but not the other JFFS.

> There is no point in keeping something as an option if it cant be used.
> dont you agree?

Either way is OK with me, you have a point too.

Thanks,

-Joel
Nishanth Menon - July 12, 2013, 4:35 a.m.
On 23:25-20130711, Joel Fernandes wrote:
> On Thu, Jul 11, 2013 at 10:49 PM, Nishanth Menon <nm@ti.com> wrote:
> > On 21:49-20130711, Joel Fernandes wrote:
> >> On Thu, Jul 11, 2013 at 4:52 PM, Nishanth Menon <nm@ti.com> wrote:
> >> > We do not use JFFS2 by default and it conflicts with
> >> > CONFIG_CMD_FS_GENERIC (ls command is the same). Since most of our
> >> > BOOTCMD can be simplified by using the FS_GENERIC, dropping JFFS2
> >> >
> >> > Signed-off-by: Nishanth Menon <nm@ti.com>
> >> > ---
> >> >  include/configs/omap3_beagle.h |    8 --------
> >> >  1 file changed, 8 deletions(-)
> >> >
> >> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> >> > index 48ce4c0..9adf4a5 100644
> >> > --- a/include/configs/omap3_beagle.h
> >> > +++ b/include/configs/omap3_beagle.h
> >> > @@ -150,7 +150,6 @@
> >> >  #define CONFIG_CMD_CACHE
> >> >  #define CONFIG_CMD_EXT2                /* EXT2 Support                 */
> >> >  #define CONFIG_CMD_FAT         /* FAT support                  */
> >> > -#define CONFIG_CMD_JFFS2       /* JFFS2 Support                */
> >> >  #define CONFIG_CMD_MTDPARTS    /* Enable MTD parts commands */
> >> >  #define CONFIG_MTD_DEVICE      /* needed for mtdparts commands */
> >> >  #define MTDIDS_DEFAULT                 "nand0=nand"
> >> > @@ -203,13 +202,6 @@
> >> >
> >> >  #define CONFIG_SYS_MAX_NAND_DEVICE     1               /* Max number of NAND */
> >> >                                                         /* devices */
> >> > -#define CONFIG_JFFS2_NAND
> >> > -/* nand device jffs2 lives on */
> >> > -#define CONFIG_JFFS2_DEV               "nand0"
> >> > -/* start of jffs2 partition */
> >> > -#define CONFIG_JFFS2_PART_OFFSET       0x680000
> >> > -#define CONFIG_JFFS2_PART_SIZE         0xf980000       /* size of jffs2 */
> >> >
> >>
> >> Minor nit, can we perhaps leave the JFFS configuration and only remove/comment
> >> CONFIG_CMD_JFFS. That way we wont lose these paritition offsets/size for folks
> >> who might need them.
> > then we have to drop the usage of FS_GENERIC. code wont build with both.
> 
> No I meant drop CMD_JFFS but not the other JFFS.

Those wont come into play unless CMD_JFFS2 is enabled, and as stated,
makes no sense to keep the second part around and trick some poor bloke
into thinking CMD_JFFS2 was left out by mistake..

> 
> > There is no point in keeping something as an option if it cant be used.
> > dont you agree?
> 
> Either way is OK with me, you have a point too.
OK, will take this to mean that you are ok with the patch as is. Thanks
for the review.

Patch

diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 48ce4c0..9adf4a5 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -150,7 +150,6 @@ 
 #define CONFIG_CMD_CACHE
 #define CONFIG_CMD_EXT2		/* EXT2 Support			*/
 #define CONFIG_CMD_FAT		/* FAT support			*/
-#define CONFIG_CMD_JFFS2	/* JFFS2 Support		*/
 #define CONFIG_CMD_MTDPARTS	/* Enable MTD parts commands */
 #define CONFIG_MTD_DEVICE	/* needed for mtdparts commands */
 #define MTDIDS_DEFAULT			"nand0=nand"
@@ -203,13 +202,6 @@ 
 
 #define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND */
 							/* devices */
-#define CONFIG_JFFS2_NAND
-/* nand device jffs2 lives on */
-#define CONFIG_JFFS2_DEV		"nand0"
-/* start of jffs2 partition */
-#define CONFIG_JFFS2_PART_OFFSET	0x680000
-#define CONFIG_JFFS2_PART_SIZE		0xf980000	/* size of jffs2 */
-							/* partition */
 
 /* Environment information */
 #define CONFIG_BOOTDELAY		3