* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
@ 2008-10-22 5:16 Kyungmin Park
2008-10-22 12:47 ` Jean-Christophe PLAGNIOL-VILLARD
2008-10-23 7:39 ` Guennadi Liakhovetski
0 siblings, 2 replies; 14+ messages in thread
From: Kyungmin Park @ 2008-10-22 5:16 UTC (permalink / raw)
To: u-boot
Move machine specific code to smdk6400.
Some board use OneNAND instead of NAND.
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
diff --git a/board/samsung/smdk6400/lowlevel_init.S b/board/samsung/smdk6400/lowlevel_init.S
index e0119a7..53d9125 100644
--- a/board/samsung/smdk6400/lowlevel_init.S
+++ b/board/samsung/smdk6400/lowlevel_init.S
@@ -104,6 +104,13 @@ lowlevel_init:
bl nand_asm_init
#endif
+ /* Memory subsystem address 0x7e00f120 */
+ ldr r0, =ELFIN_MEM_SYS_CFG
+
+ /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
+ mov r1, #0xd
+ str r1, [r0]
+
bl mem_ctrl_asm_init
/* Wakeup support. Don't know if it's going to be used, untested. */
diff --git a/cpu/arm1176/s3c64xx/cpu_init.S b/cpu/arm1176/s3c64xx/cpu_init.S
index 08bda99..32bb467 100644
--- a/cpu/arm1176/s3c64xx/cpu_init.S
+++ b/cpu/arm1176/s3c64xx/cpu_init.S
@@ -28,13 +28,6 @@
.globl mem_ctrl_asm_init
mem_ctrl_asm_init:
- /* Memory subsystem address 0x7e00f120 */
- ldr r0, =ELFIN_MEM_SYS_CFG
-
- /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
- mov r1, #0xd
- str r1, [r0]
-
/* DMC1 base address 0x7e001000 */
ldr r0, =ELFIN_DMC1_BASE
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-22 5:16 [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx Kyungmin Park
@ 2008-10-22 12:47 ` Jean-Christophe PLAGNIOL-VILLARD
2008-10-23 7:39 ` Guennadi Liakhovetski
1 sibling, 0 replies; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2008-10-22 12:47 UTC (permalink / raw)
To: u-boot
On 14:16 Wed 22 Oct , Kyungmin Park wrote:
> Move machine specific code to smdk6400.
> Some board use OneNAND instead of NAND.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/board/samsung/smdk6400/lowlevel_init.S b/board/samsung/smdk6400/lowlevel_init.S
Guennadi could you test it?
Best Regards,
J.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-22 5:16 [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx Kyungmin Park
2008-10-22 12:47 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2008-10-23 7:39 ` Guennadi Liakhovetski
2008-10-23 7:47 ` Kyungmin Park
2008-10-23 8:16 ` Wolfgang Denk
1 sibling, 2 replies; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 7:39 UTC (permalink / raw)
To: u-boot
On Wed, 22 Oct 2008, Kyungmin Park wrote:
> Move machine specific code to smdk6400.
> Some board use OneNAND instead of NAND.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/board/samsung/smdk6400/lowlevel_init.S b/board/samsung/smdk6400/lowlevel_init.S
> index e0119a7..53d9125 100644
> --- a/board/samsung/smdk6400/lowlevel_init.S
> +++ b/board/samsung/smdk6400/lowlevel_init.S
> @@ -104,6 +104,13 @@ lowlevel_init:
> bl nand_asm_init
> #endif
>
> + /* Memory subsystem address 0x7e00f120 */
> + ldr r0, =ELFIN_MEM_SYS_CFG
> +
> + /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
> + mov r1, #0xd
> + str r1, [r0]
> +
Hm, no, I don't quite agree. In principle, yes, this configuration can be
considered platform-specific and moving it this way of course works. But:
1. The patch comment is not correct. This code doesn't select between NAND
and OneNAND. It selects between (one of) NANDs and SROMs.
2. While at it, we could fix the value being written to the MEM_SYS_CFG
register too. Currently it writes 0xd =
(1 << 0) - ignored, default 0, so, better set it to 0
| (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
| (1 << 2) - ignored, default 0, so, better set it to 0
| (1 << 3) - set Xm0CSn[3] to SROMC CS3
So, we should just write an 8 in it:
+ mov r1, #0x8
+ str r1, [r0]
3. The comment in the code doesn't look right. According to the above it
should read
+ /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
The only thing that confuses me, is why the author, belonging to the
manufacturer of the chip, hasn't done this. Maybe the documentation is
wrong?
I tested it with the "8" - it works. So, once the issues are fixed, you
get my "Tested-by", although, I guess, you can test it just as well:-)
Thanks
Guennadi
> bl mem_ctrl_asm_init
>
> /* Wakeup support. Don't know if it's going to be used, untested. */
> diff --git a/cpu/arm1176/s3c64xx/cpu_init.S b/cpu/arm1176/s3c64xx/cpu_init.S
> index 08bda99..32bb467 100644
> --- a/cpu/arm1176/s3c64xx/cpu_init.S
> +++ b/cpu/arm1176/s3c64xx/cpu_init.S
> @@ -28,13 +28,6 @@
>
> .globl mem_ctrl_asm_init
> mem_ctrl_asm_init:
> - /* Memory subsystem address 0x7e00f120 */
> - ldr r0, =ELFIN_MEM_SYS_CFG
> -
> - /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
> - mov r1, #0xd
> - str r1, [r0]
> -
> /* DMC1 base address 0x7e001000 */
> ldr r0, =ELFIN_DMC1_BASE
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 7:39 ` Guennadi Liakhovetski
@ 2008-10-23 7:47 ` Kyungmin Park
2008-10-23 8:00 ` Guennadi Liakhovetski
2008-10-23 8:16 ` Wolfgang Denk
1 sibling, 1 reply; 14+ messages in thread
From: Kyungmin Park @ 2008-10-23 7:47 UTC (permalink / raw)
To: u-boot
On Thu, Oct 23, 2008 at 4:39 PM, Guennadi Liakhovetski <lg@denx.de> wrote:
> On Wed, 22 Oct 2008, Kyungmin Park wrote:
>
>> Move machine specific code to smdk6400.
>> Some board use OneNAND instead of NAND.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> diff --git a/board/samsung/smdk6400/lowlevel_init.S b/board/samsung/smdk6400/lowlevel_init.S
>> index e0119a7..53d9125 100644
>> --- a/board/samsung/smdk6400/lowlevel_init.S
>> +++ b/board/samsung/smdk6400/lowlevel_init.S
>> @@ -104,6 +104,13 @@ lowlevel_init:
>> bl nand_asm_init
>> #endif
>>
>> + /* Memory subsystem address 0x7e00f120 */
>> + ldr r0, =ELFIN_MEM_SYS_CFG
>> +
>> + /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
>> + mov r1, #0xd
>> + str r1, [r0]
>> +
>
> Hm, no, I don't quite agree. In principle, yes, this configuration can be
> considered platform-specific and moving it this way of course works. But:
>
> 1. The patch comment is not correct. This code doesn't select between NAND
> and OneNAND. It selects between (one of) NANDs and SROMs.
Yes I just move to platform since it's board specific.
>
> 2. While at it, we could fix the value being written to the MEM_SYS_CFG
> register too. Currently it writes 0xd =
>
> (1 << 0) - ignored, default 0, so, better set it to 0
> | (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
> | (1 << 2) - ignored, default 0, so, better set it to 0
> | (1 << 3) - set Xm0CSn[3] to SROMC CS3
>
> So, we should just write an 8 in it:
>
> + mov r1, #0x8
> + str r1, [r0]
>
> 3. The comment in the code doesn't look right. According to the above it
> should read
>
> + /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
Right, and also add OneNAND & NFCON is depends on XNANDSEL.
As you know mem_ctrl_asm_init is common code and other boards can use
it without board specific codes.
In OneNAND board, it should be set as 0x1002
>
> The only thing that confuses me, is why the author, belonging to the
> manufacturer of the chip, hasn't done this. Maybe the documentation is
> wrong?
>
Maybe he missed it. Document is right.
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 7:47 ` Kyungmin Park
@ 2008-10-23 8:00 ` Guennadi Liakhovetski
2008-10-23 8:34 ` Kyungmin Park
2008-10-23 8:49 ` Wolfgang Denk
0 siblings, 2 replies; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 8:00 UTC (permalink / raw)
To: u-boot
On Thu, 23 Oct 2008, Kyungmin Park wrote:
> > (1 << 0) - ignored, default 0, so, better set it to 0
> > | (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
> > | (1 << 2) - ignored, default 0, so, better set it to 0
> > | (1 << 3) - set Xm0CSn[3] to SROMC CS3
> >
> > So, we should just write an 8 in it:
> >
> > + mov r1, #0x8
> > + str r1, [r0]
> >
> > 3. The comment in the code doesn't look right. According to the above it
> > should read
> >
> > + /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
>
> Right, and also add OneNAND & NFCON is depends on XNANDSEL.
In the datasheet this signal is called XSELNAND. And I don't think we have
to quote this in the comment. This is a hardware configuration issue, not
software, and we are not explaining the complete NAND configuration here,
otherwise we would have to mention OM signals too, maybe more.
> As you know mem_ctrl_asm_init is common code and other boards can use
> it without board specific codes.
>
> In OneNAND board, it should be set as 0x1002
Sorry, do not understand what "it." If you mean the MEM_SYS_CFG then I
also don't understand this. As I quoted from the datasheet above, bit 1
set to 0 (0 << 1) is for _both_ - NAND or OneNAND. You suggest to set it
to 1, which is SROMC CS2. And (1 << 12) is the data bus width, which also
doesn't seem to be directly related to the NAND / OneNAND selection. Or
did you mean another register?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 7:39 ` Guennadi Liakhovetski
2008-10-23 7:47 ` Kyungmin Park
@ 2008-10-23 8:16 ` Wolfgang Denk
1 sibling, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2008-10-23 8:16 UTC (permalink / raw)
To: u-boot
Dear Guennadi Liakhovetski,
In message <Pine.LNX.4.64.0810230927050.4977@axis700.grange> you wrote:
>
> 2. While at it, we could fix the value being written to the MEM_SYS_CFG
> register too. Currently it writes 0xd =
>
> (1 << 0) - ignored, default 0, so, better set it to 0
> | (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
> | (1 << 2) - ignored, default 0, so, better set it to 0
> | (1 << 3) - set Xm0CSn[3] to SROMC CS3
>
> So, we should just write an 8 in it:
>
> + mov r1, #0x8
> + str r1, [r0]
No, you should not use magic numbers like 0x08 or 0x0d which nobody
can read but use meaningful preprocessor constants here so we actually
understand the code without looking up the bits in the documentation.
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: wd at denx.de
Death, when unnecessary, is a tragic thing.
-- Flint, "Requiem for Methuselah", stardate 5843.7
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:00 ` Guennadi Liakhovetski
@ 2008-10-23 8:34 ` Kyungmin Park
2008-10-23 8:42 ` Guennadi Liakhovetski
2008-10-23 8:49 ` Wolfgang Denk
1 sibling, 1 reply; 14+ messages in thread
From: Kyungmin Park @ 2008-10-23 8:34 UTC (permalink / raw)
To: u-boot
On Thu, Oct 23, 2008 at 5:00 PM, Guennadi Liakhovetski <lg@denx.de> wrote:
> On Thu, 23 Oct 2008, Kyungmin Park wrote:
>
>> > (1 << 0) - ignored, default 0, so, better set it to 0
>> > | (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
>> > | (1 << 2) - ignored, default 0, so, better set it to 0
>> > | (1 << 3) - set Xm0CSn[3] to SROMC CS3
>> >
>> > So, we should just write an 8 in it:
>> >
>> > + mov r1, #0x8
>> > + str r1, [r0]
>> >
>> > 3. The comment in the code doesn't look right. According to the above it
>> > should read
>> >
>> > + /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
>>
>> Right, and also add OneNAND & NFCON is depends on XNANDSEL.
>
> In the datasheet this signal is called XSELNAND. And I don't think we have
> to quote this in the comment. This is a hardware configuration issue, not
> software, and we are not explaining the complete NAND configuration here,
> otherwise we would have to mention OM signals too, maybe more.
>
>> As you know mem_ctrl_asm_init is common code and other boards can use
>> it without board specific codes.
>>
>> In OneNAND board, it should be set as 0x1002
>
> Sorry, do not understand what "it." If you mean the MEM_SYS_CFG then I
> also don't understand this. As I quoted from the datasheet above, bit 1
> set to 0 (0 << 1) is for _both_ - NAND or OneNAND. You suggest to set it
> to 1, which is SROMC CS2. And (1 << 12) is the data bus width, which also
> doesn't seem to be directly related to the NAND / OneNAND selection. Or
> did you mean another register?
>
Right, I write wrong value, MEM_SYS_CFG has 0x1000. In OneNAND booting
mode, MP0_CS_CFG[1] and MP0_CS_CFG[3] are ignored.
It's not easy to describe since it depends on hardware configuration.
However, there are not too much configurations
S3C64XX_MEM_SYS_CFG_NAND 0x0008
S3C64XX_MEM_SYS_CFG_ONENAND 0x1000
S3C64XX_MEM_SYS_CFG_MOVINAND 0x????
Is there more?
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:34 ` Kyungmin Park
@ 2008-10-23 8:42 ` Guennadi Liakhovetski
2008-10-23 8:48 ` Kyungmin Park
0 siblings, 1 reply; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 8:42 UTC (permalink / raw)
To: u-boot
On Thu, 23 Oct 2008, Kyungmin Park wrote:
> >> In OneNAND board, it should be set as 0x1002
> >
> > Sorry, do not understand what "it." If you mean the MEM_SYS_CFG then I
> > also don't understand this. As I quoted from the datasheet above, bit 1
> > set to 0 (0 << 1) is for _both_ - NAND or OneNAND. You suggest to set it
> > to 1, which is SROMC CS2. And (1 << 12) is the data bus width, which also
> > doesn't seem to be directly related to the NAND / OneNAND selection. Or
> > did you mean another register?
> >
>
> Right, I write wrong value, MEM_SYS_CFG has 0x1000. In OneNAND booting
> mode, MP0_CS_CFG[1] and MP0_CS_CFG[3] are ignored.
>
> It's not easy to describe since it depends on hardware configuration.
> However, there are not too much configurations
>
> S3C64XX_MEM_SYS_CFG_NAND 0x0008
> S3C64XX_MEM_SYS_CFG_ONENAND 0x1000
????? I asked above what the bus width has to do with OneNAND selection,
you didn't reply.
> S3C64XX_MEM_SYS_CFG_MOVINAND 0x????
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:42 ` Guennadi Liakhovetski
@ 2008-10-23 8:48 ` Kyungmin Park
2008-10-23 8:56 ` Guennadi Liakhovetski
0 siblings, 1 reply; 14+ messages in thread
From: Kyungmin Park @ 2008-10-23 8:48 UTC (permalink / raw)
To: u-boot
On Thu, Oct 23, 2008 at 5:42 PM, Guennadi Liakhovetski <lg@denx.de> wrote:
> On Thu, 23 Oct 2008, Kyungmin Park wrote:
>
>> >> In OneNAND board, it should be set as 0x1002
>> >
>> > Sorry, do not understand what "it." If you mean the MEM_SYS_CFG then I
>> > also don't understand this. As I quoted from the datasheet above, bit 1
>> > set to 0 (0 << 1) is for _both_ - NAND or OneNAND. You suggest to set it
>> > to 1, which is SROMC CS2. And (1 << 12) is the data bus width, which also
>> > doesn't seem to be directly related to the NAND / OneNAND selection. Or
>> > did you mean another register?
>> >
>>
>> Right, I write wrong value, MEM_SYS_CFG has 0x1000. In OneNAND booting
>> mode, MP0_CS_CFG[1] and MP0_CS_CFG[3] are ignored.
>>
>> It's not easy to describe since it depends on hardware configuration.
>> However, there are not too much configurations
>>
>> S3C64XX_MEM_SYS_CFG_NAND 0x0008
>> S3C64XX_MEM_SYS_CFG_ONENAND 0x1000
>
> ????? I asked above what the bus width has to do with OneNAND selection,
> you didn't reply.
OneNAND has always 16-bit butwidth. there's no exception.
>
>> S3C64XX_MEM_SYS_CFG_MOVINAND 0x????
I don't tested boot from MMC. So I don't know which value is right
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:00 ` Guennadi Liakhovetski
2008-10-23 8:34 ` Kyungmin Park
@ 2008-10-23 8:49 ` Wolfgang Denk
2008-10-23 9:00 ` Guennadi Liakhovetski
1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2008-10-23 8:49 UTC (permalink / raw)
To: u-boot
Dear Guennadi Liakhovetski,
In message <Pine.LNX.4.64.0810230954380.4977@axis700.grange> you wrote:
>
> > > + /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
> >
> > Right, and also add OneNAND & NFCON is depends on XNANDSEL.
>
> In the datasheet this signal is called XSELNAND. And I don't think we have
> to quote this in the comment. This is a hardware configuration issue, not
> software, and we are not explaining the complete NAND configuration here,
> otherwise we would have to mention OM signals too, maybe more.
Hey, actually I do think that describing which hardware configurations
the software performs is a Good Thing (TM).
I do NOT want to have to look up each and every bit in the reference
manual when reading the source code.
Meaningful names are a good thing, also - much better than cryptic
numbers everywhere.
> > In OneNAND board, it should be set as 0x1002
>
> Sorry, do not understand what "it." If you mean the MEM_SYS_CFG then I
> also don't understand this. As I quoted from the datasheet above, bit 1
> set to 0 (0 << 1) is for _both_ - NAND or OneNAND. You suggest to set it
> to 1, which is SROMC CS2. And (1 << 12) is the data bus width, which also
> doesn't seem to be directly related to the NAND / OneNAND selection. Or
> did you mean another register?
Get rid of these magic numbers. Use readable constants everywhere!
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: wd at denx.de
Suffocating together ... would create heroic camaraderie.
-- Khan Noonian Singh, "Space Seed", stardate 3142.8
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:48 ` Kyungmin Park
@ 2008-10-23 8:56 ` Guennadi Liakhovetski
0 siblings, 0 replies; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 8:56 UTC (permalink / raw)
To: u-boot
On Thu, 23 Oct 2008, Kyungmin Park wrote:
> >> S3C64XX_MEM_SYS_CFG_NAND 0x0008
> >> S3C64XX_MEM_SYS_CFG_ONENAND 0x1000
> >
> > ????? I asked above what the bus width has to do with OneNAND selection,
> > you didn't reply.
>
> OneNAND has always 16-bit butwidth. there's no exception.
Ok, thanks, but I wouldn't call the macro ONENAND, but rather 16BIT, but
that's just IMHO.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 8:49 ` Wolfgang Denk
@ 2008-10-23 9:00 ` Guennadi Liakhovetski
2008-10-23 9:16 ` Wolfgang Denk
0 siblings, 1 reply; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 9:00 UTC (permalink / raw)
To: u-boot
On Thu, 23 Oct 2008, Wolfgang Denk wrote:
> Dear Guennadi Liakhovetski,
>
> In message <Pine.LNX.4.64.0810230954380.4977@axis700.grange> you wrote:
> >
> > > > + /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */
> > >
> > > Right, and also add OneNAND & NFCON is depends on XNANDSEL.
> >
> > In the datasheet this signal is called XSELNAND. And I don't think we have
> > to quote this in the comment. This is a hardware configuration issue, not
> > software, and we are not explaining the complete NAND configuration here,
> > otherwise we would have to mention OM signals too, maybe more.
>
> Hey, actually I do think that describing which hardware configurations
> the software performs is a Good Thing (TM).
Exactly, "which hardware configurations the software performs", XSELNAND
is not performed in software, this is just a pin you wire high or low on
your board. That's why I said we might not need to comment upon it. That's
how I interpreted the datasheet anyway.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 9:00 ` Guennadi Liakhovetski
@ 2008-10-23 9:16 ` Wolfgang Denk
2008-10-23 9:23 ` Guennadi Liakhovetski
0 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2008-10-23 9:16 UTC (permalink / raw)
To: u-boot
Dear Guennadi Liakhovetski,
In message <Pine.LNX.4.64.0810231057420.4977@axis700.grange> you wrote:
>
> > Hey, actually I do think that describing which hardware configurations
> > the software performs is a Good Thing (TM).
>
> Exactly, "which hardware configurations the software performs", XSELNAND
> is not performed in software, this is just a pin you wire high or low on
> your board. That's why I said we might not need to comment upon it. That's
> how I interpreted the datasheet anyway.
I don;t think how this would make a difference. Even if the signal is
defined by the hardware dsign - the very moment I am referencing this
signal in any piece of software I should explain it so the reader of
the code understands what I'm doing.
Using meaningful names instead of magic numbers is a minimum to do.
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: wd at denx.de
"It ain't so much the things we don't know that get us in trouble.
It's the things we know that ain't so." - Artemus Ward aka Charles
Farrar Brown
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx
2008-10-23 9:16 ` Wolfgang Denk
@ 2008-10-23 9:23 ` Guennadi Liakhovetski
0 siblings, 0 replies; 14+ messages in thread
From: Guennadi Liakhovetski @ 2008-10-23 9:23 UTC (permalink / raw)
To: u-boot
On Thu, 23 Oct 2008, Wolfgang Denk wrote:
> Dear Guennadi Liakhovetski,
>
> In message <Pine.LNX.4.64.0810231057420.4977@axis700.grange> you wrote:
> >
> > > Hey, actually I do think that describing which hardware configurations
> > > the software performs is a Good Thing (TM).
> >
> > Exactly, "which hardware configurations the software performs", XSELNAND
> > is not performed in software, this is just a pin you wire high or low on
> > your board. That's why I said we might not need to comment upon it. That's
> > how I interpreted the datasheet anyway.
>
> I don;t think how this would make a difference. Even if the signal is
> defined by the hardware dsign - the very moment I am referencing this
> signal in any piece of software I should explain it so the reader of
> the code understands what I'm doing.
I don't think this pin is referenced in software, at least not in this
register, AFAICS. The only reason why Kyungmin mentioned it and why it is
mentioned in the datasheet in the description of this register, is that
this pin defines the selection between NAND and OneNAND - along with some
other pins (OM[4:1] or some such). So, you cannot set this pin in
software, you cannot read this pin in software (I think), maybe you have
to switch a jumper when selecting another configuration, but that again
has little to do with software, IMHO.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2008-10-23 9:23 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-22 5:16 [U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx Kyungmin Park
2008-10-22 12:47 ` Jean-Christophe PLAGNIOL-VILLARD
2008-10-23 7:39 ` Guennadi Liakhovetski
2008-10-23 7:47 ` Kyungmin Park
2008-10-23 8:00 ` Guennadi Liakhovetski
2008-10-23 8:34 ` Kyungmin Park
2008-10-23 8:42 ` Guennadi Liakhovetski
2008-10-23 8:48 ` Kyungmin Park
2008-10-23 8:56 ` Guennadi Liakhovetski
2008-10-23 8:49 ` Wolfgang Denk
2008-10-23 9:00 ` Guennadi Liakhovetski
2008-10-23 9:16 ` Wolfgang Denk
2008-10-23 9:23 ` Guennadi Liakhovetski
2008-10-23 8:16 ` Wolfgang Denk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox