Dear Daniel, In message <7909dbcf0812310223i53d16c52s208f2c704a4f6...@mail.gmail.com> you wrote: > > I'm not sure if this is the proper way to submit a patch - please feel > free to let me know what needs to be changed, if anything.
It is the proper way, but there are some problems with your patch. > > Daniel Schuler > > =================== Such explanations should follow the commit message, i., e. please put these below the "---" line, not above it. > The current SBC-2410A board from Embest ships with 2MiB of NOR FLASH, > for which CONFIG_AMD_LV160 has been defined in include/configs/sbc2410x.h Instead of extending the proprietary flash driver for this board, you should drop it completely and use the CFI flash driver instead. Needed changes are small, and you will get rid of such chip dependen- cies. > Also, CONFIG_ENV_ADDR has been made more generic by making it depend on > CONFIG_ENV_SIZE, so that only the latter needs to be changed to modify > the size of the environment. You should configure the environment such that it works with all types of flash chips, so you can use the same binary image of U-Boot on all boards. Your patch includes also other changes, which are not mentioned in the commit message, and which should (as they are unrelated) be split off into a separate patch. > In board/sbc2410x/flash.c, the appropriate #elif and case statements have > been added to support this chip. > > Signed-off-by: Daniel Schuler <dbschu...@gmail.com> > --- > board/sbc2410x/flash.c | 6 ++++++ I really recommend to get rid of this file. Use drivers/mtd/cfi_flash.c instead. > --- a/include/configs/sbc2410x.h > +++ b/include/configs/sbc2410x.h > @@ -139,7 +139,7 @@ > > #define CONFIG_SYS_LOAD_ADDR 0x33000000 /* default load > address */ > > -/* the PWM TImer 4 uses a counter of 15625 for 10 ms, so we need */ > +/* the PWM Timer 4 uses a counter of 15625 for 10 ms, so we need */ Unrelated change; should be separate patch. > /* it to wrap 100 times (total 1562500) to get 1 sec. */ > #define CONFIG_SYS_HZ 1562500 > > @@ -173,29 +173,35 @@ > */ > /* #define CONFIG_AMD_LV400 1 /\* uncomment this if you have a LV400 > flash *\/ */ > > -#define CONFIG_AMD_LV800 1 /* uncomment this if you have a LV800 > flash */ > +/* #define CONFIG_AMD_LV800 1 /\* uncomment this if you have a LV800 > flash *\/ */ > + > +#define CONFIG_AMD_LV160 1 /* uncomment this if you have a LV160 > flash */ I recommend to make the configuration independent of the actual flash chips. > #define CONFIG_SYS_MAX_FLASH_BANKS 1 /* max number of memory banks */ > > +#ifdef CONFIG_AMD_LV160 > +#define PHYS_FLASH_SIZE 0x00200000 /* 2MB */ > +#define CONFIG_SYS_MAX_FLASH_SECT (35) /* max number of sectors > on one chip */ ^^^^^^^^^^^^^^^^ and fix your mailer. Your patch is line-wrapped, so it is useless anyway. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "...all the good computer designs are bootlegged; the formally planned products, if they are built at all, are dogs!" - David E. Lundstrom, "A Few Good Men From Univac", MIT Press, 1987 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
