Dear Wolfgang/Jean-Christophe
I'd be grateful if you could give me some advice on this please as I haven't been working with u-boot for very long.
Although the code you've both commented on is new it uses a lot of the existing Samsung S3C common code in cpu/arm920t/s3c24x0 and various s3c24x0 drivers code. This common code also has the problems you commented on and additionally it doesn't meet the coding style requirements. It would probably be better for me to re-work the existing common code first and submit a patch, then submit a new SBC2440-II Board patch based on the code after the re-work patch has been applied. I'm happy to do this but I just have a few questions:
- if I re-work the existing common S3C2410 code I can check the existing Samsung S3C boards still build but how do I make sure they still work (I can't test them as I only have an SBC2440-II Board)? - once I've re-worked the existing common S3C2410 code a new patch for the SBC2440-II Board would only really make sense if it was based on the code after the common code re-work patch was applied. Is it OK to submit an SBC2440 Board patch that assumes the re-work patch has been applied first?
I'm sure this has come up before but I can't find any mention of it in the mail archives.
Regards Kevin Morfitt
03/06/2009 11:05, Wolfgang Denk wrote:
Dear "kevin.morfitt@fearnside-systems.co.uk",
In message 4A263923.2030703@fearnside-systems.co.uk you wrote:
+#define BWSCON 0x48000000
+#define DW8 (0x0) +#define DW16 (0x1) +#define DW32 (0x2) +#define WAIT (0x1 << 2) +#define UBLB (0x1 << 3)
what is all theses macro? for what use?
They make setting the fields of the memory controller registers more readable. For example BSWCON is the Bus Width and Wait Control Register, DW32 sets a Data Width value bit-field in BWSCON to 8-bit etc. They're used later in the patch to configure the memory controller registers. The names are the same as those used in the S3C2440 data sheet.
Please make sure to use C structs to descrive device register layout, and use proper I/O accessor functions for the actual access.
- gpio->GPBCON = 0x00055555;
- gpio->GPBUP = 0x000007FF;
- gpio->GPBDAT = 0x000001C0; /* Switch on LED1. */
- gpio->GPCCON = 0xAAAAAAAA;
- gpio->GPCUP = 0x0000FFFF;
- gpio->GPDCON = 0xAAAAAAAA;
Such code will not be accepted. Please use C structs and I/O accessor functions.
Best regards,
Wolfgang Denk