Dear Julius Hemanth P,
first of all, please address Bo's comment about checkpatch:
---8<--- andreas@andreas-mbp % ./tools/checkpatch.pl ~/Downloads/[PATCH\ v3]\ at91sam9x5ek_\ Pass\ serial\ and\ revision\ tags\ to\ Linux.eml ERROR: DOS line endings #70: FILE: board/atmel/at91sam9x5ek/at91sam9x5ek.c:30: +#include <asm/arch/at91_gpbr.h>^M$
ERROR: patch seems to be corrupt (line wrapped?) #75: FILE: board/atmel/at91sam9x5ek/at91sam9x5ek.c:48:
....
total: 38 errors, 7 warnings, 1 checks, 66 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE USLEEP_RANGE
/Users/andreas/Downloads/[PATCH v3] at91sam9x5ek_ Pass serial and revision tags to Linux.eml has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. --->8---
On 09.05.13 19:07, Julius Hemanth P wrote:
This code is small snippet from patch ftp://ftp.linux4sam.org/pub/uboot/u-boot-v2010.06/u-boot-5series_1.0.patch
Linux 2.6.39 (released on www.at91.com/linux4sam) requires serial and revision ATAGs to detect NAND device.
This patch provides backward compatibility for old Linux 2.6.39 by passing serial and revision ATAGs to Linux kernel.
Signed-off-by: Julius Hemanth juliushemanth@gmail.com
Changes for v3: - corrected GPBR register access - updated commit message
Changes for v2: - access GPBR using c structure - removed tailing 1 for #define - s/Miscelaneous/Miscellaneous - s/initialisations/initializations
board/atmel/at91sam9x5ek/at91sam9x5ek.c | 33 ++++++++++++++++++++++++++++++++- include/configs/at91sam9x5ek.h | 5 +++++ 2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/board/atmel/at91sam9x5ek/at91sam9x5ek.c b/board/atmel/at91sam9x5ek/at91sam9x5ek.c index 8773e6f..116bd83 100644 --- a/board/atmel/at91sam9x5ek/at91sam9x5ek.c +++ b/board/atmel/at91sam9x5ek/at91sam9x5ek.c @@ -27,6 +27,7 @@ #include <asm/arch/at91_common.h> #include <asm/arch/at91_pmc.h> #include <asm/arch/at91_rstc.h> +#include <asm/arch/at91_gpbr.h> #include <asm/arch/gpio.h> #include <asm/arch/clk.h> #include <lcd.h> @@ -48,8 +49,34 @@ DECLARE_GLOBAL_DATA_PTR;
/* ------------------------------------------------------------------------- */ /*
- Miscelaneous platform dependent initialisations
*/
- Miscellaneous platform dependent initializations
+#ifdef CONFIG_LOAD_ONE_WIRE_INFO
What the hell has this one wire thing to do with reading internal GPBR's? This is a new config parameter (which lacks documentation in this patch) is not required at all cause it is enabled in any case, or do I miss something?
+static u32 system_rev; +static u32 system_serial_low;
Can we please omit these global variables? We could just read the GPBR's in respective get-functions. This will reduce the .bss and .text size and is therefore reasonable.
+u32 get_board_rev(void) +{
return system_rev;+}
+void get_board_serial(struct tag_serialnr *serialnr) +{
serialnr->high = 0; /* Not used */serialnr->low = system_serial_low;+}
+void load_1wire_info(void) +{
at91_gpbr_t *gpbr = (at91_gpbr_t *) ATMEL_BASE_GPBR;/* serial is in GPBR #2 and revision is in GPBR #3 */
Why is that so? Which code places the serial and revision version into the GPBR's? Please add documentation and mark the usage in at91_gpbr.h. As at91_gpbr.h states GPBR[3] is already used for bootcount ... so I tend to completely NAK this patch.
As I understand Bo's comment in a prior mail this patch is only required for one specific kernel which is outdated. Can't you just patch the kernel to get the whole thing working?
Best regards
Andreas Bießmann