* [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly @ 2009-03-09 16:21 ` Mikhail Zolotaryov 0 siblings, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-09 16:21 UTC (permalink / raw) To: linux-kernel; +Cc: linuxppc-dev, jwboyer [-- Attachment #1: Type: text/plain, Size: 434 bytes --] Hi, according to the PPC440EPx Embedded Processor User Manual (rev 1.14) DDR0_14[REDUC] bit has the following meaning: REDUC - Enable the half data path feature of the controller 0 = Standard operation using full 64/72-bit memory bus 1 = Memory data path width is 32/40 bits However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus by the kernel code. My understanding is this is not correct - patch attached. [-- Attachment #2: patch-ibm4xx_denali_fixup_memsize.patch --] [-- Type: text/plain, Size: 412 bytes --] --- linux.orig/arch/powerpc/boot/4xx.c 2009-03-09 17:55:01.000000000 +0200 +++ linux/arch/powerpc/boot/4xx.c 2009-03-09 17:58:07.000000000 +0200 @@ -193,9 +193,9 @@ val = SDRAM0_READ(DDR0_14); if (DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT)) - dpath = 8; /* 64 bits */ - else dpath = 4; /* 32 bits */ + else + dpath = 8; /* 64 bits */ /* get address pins (rows) */ val = SDRAM0_READ(DDR0_42); ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly @ 2009-03-09 16:21 ` Mikhail Zolotaryov 0 siblings, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-09 16:21 UTC (permalink / raw) To: linux-kernel; +Cc: linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 434 bytes --] Hi, according to the PPC440EPx Embedded Processor User Manual (rev 1.14) DDR0_14[REDUC] bit has the following meaning: REDUC - Enable the half data path feature of the controller 0 = Standard operation using full 64/72-bit memory bus 1 = Memory data path width is 32/40 bits However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus by the kernel code. My understanding is this is not correct - patch attached. [-- Attachment #2: patch-ibm4xx_denali_fixup_memsize.patch --] [-- Type: text/plain, Size: 412 bytes --] --- linux.orig/arch/powerpc/boot/4xx.c 2009-03-09 17:55:01.000000000 +0200 +++ linux/arch/powerpc/boot/4xx.c 2009-03-09 17:58:07.000000000 +0200 @@ -193,9 +193,9 @@ val = SDRAM0_READ(DDR0_14); if (DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT)) - dpath = 8; /* 64 bits */ - else dpath = 4; /* 32 bits */ + else + dpath = 8; /* 64 bits */ /* get address pins (rows) */ val = SDRAM0_READ(DDR0_42); ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly 2009-03-09 16:21 ` Mikhail Zolotaryov @ 2009-03-09 17:12 ` Josh Boyer -1 siblings, 0 replies; 26+ messages in thread From: Josh Boyer @ 2009-03-09 17:12 UTC (permalink / raw) To: Mikhail Zolotaryov; +Cc: linux-kernel, linuxppc-dev On Mon, Mar 09, 2009 at 06:21:36PM +0200, Mikhail Zolotaryov wrote: > Hi, > > according to the PPC440EPx Embedded Processor User Manual (rev 1.14) > DDR0_14[REDUC] bit has the following meaning: > > REDUC - Enable the half data path feature of the controller > 0 = Standard operation using full 64/72-bit memory bus > 1 = Memory data path width is 32/40 bits > > However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus > by the kernel code. My understanding is this is not correct - patch > attached. You are missing the Signed-off-by tag that is needed. Aside from that, I'm curious if you found this just through inspection, or if you actually had a problem because of it. Your fix seems correct, yet I've had no problems with my boards at all. josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly @ 2009-03-09 17:12 ` Josh Boyer 0 siblings, 0 replies; 26+ messages in thread From: Josh Boyer @ 2009-03-09 17:12 UTC (permalink / raw) To: Mikhail Zolotaryov; +Cc: linuxppc-dev, linux-kernel On Mon, Mar 09, 2009 at 06:21:36PM +0200, Mikhail Zolotaryov wrote: > Hi, > > according to the PPC440EPx Embedded Processor User Manual (rev 1.14) > DDR0_14[REDUC] bit has the following meaning: > > REDUC - Enable the half data path feature of the controller > 0 = Standard operation using full 64/72-bit memory bus > 1 = Memory data path width is 32/40 bits > > However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus > by the kernel code. My understanding is this is not correct - patch > attached. You are missing the Signed-off-by tag that is needed. Aside from that, I'm curious if you found this just through inspection, or if you actually had a problem because of it. Your fix seems correct, yet I've had no problems with my boards at all. josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly 2009-03-09 17:12 ` Josh Boyer @ 2009-03-09 21:17 ` Mikhail Zolotaryov -1 siblings, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-09 21:17 UTC (permalink / raw) To: Josh Boyer; +Cc: linux-kernel, linuxppc-dev >> However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus >> by the kernel code. My understanding is this is not correct - patch >> attached. > > You are missing the Signed-off-by tag that is needed. > I'm sorry. Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> > Aside from that, I'm curious if you found this just through inspection, or > if you actually had a problem because of it. Your fix seems correct, yet > I've had no problems with my boards at all. > I'm actually working on custom PowerPC 440EPX board development here and have found the problem: without applying changes described, Linux detects memory size incorrectly and is unable to start. The rest of memory size computation process seems to be correct. I have checked Sequoia evaluation board with U-Boot 2009.1 and confirm that original kernel code reported memory size correctly. So, for my understanding, if computation algorithm was wrong but result was correct - the problem is source data i.e. initial DDR configuration parameters loaded by U-Boot (please don't give a damn that memory size is always reported correctly by U-Boot - it's hard-coded constant there). I have checked Sequoia board schematics (DES0211_11_SCH_11.pdf, page 5, unit U1D) and noted that BankSel#1 is not connected, while bootloader memory configuration is (board/amcc/sequoia/sdram.c): mtsdram(DDR0_10, 0x00000300); i.e. both Chip Selects used. If I change this line to: mtsdram(DDR0_10, 0x00000100); memory is accessible, patched kernel detects memory size correctly and kernel seems to be working well. Please check my considerations, possibly it may be necessary to request additional information from board manufacturer. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly @ 2009-03-09 21:17 ` Mikhail Zolotaryov 0 siblings, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-09 21:17 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev, linux-kernel >> However, "1" is decoded as 64 bit (8 byte), "0" - as 32 bit (4 byte) bus >> by the kernel code. My understanding is this is not correct - patch >> attached. > > You are missing the Signed-off-by tag that is needed. > I'm sorry. Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> > Aside from that, I'm curious if you found this just through inspection, or > if you actually had a problem because of it. Your fix seems correct, yet > I've had no problems with my boards at all. > I'm actually working on custom PowerPC 440EPX board development here and have found the problem: without applying changes described, Linux detects memory size incorrectly and is unable to start. The rest of memory size computation process seems to be correct. I have checked Sequoia evaluation board with U-Boot 2009.1 and confirm that original kernel code reported memory size correctly. So, for my understanding, if computation algorithm was wrong but result was correct - the problem is source data i.e. initial DDR configuration parameters loaded by U-Boot (please don't give a damn that memory size is always reported correctly by U-Boot - it's hard-coded constant there). I have checked Sequoia board schematics (DES0211_11_SCH_11.pdf, page 5, unit U1D) and noted that BankSel#1 is not connected, while bootloader memory configuration is (board/amcc/sequoia/sdram.c): mtsdram(DDR0_10, 0x00000300); i.e. both Chip Selects used. If I change this line to: mtsdram(DDR0_10, 0x00000100); memory is accessible, patched kernel detects memory size correctly and kernel seems to be working well. Please check my considerations, possibly it may be necessary to request additional information from board manufacturer. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-09 21:17 ` Mikhail Zolotaryov (?) @ 2009-03-10 19:50 ` Valentine Barshak 2009-03-10 20:57 ` Mikhail Zolotaryov 2009-03-11 10:37 ` Josh Boyer -1 siblings, 2 replies; 26+ messages in thread From: Valentine Barshak @ 2009-03-10 19:50 UTC (permalink / raw) To: linuxppc-dev; +Cc: lebon I was just going to submit a patch for that too. Indeed, the denali_fixup_memsize() miscalculated a couple of address field widths. We were lucky to eventually get the right result, because the effect of the first error was killed by the other one. According to the AMCC 440EPX/GRX user manual, the Chip Select width is always fixed at 1 bit no matter what is actually read from register DDR_10. The workaround is to use a predefined chipselect value for 440EPx/GRx. Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path. If REDUC = 0, full data path of 64 bits is used. Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com> Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> --- a/arch/powerpc/boot/4xx.c 2008-04-26 02:18:34.000000000 +0400 +++ b/arch/powerpc/boot/4xx.c 2008-10-26 01:40:27.000000000 +0400 @@ -173,15 +173,20 @@ void ibm4xx_denali_fixup_memsize(void) max_col = DDR_GET_VAL(val, DDR_MAX_COL_REG, DDR_MAX_COL_REG_SHIFT); max_row = DDR_GET_VAL(val, DDR_MAX_ROW_REG, DDR_MAX_ROW_REG_SHIFT); - /* get CS value */ - val = SDRAM0_READ(DDR0_10); - - val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); - cs = 0; - while (val) { - if (val & 0x1) - cs++; - val = val >> 1; + /* 440EPx/GRx chipselect always fixed at 1 bit */ + if ((mfpvr() & 0xf0000ff0) == 0x200008D0) + cs = 1; + else { + /* get CS value */ + val = SDRAM0_READ(DDR0_10); + val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); + + cs = 0; + while (val) { + if (val & 0x1) + cs++; + val = val >> 1; + } } if (!cs) @@ -192,7 +197,7 @@ void ibm4xx_denali_fixup_memsize(void) /* get data path bytes */ val = SDRAM0_READ(DDR0_14); - if (DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT)) + if (!DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT)) dpath = 8; /* 64 bits */ else dpath = 4; /* 32 bits */ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-10 19:50 ` [PATCH] PowerPC 440EPx/GRx fix memory size calculation Valentine Barshak @ 2009-03-10 20:57 ` Mikhail Zolotaryov 2009-03-11 1:40 ` Valentine 2009-03-11 10:37 ` Josh Boyer 1 sibling, 1 reply; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-10 20:57 UTC (permalink / raw) To: linuxppc-dev Valentine Barshak wrote: > According to the AMCC 440EPX/GRX user manual, > the Chip Select width is always fixed at 1 bit no matter > what is actually read from register DDR_10. Well, from my point of view original kernel code is correct in this part. Adding one bit into memory address means multiplying memory size by 2 i.e. cs=2. The question is: is Chip Select bit used in memory address. ChipSelect input of memory chip enables or disabled it, so if we have only one BankSel installed/connected (DDR0_10[22:23] is 01 or 10) there's no need to use Chip Select bit in an address. On the contrary, if both BankSel lines are connected (DDR0_10[22:23] is 11), to let memory controller know which memory rank to use, Chip Select bit is added into memory address. (and yes, if DDR0_10[22:23] is 00 - no ranks installed, memory size is 0, cs=0) Original kernel code use exactly the same logic as I described above. Please suggest if it's wrong. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-10 20:57 ` Mikhail Zolotaryov @ 2009-03-11 1:40 ` Valentine 2009-03-11 2:26 ` Benjamin Herrenschmidt 2009-03-11 8:29 ` Mikhail Zolotaryov 0 siblings, 2 replies; 26+ messages in thread From: Valentine @ 2009-03-11 1:40 UTC (permalink / raw) To: Mikhail Zolotaryov; +Cc: linuxppc-dev Mikhail Zolotaryov wrote: > Valentine Barshak wrote: >> According to the AMCC 440EPX/GRX user manual, >> the Chip Select width is always fixed at 1 bit no matter >> what is actually read from register DDR_10. > > Well, from my point of view original kernel code is correct in this part. > > Adding one bit into memory address means multiplying memory size by 2 > i.e. cs=2. The question is: is Chip Select bit used in memory address. > ChipSelect input of memory chip enables or disabled it, so if we have > only one BankSel installed/connected (DDR0_10[22:23] is 01 or 10) > there's no need to use Chip Select bit in an address. On the contrary, > if both BankSel lines are connected (DDR0_10[22:23] is 11), to let > memory controller know which memory rank to use, Chip Select bit is > added into memory address. (and yes, if DDR0_10[22:23] is 00 - no > ranks installed, memory size is 0, cs=0) Yes, you could phrase it that way. According to the PPC440EPx manual, the total memory size is calculated based on the following formula: memsize = cs * (1 << (col+row)) * bank * dpath; So, if both chipselects are used, we add an extra bit to the memory address to distinguish between these chipselects. There's nothing wrong with this part of the code. The problem is that the controller is hardwired to use only one chipselect, even if both are enabled in the DDR0_10 on PPC440EPx/GRx processors. So, the patch provides a workaround to always use single cs for 440EPx/GRx (use predefined value instead of reading DDR0_10). > > Original kernel code use exactly the same logic as I described above. > Please suggest if it's wrong. Thanks, Valentine. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 1:40 ` Valentine @ 2009-03-11 2:26 ` Benjamin Herrenschmidt 2009-03-11 8:24 ` Mikhail Zolotaryov 2009-03-11 8:29 ` Mikhail Zolotaryov 1 sibling, 1 reply; 26+ messages in thread From: Benjamin Herrenschmidt @ 2009-03-11 2:26 UTC (permalink / raw) To: Valentine; +Cc: linuxppc-dev, Mikhail Zolotaryov > Yes, you could phrase it that way. According to the PPC440EPx manual, > the total memory size is calculated based on the following formula: > memsize = cs * (1 << (col+row)) * bank * dpath; > So, if both chipselects are used, we add an extra bit to the memory > address to distinguish between these chipselects. > There's nothing wrong with this part of the code. > The problem is that the controller is hardwired to use only one > chipselect, even if both are enabled in the DDR0_10 on PPC440EPx/GRx > processors. > So, the patch provides a workaround to always use single cs for > 440EPx/GRx (use predefined value instead of reading DDR0_10). Mikhail, can you verify that Valentine's patch works for you ? Cheers, Ben. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 2:26 ` Benjamin Herrenschmidt @ 2009-03-11 8:24 ` Mikhail Zolotaryov 0 siblings, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-11 8:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev Benjamin Herrenschmidt wrote: >> The problem is that the controller is hardwired to use only one >> chipselect, even if both are enabled in the DDR0_10 on PPC440EPx/GRx >> processors > > Mikhail, can you verify that Valentine's patch works for you ? Ben, unfortunately on my board(s) I don't have both bits enabled in DDR0_10 i.e. I'll have cs=1 calculated even by original Linux code. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 1:40 ` Valentine 2009-03-11 2:26 ` Benjamin Herrenschmidt @ 2009-03-11 8:29 ` Mikhail Zolotaryov 1 sibling, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-11 8:29 UTC (permalink / raw) To: Valentine; +Cc: linuxppc-dev Valentine wrote: > The problem is that the controller is hardwired to use only one > chipselect, even if both are enabled in the DDR0_10 on PPC440EPx/GRx > processors. > It's new information for me. Is this problem described by some ERRATA or manual, could you please point me to the document (and page) ? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-10 19:50 ` [PATCH] PowerPC 440EPx/GRx fix memory size calculation Valentine Barshak 2009-03-10 20:57 ` Mikhail Zolotaryov @ 2009-03-11 10:37 ` Josh Boyer 2009-03-11 19:06 ` Valentine Barshak 1 sibling, 1 reply; 26+ messages in thread From: Josh Boyer @ 2009-03-11 10:37 UTC (permalink / raw) To: Valentine Barshak; +Cc: linuxppc-dev, lebon On Tue, Mar 10, 2009 at 10:50:13PM +0300, Valentine Barshak wrote: >I was just going to submit a patch for that too. >Indeed, the denali_fixup_memsize() miscalculated a couple of address >field widths. We were lucky to eventually get the right result, >because the effect of the first error was killed by the other one. >According to the AMCC 440EPX/GRX user manual, >the Chip Select width is always fixed at 1 bit no matter >what is actually read from register DDR_10. >The workaround is to use a predefined chipselect value for 440EPx/GRx. >Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path. >If REDUC = 0, full data path of 64 bits is used. > >Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com> >Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> I've been looking over this one a bit more. At the moment, I'm inclined to queue this up in my -next branch. I would like to see if Mikhail could test it though, and have Valentine answer the question in the hard wired part. josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 10:37 ` Josh Boyer @ 2009-03-11 19:06 ` Valentine Barshak 2009-03-11 21:57 ` Josh Boyer 2009-03-12 6:02 ` Stefan Roese 0 siblings, 2 replies; 26+ messages in thread From: Valentine Barshak @ 2009-03-11 19:06 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev, Stefan Roese, lebon Josh Boyer wrote: > On Tue, Mar 10, 2009 at 10:50:13PM +0300, Valentine Barshak wrote: >> I was just going to submit a patch for that too. >> Indeed, the denali_fixup_memsize() miscalculated a couple of address >> field widths. We were lucky to eventually get the right result, >> because the effect of the first error was killed by the other one. >> According to the AMCC 440EPX/GRX user manual, >> the Chip Select width is always fixed at 1 bit no matter >> what is actually read from register DDR_10. >> The workaround is to use a predefined chipselect value for 440EPx/GRx. >> Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path. >> If REDUC = 0, full data path of 64 bits is used. >> >> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com> >> Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> > > I've been looking over this one a bit more. At the moment, I'm inclined > to queue this up in my -next branch. I would like to see if Mikhail > could test it though, and have Valentine answer the question in the hard > wired part. I've been looking at the docs once again and actually I couldn't find an explanation there. And I don't have that e-mail from AMCC support that I got a while back regarding the issue anymore. There might have been some misunderstanding. The docs (PPC440EPX UM 19.2 Device Address Mapping) say that the chip select field width is always fixed at one bit, but this doesn't actually mean that there's always one chip select used. The patch works fine on Sequoia and another Sequoia-like board with 1GB RAM installed, but it might not work with 2GB RAM. I've tried to play with DDR0_10 settings and Sequoia works fine regardless of what's actually written to DDR0_10. So, probably the best way would be to fix that in u-boot amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead of mtsdram(DDR0_10, 0x00000300); Sorry, for confusion, but after reviewing the docs, I think that only REDUC interpretation has to be fixed. The chips select part should be fixed in u-boot sdram code for Sequoia as was originally proposed by Mikhail. Stefan, could you please take a look? Thanks, Valentine. > > josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 19:06 ` Valentine Barshak @ 2009-03-11 21:57 ` Josh Boyer 2009-03-11 22:08 ` Valentine 2009-03-12 6:02 ` Stefan Roese 1 sibling, 1 reply; 26+ messages in thread From: Josh Boyer @ 2009-03-11 21:57 UTC (permalink / raw) To: Valentine Barshak; +Cc: linuxppc-dev, Stefan Roese, lebon On Wed, Mar 11, 2009 at 10:06:11PM +0300, Valentine Barshak wrote: > Josh Boyer wrote: >> On Tue, Mar 10, 2009 at 10:50:13PM +0300, Valentine Barshak wrote: >>> I was just going to submit a patch for that too. >>> Indeed, the denali_fixup_memsize() miscalculated a couple of address >>> field widths. We were lucky to eventually get the right result, >>> because the effect of the first error was killed by the other one. >>> According to the AMCC 440EPX/GRX user manual, >>> the Chip Select width is always fixed at 1 bit no matter >>> what is actually read from register DDR_10. >>> The workaround is to use a predefined chipselect value for 440EPx/GRx. >>> Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path. >>> If REDUC = 0, full data path of 64 bits is used. >>> >>> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com> >>> Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> >> >> I've been looking over this one a bit more. At the moment, I'm inclined >> to queue this up in my -next branch. I would like to see if Mikhail >> could test it though, and have Valentine answer the question in the hard >> wired part. > > I've been looking at the docs once again and actually I couldn't find an > explanation there. And I don't have that e-mail from AMCC support that > I got a while back regarding the issue anymore. > There might have been some misunderstanding. > The docs (PPC440EPX UM 19.2 Device Address Mapping) say that the chip > select field width is always fixed at one bit, but this doesn't actually > mean that there's always one chip select used. > The patch works fine on Sequoia and another Sequoia-like board with 1GB > RAM installed, but it might not work with 2GB RAM. I've tried to play > with DDR0_10 settings and Sequoia works fine regardless of what's > actually written to DDR0_10. > So, probably the best way would be to fix that in u-boot > amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead of > mtsdram(DDR0_10, 0x00000300); > Sorry, for confusion, but after reviewing the docs, I think that > only REDUC interpretation has to be fixed. The chips select part should > be fixed in u-boot sdram code for Sequoia as was originally proposed by > Mikhail. Ok, so we're back to using Mikhail's original patch then? josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 21:57 ` Josh Boyer @ 2009-03-11 22:08 ` Valentine 2009-03-11 23:07 ` Josh Boyer 0 siblings, 1 reply; 26+ messages in thread From: Valentine @ 2009-03-11 22:08 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev, Stefan Roese, lebon Josh Boyer wrote: > On Wed, Mar 11, 2009 at 10:06:11PM +0300, Valentine Barshak wrote: >> Josh Boyer wrote: >>> On Tue, Mar 10, 2009 at 10:50:13PM +0300, Valentine Barshak wrote: >>>> I was just going to submit a patch for that too. >>>> Indeed, the denali_fixup_memsize() miscalculated a couple of address >>>> field widths. We were lucky to eventually get the right result, >>>> because the effect of the first error was killed by the other one. >>>> According to the AMCC 440EPX/GRX user manual, >>>> the Chip Select width is always fixed at 1 bit no matter >>>> what is actually read from register DDR_10. >>>> The workaround is to use a predefined chipselect value for 440EPx/GRx. >>>> Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path. >>>> If REDUC = 0, full data path of 64 bits is used. >>>> >>>> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com> >>>> Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua> >>> I've been looking over this one a bit more. At the moment, I'm inclined >>> to queue this up in my -next branch. I would like to see if Mikhail >>> could test it though, and have Valentine answer the question in the hard >>> wired part. >> I've been looking at the docs once again and actually I couldn't find an >> explanation there. And I don't have that e-mail from AMCC support that >> I got a while back regarding the issue anymore. >> There might have been some misunderstanding. >> The docs (PPC440EPX UM 19.2 Device Address Mapping) say that the chip >> select field width is always fixed at one bit, but this doesn't actually >> mean that there's always one chip select used. >> The patch works fine on Sequoia and another Sequoia-like board with 1GB >> RAM installed, but it might not work with 2GB RAM. I've tried to play >> with DDR0_10 settings and Sequoia works fine regardless of what's >> actually written to DDR0_10. >> So, probably the best way would be to fix that in u-boot >> amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead of >> mtsdram(DDR0_10, 0x00000300); >> Sorry, for confusion, but after reviewing the docs, I think that >> only REDUC interpretation has to be fixed. The chips select part should >> be fixed in u-boot sdram code for Sequoia as was originally proposed by >> Mikhail. > > Ok, so we're back to using Mikhail's original patch then? > > josh Yes, but until u-boot is fixed this will break Sequoia/Rainier support. Thanks, Valentine. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 22:08 ` Valentine @ 2009-03-11 23:07 ` Josh Boyer 0 siblings, 0 replies; 26+ messages in thread From: Josh Boyer @ 2009-03-11 23:07 UTC (permalink / raw) To: Valentine; +Cc: linuxppc-dev, Stefan Roese, lebon On Thu, Mar 12, 2009 at 01:08:59AM +0300, Valentine wrote: >>> So, probably the best way would be to fix that in u-boot >>> amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead >>> of mtsdram(DDR0_10, 0x00000300); >>> Sorry, for confusion, but after reviewing the docs, I think that >>> only REDUC interpretation has to be fixed. The chips select part >>> should be fixed in u-boot sdram code for Sequoia as was originally >>> proposed by Mikhail. >> >> Ok, so we're back to using Mikhail's original patch then? >> >> josh > > Yes, but until u-boot is fixed this will break Sequoia/Rainier support. Well, that's sort of a problem. The wrapper will have to deal with both a fixed and unfixed u-boot because not everyone will update their u-boot with the fix. So we need a patch for the wrapper that works in all cases. josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-11 19:06 ` Valentine Barshak 2009-03-11 21:57 ` Josh Boyer @ 2009-03-12 6:02 ` Stefan Roese 2009-03-12 7:32 ` Benjamin Herrenschmidt 2009-03-13 23:01 ` Feng Kan 1 sibling, 2 replies; 26+ messages in thread From: Stefan Roese @ 2009-03-12 6:02 UTC (permalink / raw) To: linuxppc-dev; +Cc: lebon On Wednesday 11 March 2009, Valentine Barshak wrote: > I've been looking at the docs once again and actually I couldn't find an > explanation there. And I don't have that e-mail from AMCC support > that I got a while back regarding the issue anymore. > There might have been some misunderstanding. > The docs (PPC440EPX UM 19.2 Device Address Mapping) say that the chip > select field width is always fixed at one bit, but this doesn't actually > mean that there's always one chip select used. > The patch works fine on Sequoia and another Sequoia-like board with 1GB > RAM installed, but it might not work with 2GB RAM. I've tried to play > with DDR0_10 settings and Sequoia works fine regardless of what's > actually written to DDR0_10. > So, probably the best way would be to fix that in u-boot > amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead of > mtsdram(DDR0_10, 0x00000300); > Sorry, for confusion, but after reviewing the docs, I think that > only REDUC interpretation has to be fixed. The chips select part should > be fixed in u-boot sdram code for Sequoia as was originally proposed by > Mikhail. > > Stefan, could you please take a look? I'll apply the U-Boot patch today. But as Josh pointed out, we should try to find a way for the bootwrapper to work in all cases. Best regards, Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 6:02 ` Stefan Roese @ 2009-03-12 7:32 ` Benjamin Herrenschmidt 2009-03-12 8:05 ` Stefan Roese 2009-03-13 23:01 ` Feng Kan 1 sibling, 1 reply; 26+ messages in thread From: Benjamin Herrenschmidt @ 2009-03-12 7:32 UTC (permalink / raw) To: Stefan Roese; +Cc: linuxppc-dev, lebon On Thu, 2009-03-12 at 07:02 +0100, Stefan Roese wrote: > > I'll apply the U-Boot patch today. But as Josh pointed out, we should > try to > find a way for the bootwrapper to work in all cases. uboot is passing some kind of bt_t to the wrapper or a full device-tree ? Ben. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 7:32 ` Benjamin Herrenschmidt @ 2009-03-12 8:05 ` Stefan Roese 2009-03-12 8:12 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 26+ messages in thread From: Stefan Roese @ 2009-03-12 8:05 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, lebon On Thursday 12 March 2009, Benjamin Herrenschmidt wrote: > On Thu, 2009-03-12 at 07:02 +0100, Stefan Roese wrote: > > I'll apply the U-Boot patch today. But as Josh pointed out, we should > > try to > > find a way for the bootwrapper to work in all cases. > > uboot is passing some kind of bt_t to the wrapper or a full > device-tree ? Both is possible. Older U-Boot versions only passed the bd_t struct to the kernel. For those U-Boot's the wrapper is needed. More recent U-Boot versions support passing a device-tree blob to the kernel. U-Boot patches the correct memory size in this blob. As a matter of fact, I never used the wrapper before. U-Boot supports passing the device-tree blob to Linux since quite some time now. Best regards, Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 8:05 ` Stefan Roese @ 2009-03-12 8:12 ` Benjamin Herrenschmidt 2009-03-12 8:24 ` Stefan Roese 0 siblings, 1 reply; 26+ messages in thread From: Benjamin Herrenschmidt @ 2009-03-12 8:12 UTC (permalink / raw) To: Stefan Roese; +Cc: linuxppc-dev, lebon On Thu, 2009-03-12 at 09:05 +0100, Stefan Roese wrote: > > Both is possible. Older U-Boot versions only passed the bd_t struct to the > kernel. For those U-Boot's the wrapper is needed. More recent U-Boot versions > support passing a device-tree blob to the kernel. U-Boot patches the correct > memory size in this blob. > > As a matter of fact, I never used the wrapper before. U-Boot supports passing > the device-tree blob to Linux since quite some time now. Yes, that's also how I use it on canyonlands... now, the wrapper could probably be used to look at the bd_t anyways, no ? Either get the mem size from there or some flag or version in there can indicate if it's been "fixed". Ben. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 8:12 ` Benjamin Herrenschmidt @ 2009-03-12 8:24 ` Stefan Roese 2009-03-12 8:45 ` Mikhail Zolotaryov 2009-03-12 10:45 ` Josh Boyer 0 siblings, 2 replies; 26+ messages in thread From: Stefan Roese @ 2009-03-12 8:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, lebon On Thursday 12 March 2009, Benjamin Herrenschmidt wrote: > On Thu, 2009-03-12 at 09:05 +0100, Stefan Roese wrote: > > Both is possible. Older U-Boot versions only passed the bd_t struct to > > the kernel. For those U-Boot's the wrapper is needed. More recent U-Boot > > versions support passing a device-tree blob to the kernel. U-Boot patches > > the correct memory size in this blob. > > > > As a matter of fact, I never used the wrapper before. U-Boot supports > > passing the device-tree blob to Linux since quite some time now. > > Yes, that's also how I use it on canyonlands... now, the wrapper could > probably be used to look at the bd_t anyways, no ? Sure. > Either get the mem > size from there or some flag or version in there can indicate if it's > been "fixed". I don't think that we have some flag and/or version information in the bd_info struct. And extending this struct doesn't sound like a good idea to me. Best regards, Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 8:24 ` Stefan Roese @ 2009-03-12 8:45 ` Mikhail Zolotaryov 2009-03-12 10:45 ` Josh Boyer 1 sibling, 0 replies; 26+ messages in thread From: Mikhail Zolotaryov @ 2009-03-12 8:45 UTC (permalink / raw) To: Stefan Roese; +Cc: linuxppc-dev Stefan Roese wrote: >> Either get the mem >> size from there or some flag or version in there can indicate if it's >> been "fixed". > > I don't think that we have some flag and/or version information in the bd_info > struct. And extending this struct doesn't sound like a good idea to me. May I suggest an easier way ? The problem we currently have is some evaluation board(s), we know them, use wrong DDR configuration parameters, so do as U-Boot does - simply hardcode memory size for these particular board(s), don't calculate, but use patched function to calculate memory size for all other boards, including variety of customers' made. To be absolutely sure, we can check board revision register - it's theoretically possible that future board revisions will have more or less memory installed. This way we can avoid U-Boot to Linux compatibility issues. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 8:24 ` Stefan Roese 2009-03-12 8:45 ` Mikhail Zolotaryov @ 2009-03-12 10:45 ` Josh Boyer 2009-03-12 11:02 ` Stefan Roese 1 sibling, 1 reply; 26+ messages in thread From: Josh Boyer @ 2009-03-12 10:45 UTC (permalink / raw) To: Stefan Roese; +Cc: lebon, linuxppc-dev On Thu, Mar 12, 2009 at 09:24:13AM +0100, Stefan Roese wrote: >On Thursday 12 March 2009, Benjamin Herrenschmidt wrote: >> On Thu, 2009-03-12 at 09:05 +0100, Stefan Roese wrote: >> > Both is possible. Older U-Boot versions only passed the bd_t struct to >> > the kernel. For those U-Boot's the wrapper is needed. More recent U-Boot >> > versions support passing a device-tree blob to the kernel. U-Boot patches >> > the correct memory size in this blob. >> > >> > As a matter of fact, I never used the wrapper before. U-Boot supports >> > passing the device-tree blob to Linux since quite some time now. >> >> Yes, that's also how I use it on canyonlands... now, the wrapper could >> probably be used to look at the bd_t anyways, no ? > >Sure. Do newer U-Boot versions pass both the dtb and the bd_t? If not, the wrapper would have to look for one, then the other and not get confused. >> Either get the mem >> size from there or some flag or version in there can indicate if it's >> been "fixed". > >I don't think that we have some flag and/or version information in the bd_info >struct. And extending this struct doesn't sound like a good idea to me. Yeah, we've already had some issues pop up in the past where the bd_t wasn't correct for a board in the U-Boot version that shipped with it (like the acadia boards). There's not much that can be done to fix it. josh ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 10:45 ` Josh Boyer @ 2009-03-12 11:02 ` Stefan Roese 0 siblings, 0 replies; 26+ messages in thread From: Stefan Roese @ 2009-03-12 11:02 UTC (permalink / raw) To: Josh Boyer; +Cc: lebon, linuxppc-dev On Thursday 12 March 2009, Josh Boyer wrote: > >> Yes, that's also how I use it on canyonlands... now, the wrapper could > >> probably be used to look at the bd_t anyways, no ? > > > >Sure. > > Do newer U-Boot versions pass both the dtb and the bd_t? Both is possible. The user can choose by using different boot commands (with or without device tree blob). When using the wrapper, the boot command has to be without the device tree and therefor the bd_t is passed to the kernel. Best regards, Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation 2009-03-12 6:02 ` Stefan Roese 2009-03-12 7:32 ` Benjamin Herrenschmidt @ 2009-03-13 23:01 ` Feng Kan 1 sibling, 0 replies; 26+ messages in thread From: Feng Kan @ 2009-03-13 23:01 UTC (permalink / raw) To: Stefan Roese; +Cc: linuxppc-dev, lebon Hi Guys: Sequoia uses on board discrete memory with one rank. So one chip select would be fine. Turning on both won't matter, since the other cs is never used. Feng Kan Stefan Roese wrote: > On Wednesday 11 March 2009, Valentine Barshak wrote: > >> I've been looking at the docs once again and actually I couldn't find an >> explanation there. And I don't have that e-mail from AMCC support >> that I got a while back regarding the issue anymore. >> There might have been some misunderstanding. >> The docs (PPC440EPX UM 19.2 Device Address Mapping) say that the chip >> select field width is always fixed at one bit, but this doesn't actually >> mean that there's always one chip select used. >> The patch works fine on Sequoia and another Sequoia-like board with 1GB >> RAM installed, but it might not work with 2GB RAM. I've tried to play >> with DDR0_10 settings and Sequoia works fine regardless of what's >> actually written to DDR0_10. >> So, probably the best way would be to fix that in u-boot >> amcc/sequoia/sdram.c by doing mtsdram(DDR0_10, 0x00000100); instead of >> mtsdram(DDR0_10, 0x00000300); >> Sorry, for confusion, but after reviewing the docs, I think that >> only REDUC interpretation has to be fixed. The chips select part should >> be fixed in u-boot sdram code for Sequoia as was originally proposed by >> Mikhail. >> >> Stefan, could you please take a look? >> > > I'll apply the U-Boot patch today. But as Josh pointed out, we should try to > find a way for the bootwrapper to work in all cases. > > Best regards, > Stefan > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev > > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2009-03-13 23:12 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-03-09 16:21 [PATCH] powerpc 4xx: DDR0_14[REDUC] decoded incorrectly Mikhail Zolotaryov 2009-03-09 16:21 ` Mikhail Zolotaryov 2009-03-09 17:12 ` Josh Boyer 2009-03-09 17:12 ` Josh Boyer 2009-03-09 21:17 ` Mikhail Zolotaryov 2009-03-09 21:17 ` Mikhail Zolotaryov 2009-03-10 19:50 ` [PATCH] PowerPC 440EPx/GRx fix memory size calculation Valentine Barshak 2009-03-10 20:57 ` Mikhail Zolotaryov 2009-03-11 1:40 ` Valentine 2009-03-11 2:26 ` Benjamin Herrenschmidt 2009-03-11 8:24 ` Mikhail Zolotaryov 2009-03-11 8:29 ` Mikhail Zolotaryov 2009-03-11 10:37 ` Josh Boyer 2009-03-11 19:06 ` Valentine Barshak 2009-03-11 21:57 ` Josh Boyer 2009-03-11 22:08 ` Valentine 2009-03-11 23:07 ` Josh Boyer 2009-03-12 6:02 ` Stefan Roese 2009-03-12 7:32 ` Benjamin Herrenschmidt 2009-03-12 8:05 ` Stefan Roese 2009-03-12 8:12 ` Benjamin Herrenschmidt 2009-03-12 8:24 ` Stefan Roese 2009-03-12 8:45 ` Mikhail Zolotaryov 2009-03-12 10:45 ` Josh Boyer 2009-03-12 11:02 ` Stefan Roese 2009-03-13 23:01 ` Feng Kan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.