t1040 IFC flash driver Extended Chip Select

PATCH Daniel Walker 2 participants 18 messages Jul 6, 2016 – Jul 11, 2016

Subsystems (2)

Device tree bindings Memory controller drivers

lore

Share & tools

lore

Daniel Walker

author

2016-07-06 20:33 UTC #0 lore raw

Hi,

We are using the t1040 platform, and we have found that we need to populate this register. In the Technical Reference Manual it's description is section 24.3.2. This option appears in the driver, but it doesn't appears to be used anyplace.

We we're considered adding something to the device tree to allow populating this value, but I'm wondering if any of you have specific considerations on how this is done. Or maybe it's not needed at all, and we're just missing something.

(not a good patch, just an example.)

Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt +2 -0

index 89427b0..b506001 100644
@@ -24,6 +24,8 @@ Properties:
 - ranges : Each range corresponds to a single chipselect, and covers
            the entire access window as configured.
 
+- cspr_ext : This value sets the extended chip select for all banks.
+
 Child device nodes describe the devices connected to IFC such as NOR (e.g.
 cfi-flash) and NAND (fsl,ifc-nand). There might be board specific devices
 like FPGAs, CPLDs, etc.

Daniel

Scott Wood

2016-07-07 05:30 UTC #1 lore raw

On 07/06/2016 03:23 PM, Daniel Walker wrote:=0A=

Quoted 5 lines

Hi,=0A=
=0A=
We are using the t1040 platform, and we have found that we need to =0A= populate this register. In the Technical Reference Manual it's =0A= description is section 24.3.2. This option appears in the driver, but it =

=0A=

Quoted 1 line

doesn't appears to be used anyplace.=0A=

=0A=
I'm not sure what you mean by "in the driver". U-boot sets these registers= .=0A=
=0A=

Quoted 3 lines

We we're considered adding something to the device tree to allow =0A= populating this value, but I'm wondering if any of you have specific =0A= considerations on how this is done. Or maybe it's not needed at all, and =

=0A=

Quoted 13 lines

we're just missing something.=0A=
=0A=
(not a good patch, just an example.)=0A=
=0A=
(quoted diff) +0 -0
a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt =0A= b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=0A= index 89427b0..b506001 100644=0A=
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=0A= +0 -0
@@ -24,6 +24,8 @@ Properties:=0A=
  - ranges : Each range corresponds to a single chipselect, and covers=0A=
             the entire access window as configured.=0A=

Quoted 2 lines

=0A=
+- cspr_ext : This value sets the extended chip select for all banks.=0A=

=0A=
I see no reason to put this here. Boot software should be setting the=0A= chipselect registers, and if for some reason you don't want to do that,=0A= you could just translate the address through ranges to find the value to=0A= write. Why would you have the binding assume it's the same for all banks?= =0A=
=0A=
The information that is missing from the device tree, that currently=0A= must come from boot software programming the registers, is the various=0A= attributes that get programmed in CSPR/CSOR.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 15:58 UTC #2 lore raw

Quoted 8 lines · Scott Wood

Quoted 6 lines · Daniel Walker
Hi,
We are using the t1040 platform, and we have found that we need to populate this register. In the Technical Reference Manual it's description is section 24.3.2. This option appears in the driver, but it doesn't appears to be used anyplace.
I'm not sure what you mean by "in the driver". U-boot sets these registers.

My hardware doesn't use u-boot , and it doesn't set these values. However, there are other reasons to have this. For example, you use u-boot but it's got a defect which sets the values incorrectly. You may not be able to update your bootloader on a shipped product.

Quoted 18 lines

Quoted 18 lines
We we're considered adding something to the device tree to allow populating this value, but I'm wondering if any of you have specific considerations on how this is done. Or maybe it's not needed at all, and we're just missing something.
(not a good patch, just an example.)
diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt index 89427b0..b506001 100644
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt +0 -0
@@ -24,6 +24,8 @@ Properties:
   - ranges : Each range corresponds to a single chipselect, and covers
              the entire access window as configured.
 

Quoted 5 lines

Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt Properties: · line 27
Quoted 1 line
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt +1 -0
+- cspr_ext : This value sets the extended chip select for all banks.
I see no reason to put this here. Boot software should be setting the chipselect registers, and if for some reason you don't want to do that, you could just translate the address through ranges to find the value to write. Why would you have the binding assume it's the same for all banks?

It was only an example, to start the conversation. I don't understand what you mean by translating the address thru ranges?

Quoted 5 lines

The information that is missing from the device tree, that currently must come from boot software programming the registers, is the various attributes that get programmed in CSPR/CSOR.

Like I said mine doesn't do this, so it's required that it be set in an alternative way. The only alternative we have currently is adding some code to manually set the values but it's not ideal (and not upstreamable).

Daniel

Scott Wood

2016-07-07 20:55 UTC #3 lore raw

On 07/07/2016 10:48 AM, Daniel Walker wrote:=0A=

Quoted 7 lines

On 07/06/2016 05:57 PM, Scott Wood wrote:=0A=
Quoted 6 lines
On 07/06/2016 03:23 PM, Daniel Walker wrote:=0A=
Quoted 5 lines
Hi,=0A=
=0A=
We are using the t1040 platform, and we have found that we need to=0A= populate this register. In the Technical Reference Manual it's=0A= description is section 24.3.2. This option appears in the driver, but i=

t=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 1 line
doesn't appears to be used anyplace.=0A=
I'm not sure what you mean by "in the driver". U-boot sets these regist=

ers.=0A=

Quoted 4 lines

=0A=
My hardware doesn't use u-boot , and it doesn't set these values. =0A= However, there are other reasons to have this. For example, you use =0A= u-boot but it's got a defect which sets the values incorrectly. You may =

=0A=

Quoted 1 line

not be able to update your bootloader on a shipped product.=0A=

=0A=
How did you get to the point of shipping a product with this wrong? Did=0A= previous software versions not use IFC?=0A=
=0A=
We could have the Linux driver initialize things, but as I said, the=0A= missing piece from the device tree isn't this register, it's the attributes= .=0A=
=0A=
What else does/doesn't your loader do? Does it set up a LAW that covers=0A= IFC? You only put this one register here -- does your loader set up=0A= CSPR/CSOR? If it's just this one register that was overlooked, and you=0A= can't update the bootloader, simplest may be to just have your board's=0A= platform code write it.=0A=
=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
We we're considered adding something to the device tree to allow=0A= populating this value, but I'm wondering if any of you have specific=0A= considerations on how this is done. Or maybe it's not needed at all, an=

d=0A=

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
we're just missing something.=0A=
=0A=
(not a good patch, just an example.)=0A=
=0A=
diff --git=0A= a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=0A= b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=0A= index 89427b0..b506001 100644=0A=
--- a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
+++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt=

=0A=

Quoted 2 lines

Quoted 2 lines
Review Properties:=0A= · line 24
Quoted 2 lines
(quoted diff) +0 -0
@@ -24,6 +24,8 @@ Properties:=0A=
   - ranges : Each range corresponds to a single chipselect, and covers=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
(quoted diff) +0 -0
              the entire access window as configured.=0A=
=0A=
+- cspr_ext : This value sets the extended chip select for all banks.=

=0A=

Quoted 1 line

Quoted 1 line
I see no reason to put this here. Boot software should be setting the=

=0A=

Quoted 1 line

Quoted 1 line
chipselect registers, and if for some reason you don't want to do that,=

=0A=

Quoted 1 line

Quoted 1 line
you could just translate the address through ranges to find the value to=

=0A=

Quoted 1 line

Quoted 1 line
write. Why would you have the binding assume it's the same for all bank=

s?=0A=

Quoted 3 lines

=0A=
It was only an example, to start the conversation. I don't understand =0A= what you mean by translating the address thru ranges?=0A=

=0A=
of_address_to_resource()=0A=
=0A=

Quoted 2 lines

Quoted 2 lines
The information that is missing from the device tree, that currently=0A= must come from boot software programming the registers, is the various=

=0A=

Quoted 4 lines

Quoted 2 lines
attributes that get programmed in CSPR/CSOR.=0A=
=0A=
=0A=
Like I said mine doesn't do this, so it's required that it be set in an =

=0A=

Quoted 1 line

alternative way. The only alternative we have currently is adding some =

=0A=

Quoted 1 line

code to manually set the values but it's not ideal (and not upstreamable)=

.=0A=
=0A=
I wouldn't have a problem merging code in a platform board file that=0A= writes a single register that a hard-to-update bootloader forgot to write.= =0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 19:44 UTC #4 lore raw

Quoted 9 lines · Scott Wood

Quoted 8 lines · Daniel Walker
Quoted 7 lines · Scott Wood
Quoted 6 lines · Daniel Walker
Hi,
We are using the t1040 platform, and we have found that we need to populate this register. In the Technical Reference Manual it's description is section 24.3.2. This option appears in the driver, but it doesn't appears to be used anyplace.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
I'm not sure what you mean by "in the driver". U-boot sets these registers.
My hardware doesn't use u-boot , and it doesn't set these values. However, there are other reasons to have this. For example, you use u-boot but it's got a defect which sets the values incorrectly. You may not be able to update your bootloader on a shipped product.
How did you get to the point of shipping a product with this wrong? Did previous software versions not use IFC?

I don't know the details .. We have a hack in place to set the values in the kernel, but I don't see that as acceptable.

Quoted 9 lines

We could have the Linux driver initialize things, but as I said, the missing piece from the device tree isn't this register, it's the attributes.
What else does/doesn't your loader do? Does it set up a LAW that covers IFC? You only put this one register here -- does your loader set up CSPR/CSOR? If it's just this one register that was overlooked, and you can't update the bootloader, simplest may be to just have your board's platform code write it.

I don't know those details .. I can say that cspr_ext is the only thing we need to get IFC working, the only thing we have found.

It seems natual that if cspr is in the device tree, you would also want cspr_ext because both are used to identify the device. The fact that it's missing to me is strange. As I said in my prior email, even if uboot sets those, you could have cases when it's wrong. Why would I not be able to simply change the device tree to correct it ?

Quoted 30 lines

Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
We we're considered adding something to the device tree to allow populating this value, but I'm wondering if any of you have specific considerations on how this is done. Or maybe it's not needed at all, and we're just missing something.
(not a good patch, just an example.)
diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt index 89427b0..b506001 100644
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt +0 -0
@@ -24,6 +24,8 @@ Properties:
    - ranges : Each range corresponds to a single chipselect, and covers
               the entire access window as configured.
 
Quoted 7 lines
Quoted 5 lines
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt Properties: · line 27
Quoted 1 line
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt +1 -0
+- cspr_ext : This value sets the extended chip select for all banks.
I see no reason to put this here. Boot software should be setting the chipselect registers, and if for some reason you don't want to do that, you could just translate the address through ranges to find the value to write. Why would you have the binding assume it's the same for all banks?
It was only an example, to start the conversation. I don't understand what you mean by translating the address thru ranges?
of_address_to_resource()
Quoted 4 lines
Quoted 4 lines
The information that is missing from the device tree, that currently must come from boot software programming the registers, is the various attributes that get programmed in CSPR/CSOR.

Quoted 5 lines

Quoted 3 lines
Like I said mine doesn't do this, so it's required that it be set in an alternative way. The only alternative we have currently is adding some code to manually set the values but it's not ideal (and not upstreamable).
I wouldn't have a problem merging code in a platform board file that writes a single register that a hard-to-update bootloader forgot to write.

I can submit it to you, but I would much prefer a general solution that others can use without having to create board files. Our goal has been to reduce board files as much as possible, do you not agree with that?

Daniel

Scott Wood

2016-07-07 23:40 UTC #5 lore raw

On 07/07/2016 02:44 PM, Daniel Walker wrote:=0A=

Quoted 1 line

It seems natual that if cspr is in the device tree, you would also want =

=0A=

Quoted 3 lines

cspr_ext because both are used to identify the device. The fact that =0A= it's missing to me is strange. As I said in my prior email, even if =0A= uboot sets those, you could have cases when it's wrong. Why would I not =

=0A=

Quoted 1 line

be able to simply change the device tree to correct it ?=0A=

=0A=
CSPR is not in the device tree. The physical address of each chipselect=0A= is in the device tree (via the ranges property on the IFC node) and that=0A= covers both the address portion of CSPR, and CSPR_EXT.=0A=
=0A=
What I do see missing from the driver is using CSPR_EXT to match the=0A= device, most likely because the initial IFC version didn't have=0A= CSPR_EXT. Fixing that doesn't require a device tree change.=0A= =0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
The information that is missing from the device tree, that currently=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
must come from boot software programming the registers, is the various=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
attributes that get programmed in CSPR/CSOR.=0A=
=0A=
Like I said mine doesn't do this, so it's required that it be set in an=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
alternative way. The only alternative we have currently is adding some=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
code to manually set the values but it's not ideal (and not upstreamabl=

e).=0A=

Quoted 2 lines

Quoted 2 lines
I wouldn't have a problem merging code in a platform board file that=0A= writes a single register that a hard-to-update bootloader forgot to writ=

e.=0A=

Quoted 2 lines

=0A=
I can submit it to you, but I would much prefer a general solution that =

=0A=

Quoted 1 line

others can use without having to create board files. Our goal has been =

=0A=

Quoted 1 line

to reduce board files as much as possible, do you not agree with that?=0A=

=0A=
I do agree that board files are not ideal, but they're still a=0A= reasonable place to put board-specific quirks.=0A=
=0A=
I don't want to put a half-measure into the main driver and pretend it's=0A= a general solution. If the driver is to set the address, it should also=0A= set the rest of CSPR/CSOR, which requires that information to be added=0A= to the device tree. If you want to propose the latter I have no problem=0A= with that, as long as compatibility is maintained.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 20:52 UTC #6 lore raw

Quoted 6 lines · Scott Wood

Quoted 5 lines · Daniel Walker
It seems natual that if cspr is in the device tree, you would also want cspr_ext because both are used to identify the device. The fact that it's missing to me is strange. As I said in my prior email, even if uboot sets those, you could have cases when it's wrong. Why would I not be able to simply change the device tree to correct it ?

Quoted 7 lines

CSPR is not in the device tree. The physical address of each chipselect is in the device tree (via the ranges property on the IFC node) and that covers both the address portion of CSPR, and CSPR_EXT.
What I do see missing from the driver is using CSPR_EXT to match the device, most likely because the initial IFC version didn't have CSPR_EXT. Fixing that doesn't require a device tree change.

Ok ..

Quoted 13 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
The information that is missing from the device tree, that currently must come from boot software programming the registers, is the various attributes that get programmed in CSPR/CSOR.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Like I said mine doesn't do this, so it's required that it be set in an alternative way. The only alternative we have currently is adding some code to manually set the values but it's not ideal (and not upstreamable).
I wouldn't have a problem merging code in a platform board file that writes a single register that a hard-to-update bootloader forgot to write.
I can submit it to you, but I would much prefer a general solution that others can use without having to create board files. Our goal has been to reduce board files as much as possible, do you not agree with that?

Quoted 8 lines

I do agree that board files are not ideal, but they're still a reasonable place to put board-specific quirks.
I don't want to put a half-measure into the main driver and pretend it's a general solution. If the driver is to set the address, it should also set the rest of CSPR/CSOR, which requires that information to be added to the device tree. If you want to propose the latter I have no problem with that, as long as compatibility is maintained.

I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ?

I'm not really stuck on a particular device tree solution, but it was what we initial though of.

So you would support adding usage of of_address_to_resource to set the cspr and cspr_ext in the driver ?

Daniel

Scott Wood

2016-07-07 21:53 UTC #7 lore raw

On 07/07/2016 03:52 PM, Daniel Walker wrote:=0A=

Quoted 3 lines

On 07/07/2016 01:34 PM, Scott Wood wrote:=0A=
Quoted 2 lines
On 07/07/2016 02:44 PM, Daniel Walker wrote:=0A=
Quoted 1 line
It seems natual that if cspr is in the device tree, you would also want=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
cspr_ext because both are used to identify the device. The fact that=0A= it's missing to me is strange. As I said in my prior email, even if=0A= uboot sets those, you could have cases when it's wrong. Why would I not=

=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 1 line
be able to simply change the device tree to correct it ?=0A=
CSPR is not in the device tree. The physical address of each chipselect=

=0A=

Quoted 1 line

Quoted 1 line
is in the device tree (via the ranges property on the IFC node) and that=

=0A=

Quoted 10 lines

Quoted 5 lines
covers both the address portion of CSPR, and CSPR_EXT.=0A= =0A=
What I do see missing from the driver is using CSPR_EXT to match the=0A= device, most likely because the initial IFC version didn't have=0A= CSPR_EXT. Fixing that doesn't require a device tree change.=0A=
=0A=
Ok ..=0A=
=0A=
Quoted 2 lines
=0A=
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
The information that is missing from the device tree, that currently=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
must come from boot software programming the registers, is the vario=

us=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
attributes that get programmed in CSPR/CSOR.=0A=
=0A=
Like I said mine doesn't do this, so it's required that it be set in =

an=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
alternative way. The only alternative we have currently is adding som=

e=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
code to manually set the values but it's not ideal (and not upstreama=

ble).=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
I wouldn't have a problem merging code in a platform board file that=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
writes a single register that a hard-to-update bootloader forgot to wr=

ite.=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
I can submit it to you, but I would much prefer a general solution that=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
others can use without having to create board files. Our goal has been=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
to reduce board files as much as possible, do you not agree with that?=

=0A=

Quoted 4 lines

Quoted 4 lines
I do agree that board files are not ideal, but they're still a=0A= reasonable place to put board-specific quirks.=0A=
=0A=
I don't want to put a half-measure into the main driver and pretend it's=

=0A=

Quoted 1 line

Quoted 1 line
a general solution. If the driver is to set the address, it should also=

=0A=

Quoted 1 line

Quoted 1 line
set the rest of CSPR/CSOR, which requires that information to be added=

=0A=

Quoted 1 line

Quoted 1 line
to the device tree. If you want to propose the latter I have no problem=

=0A=

Quoted 3 lines

Quoted 1 line
with that, as long as compatibility is maintained.=0A=
=0A=
I suspect that add the usage of cspr_ext into the driver would fix the =

=0A=

Quoted 1 line

issue we have. It reads like you would find that acceptable ?=0A=

=0A=
What specifically is the problem you're having? Is it that CSPR_EXT is=0A= not getting written to, and thus the device does not appear at the=0A= address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver's lack=0A= of using CSPR_EXT to match would be a problem would be if you have=0A= multiple chipselects with the same address in the lower 32 bits, and=0A= only CSPR_EXT distinguishing them. Since you proposed a device tree=0A= binding that assumes all devices have the same CSPR_EXT, I doubt that's=0A= the case, so I doubt adding CSPR_EXT matching to the driver will solve=0A= your problem.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 21:49 UTC #8 lore raw

Quoted 15 lines · Scott Wood

I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ? What specifically is the problem you're having? Is it that CSPR_EXT is not getting written to, and thus the device does not appear at the address that it should?
Or is the driver matching incorrectly? The only way the driver's lack of using CSPR_EXT to match would be a problem would be if you have multiple chipselects with the same address in the lower 32 bits, and only CSPR_EXT distinguishing them. Since you proposed a device tree binding that assumes all devices have the same CSPR_EXT, I doubt that's the case, so I doubt adding CSPR_EXT matching to the driver will solve your problem.

Quoted 2 lines

-Scott

I didn't do the debug on this. From my perspective it's either flash works, or it doesn't work. We need the code below for it to work,

+#define IFC_REG_BASEADDR 0x124000
+
+static void rsp3_setup_ifc(void)
+{

+       /* set Extended Base Address for external flash chips */
+       void __iomem *ccsr_ifc;
+       ccsr_ifc = g_ccsrbp + IFC_REG_BASEADDR;
+       iowrite32be(0xF, (ccsr_ifc + 0x0C)); /* Extended Address */

+}
+

And this is our device tree blob,

+ ifc: localbus@ffb124000 {

+               #address-cells = <2>;
+               #size-cells = <1>;
+               compatible = "fsl,ifc", "simple-bus";
+               interrupts = <0x19 0x2 0x0 0x0>;
+               reg = <0xf 0xfb124000 0 0x2000>;
+               ranges = <0 0 0xf 0xfc000000 0x4000000>;
+
+               flash@0,0 {
+                       #address-cells = <1>;
+                       #size-cells = <1>;
+                       compatible = "cfi-flash";
+                       reg = <0x0 0x0 0x4000000>;

+
+ bank-width = <2>;
+ device-width = <1>;
+
+ csl_s@200000 {
+ label = "csl_s";
+ reg = <0x20000 0x200000>;
+ };
+
+ space_ava@400000 {
+ label ="Space Available";
+ reg = <0x400000 0x2BA0000>;
+ };
+
+ nvram_backup@35e0000 {
+ label ="NVRAM Backup";
+ reg = <0x35e0000 0x20000>;
+ };
+
+ upgrade@3600000 {
+ label ="Upgrade Boot Rom";
+ reg = <0x3600000 0x200000>;
+ };
+
+ nvram@3de0000 {
+ label ="NVRAM";
+ reg = <0x3de0000 0x20000>;
+ };
+
+ bootrom@3e00000 {

+                               label ="Running Boot Rom";
+                               reg = <0x3e00000 0x200000>;
+                               read-only;

+ };
+ };
+ };
+

Scott Wood

2016-07-08 03:10 UTC #9 lore raw

On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=

Quoted 3 lines

On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=
Quoted 2 lines
=0A=
I suspect that add the usage of cspr_ext into the driver would fix the=

=0A=

Quoted 2 lines

Quoted 2 lines
issue we have. It reads like you would find that acceptable ?=0A= What specifically is the problem you're having? Is it that CSPR_EXT is=

=0A=

Quoted 4 lines

Quoted 4 lines
not getting written to, and thus the device does not appear at the=0A= address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver's lack=

=0A=

Quoted 4 lines

Quoted 4 lines
of using CSPR_EXT to match would be a problem would be if you have=0A= multiple chipselects with the same address in the lower 32 bits, and=0A= only CSPR_EXT distinguishing them. Since you proposed a device tree=0A= binding that assumes all devices have the same CSPR_EXT, I doubt that's=

=0A=

Quoted 1 line

Quoted 1 line
the case, so I doubt adding CSPR_EXT matching to the driver will solve=

=0A=

Quoted 7 lines

Quoted 4 lines
your problem.=0A=
=0A=
-Scott=0A=
=0A=
=0A=
I didn't do the debug on this. From my perspective it's either flash =0A= works, or it doesn't work. We need the code below for it to work,=0A=

=0A=
Adding CSPR_EXT matching to the driver will not accomplish the same=0A= thing as that code.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 22:01 UTC #10 lore raw

Quoted 16 lines · Scott Wood

Quoted 15 lines · Daniel Walker
Quoted 14 lines · Scott Wood
I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ? What specifically is the problem you're having? Is it that CSPR_EXT is not getting written to, and thus the device does not appear at the address that it should?
Or is the driver matching incorrectly? The only way the driver's lack of using CSPR_EXT to match would be a problem would be if you have multiple chipselects with the same address in the lower 32 bits, and only CSPR_EXT distinguishing them. Since you proposed a device tree binding that assumes all devices have the same CSPR_EXT, I doubt that's the case, so I doubt adding CSPR_EXT matching to the driver will solve your problem.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
-Scott
I didn't do the debug on this. From my perspective it's either flash works, or it doesn't work. We need the code below for it to work,
Adding CSPR_EXT matching to the driver will not accomplish the same thing as that code.

So from u-boot perspective, the values in the device tree under "ranges" or parts of it, are place into the cspr and cspr_ext ? Is that how it's suppose to work ?

Daniel

Scott Wood

2016-07-07 22:37 UTC #11 lore raw

On 07/07/2016 05:01 PM, Daniel Walker wrote:=0A=

Quoted 4 lines

On 07/07/2016 02:59 PM, Scott Wood wrote:=0A=
Quoted 3 lines
On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=
Quoted 2 lines
On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=
Quoted 1 line
I suspect that add the usage of cspr_ext into the driver would fix the=

=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
issue we have. It reads like you would find that acceptable ?=0A= What specifically is the problem you're having? Is it that CSPR_EXT i=

s=0A=

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
not getting written to, and thus the device does not appear at the=0A= address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver's lack=

=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
of using CSPR_EXT to match would be a problem would be if you have=0A= multiple chipselects with the same address in the lower 32 bits, and=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
only CSPR_EXT distinguishing them. Since you proposed a device tree=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
binding that assumes all devices have the same CSPR_EXT, I doubt that'=

s=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
the case, so I doubt adding CSPR_EXT matching to the driver will solve=

=0A=

Quoted 11 lines

Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
your problem.=0A=
=0A=
-Scott=0A=
=0A=
I didn't do the debug on this. From my perspective it's either flash=0A= works, or it doesn't work. We need the code below for it to work,=0A=
Adding CSPR_EXT matching to the driver will not accomplish the same=0A= thing as that code.=0A=
=0A=
=0A=
So from u-boot perspective, the values in the device tree under "ranges" =

=0A=

Quoted 1 line

or parts of it, are place into the cspr and cspr_ext ? Is that how it's =

=0A=

Quoted 1 line

suppose to work ?=0A=

=0A=
U-Boot writes values that are hardcoded in the board config header.=0A= These values (as well as the area covered by the IFC LAW) need to match=0A= the address in the device tree, but U-Boot doesn't get them from the=0A= device tree.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-07 23:48 UTC #12 lore raw

Quoted 25 lines · Scott Wood

Quoted 24 lines · Daniel Walker
Quoted 23 lines · Scott Wood
Quoted 15 lines · Daniel Walker
Quoted 14 lines · Scott Wood
I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ? What specifically is the problem you're having? Is it that CSPR_EXT is not getting written to, and thus the device does not appear at the address that it should?
Or is the driver matching incorrectly? The only way the driver's lack of using CSPR_EXT to match would be a problem would be if you have multiple chipselects with the same address in the lower 32 bits, and only CSPR_EXT distinguishing them. Since you proposed a device tree binding that assumes all devices have the same CSPR_EXT, I doubt that's the case, so I doubt adding CSPR_EXT matching to the driver will solve your problem.
Quoted 4 lines
Quoted 2 lines
-Scott
I didn't do the debug on this. From my perspective it's either flash works, or it doesn't work. We need the code below for it to work,
Adding CSPR_EXT matching to the driver will not accomplish the same thing as that code.

Quoted 8 lines

Quoted 3 lines
So from u-boot perspective, the values in the device tree under "ranges" or parts of it, are place into the cspr and cspr_ext ? Is that how it's suppose to work ?
U-Boot writes values that are hardcoded in the board config header. These values (as well as the area covered by the IFC LAW) need to match the address in the device tree, but U-Boot doesn't get them from the device tree.

I was suggesting the values it writes are the same as the ones inside the device tree. So we could have both csrp and csrp_ext written from the driver and the values would come from the ranges property.

Daniel

Scott Wood

2016-07-09 01:12 UTC #13 lore raw

On 07/07/2016 06:48 PM, Daniel Walker wrote:=0A=

Quoted 6 lines

On 07/07/2016 03:37 PM, Scott Wood wrote:=0A=
Quoted 5 lines
On 07/07/2016 05:01 PM, Daniel Walker wrote:=0A=
Quoted 4 lines
On 07/07/2016 02:59 PM, Scott Wood wrote:=0A=
Quoted 3 lines
On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=
Quoted 2 lines
On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=
Quoted 1 line
I suspect that add the usage of cspr_ext into the driver would fix t=

he=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
issue we have. It reads like you would find that acceptable ?=0A= What specifically is the problem you're having? Is it that CSPR_EXT=

is=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
not getting written to, and thus the device does not appear at the=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver's la=

ck=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
of using CSPR_EXT to match would be a problem would be if you have=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
multiple chipselects with the same address in the lower 32 bits, and=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
only CSPR_EXT distinguishing them. Since you proposed a device tree=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
binding that assumes all devices have the same CSPR_EXT, I doubt tha=

t's=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the case, so I doubt adding CSPR_EXT matching to the driver will sol=

ve=0A=

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
your problem.=0A=
=0A=
-Scott=0A=
=0A=
I didn't do the debug on this. From my perspective it's either flash=

=0A=

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
works, or it doesn't work. We need the code below for it to work,=0A=
Adding CSPR_EXT matching to the driver will not accomplish the same=0A= thing as that code.=0A=
=0A=
So from u-boot perspective, the values in the device tree under "ranges=

"=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
or parts of it, are place into the cspr and cspr_ext ? Is that how it's=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 1 line
suppose to work ?=0A=
U-Boot writes values that are hardcoded in the board config header.=0A= These values (as well as the area covered by the IFC LAW) need to match=

=0A=

Quoted 8 lines

Quoted 3 lines
the address in the device tree, but U-Boot doesn't get them from the=0A= device tree.=0A=
=0A=
=0A=
I was suggesting the values it writes are the same as the ones inside =0A= the device tree. So we could have both csrp and csrp_ext written from =0A= the driver and the values would=0A=
come from the ranges property.=0A=

=0A=
There's more to CSPR than just the address. The driver should either be=0A= able to assume that all of CSPR/CSOR has been correctly initialized, or=0A= it should assume none of that has been initialized -- which again,=0A= requires the attribute information to be in the device tree. If you're=0A= doing something in between, then that's a board quirk rather than a=0A= general solution.=0A=
=0A=
-Scott=0A=
=0A=

Daniel Walker

author

2016-07-11 16:36 UTC #14 lore raw

Quoted 39 lines · Scott Wood

Quoted 34 lines · Daniel Walker
Quoted 25 lines · Scott Wood
Quoted 24 lines · Daniel Walker
Quoted 23 lines · Scott Wood
Quoted 15 lines · Daniel Walker
Quoted 14 lines · Scott Wood
I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ? What specifically is the problem you're having? Is it that CSPR_EXT is not getting written to, and thus the device does not appear at the address that it should?
Or is the driver matching incorrectly? The only way the driver's lack of using CSPR_EXT to match would be a problem would be if you have multiple chipselects with the same address in the lower 32 bits, and only CSPR_EXT distinguishing them. Since you proposed a device tree binding that assumes all devices have the same CSPR_EXT, I doubt that's the case, so I doubt adding CSPR_EXT matching to the driver will solve your problem.
Quoted 4 lines
Quoted 2 lines
-Scott
I didn't do the debug on this. From my perspective it's either flash works, or it doesn't work. We need the code below for it to work,
Adding CSPR_EXT matching to the driver will not accomplish the same thing as that code.
Quoted 8 lines
Quoted 3 lines
So from u-boot perspective, the values in the device tree under "ranges" or parts of it, are place into the cspr and cspr_ext ? Is that how it's suppose to work ?
U-Boot writes values that are hardcoded in the board config header. These values (as well as the area covered by the IFC LAW) need to match the address in the device tree, but U-Boot doesn't get them from the device tree.
Quoted 4 lines
I was suggesting the values it writes are the same as the ones inside the device tree. So we could have both csrp and csrp_ext written from the driver and the values would
come from the ranges property.

Quoted 9 lines

There's more to CSPR than just the address. The driver should either be able to assume that all of CSPR/CSOR has been correctly initialized, or it should assume none of that has been initialized -- which again, requires the attribute information to be in the device tree. If you're doing something in between, then that's a board quirk rather than a general solution.
-Scott

It would seems like a good idea to add it then. I think it can be piece mail, rather than all or nothing tho. How difficult is adding the other part to the driver , v.s. just the cspr_ext ?

Daniel

Scott Wood

2016-07-11 17:11 UTC #15 lore raw

On 07/11/2016 11:36 AM, Daniel Walker wrote:=0A=

Quoted 8 lines

On 07/08/2016 06:12 PM, Scott Wood wrote:=0A=
Quoted 7 lines
On 07/07/2016 06:48 PM, Daniel Walker wrote:=0A=
Quoted 6 lines
On 07/07/2016 03:37 PM, Scott Wood wrote:=0A=
Quoted 5 lines
On 07/07/2016 05:01 PM, Daniel Walker wrote:=0A=
Quoted 4 lines
On 07/07/2016 02:59 PM, Scott Wood wrote:=0A=
Quoted 3 lines
On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=
Quoted 2 lines
On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=
Quoted 1 line
I suspect that add the usage of cspr_ext into the driver would fix=

the=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
issue we have. It reads like you would find that acceptable ?=0A= What specifically is the problem you're having? Is it that CSPR_E=

XT is=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
not getting written to, and thus the device does not appear at the=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver's =

lack=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
of using CSPR_EXT to match would be a problem would be if you have=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
multiple chipselects with the same address in the lower 32 bits, a=

nd=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
only CSPR_EXT distinguishing them. Since you proposed a device tr=

ee=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
binding that assumes all devices have the same CSPR_EXT, I doubt t=

hat's=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the case, so I doubt adding CSPR_EXT matching to the driver will s=

olve=0A=

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
your problem.=0A=
=0A=
-Scott=0A=
=0A=
I didn't do the debug on this. From my perspective it's either flas=

h=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
works, or it doesn't work. We need the code below for it to work,=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Adding CSPR_EXT matching to the driver will not accomplish the same=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
thing as that code.=0A=
=0A=
So from u-boot perspective, the values in the device tree under "rang=

es"=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
or parts of it, are place into the cspr and cspr_ext ? Is that how it=

's=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
suppose to work ?=0A=
U-Boot writes values that are hardcoded in the board config header.=0A= These values (as well as the area covered by the IFC LAW) need to matc=

h=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
the address in the device tree, but U-Boot doesn't get them from the=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
device tree.=0A=
=0A=
I was suggesting the values it writes are the same as the ones inside=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
the device tree. So we could have both csrp and csrp_ext written from=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 2 lines
the driver and the values would=0A=
come from the ranges property.=0A=
There's more to CSPR than just the address. The driver should either be=

=0A=

Quoted 1 line

Quoted 1 line
able to assume that all of CSPR/CSOR has been correctly initialized, or=

=0A=

Quoted 2 lines

Quoted 2 lines
it should assume none of that has been initialized -- which again,=0A= requires the attribute information to be in the device tree. If you're=

=0A=

Quoted 7 lines

Quoted 5 lines
doing something in between, then that's a board quirk rather than a=0A= general solution.=0A=
=0A=
-Scott=0A=
=0A=
=0A=
It would seems like a good idea to add it then. I think it can be piece =

=0A=

Quoted 1 line

mail, rather than all or nothing tho. How difficult is adding the other =

=0A=

Quoted 1 line

part to the driver , v.s. just the cspr_ext ?=0A=

=0A=
Writing only cspr_ext is a hack to work around a bug and should not be=0A= disguised as a "piecemeal" implementation of something different.=0A= =0A=
-Scott=0A=

Daniel Walker

author

2016-07-11 17:10 UTC #16 lore raw

Quoted 50 lines · Scott Wood

Quoted 49 lines · Daniel Walker
Quoted 48 lines · Scott Wood
Quoted 34 lines · Daniel Walker
Quoted 25 lines · Scott Wood
Quoted 24 lines · Daniel Walker
Quoted 23 lines · Scott Wood
Quoted 15 lines · Daniel Walker
Quoted 14 lines · Scott Wood
I suspect that add the usage of cspr_ext into the driver would fix the issue we have. It reads like you would find that acceptable ? What specifically is the problem you're having? Is it that CSPR_EXT is not getting written to, and thus the device does not appear at the address that it should?
Or is the driver matching incorrectly? The only way the driver's lack of using CSPR_EXT to match would be a problem would be if you have multiple chipselects with the same address in the lower 32 bits, and only CSPR_EXT distinguishing them. Since you proposed a device tree binding that assumes all devices have the same CSPR_EXT, I doubt that's the case, so I doubt adding CSPR_EXT matching to the driver will solve your problem.
Quoted 4 lines
Quoted 2 lines
-Scott
I didn't do the debug on this. From my perspective it's either flash works, or it doesn't work. We need the code below for it to work,
Adding CSPR_EXT matching to the driver will not accomplish the same thing as that code.
Quoted 8 lines
Quoted 3 lines
So from u-boot perspective, the values in the device tree under "ranges" or parts of it, are place into the cspr and cspr_ext ? Is that how it's suppose to work ?
U-Boot writes values that are hardcoded in the board config header. These values (as well as the area covered by the IFC LAW) need to match the address in the device tree, but U-Boot doesn't get them from the device tree.
Quoted 4 lines
I was suggesting the values it writes are the same as the ones inside the device tree. So we could have both csrp and csrp_ext written from the driver and the values would
come from the ranges property.
There's more to CSPR than just the address. The driver should either be able to assume that all of CSPR/CSOR has been correctly initialized, or it should assume none of that has been initialized -- which again, requires the attribute information to be in the device tree. If you're doing something in between, then that's a board quirk rather than a general solution.
-Scott

Quoted 7 lines

Quoted 3 lines
It would seems like a good idea to add it then. I think it can be piece mail, rather than all or nothing tho. How difficult is adding the other part to the driver , v.s. just the cspr_ext ?
Writing only cspr_ext is a hack to work around a bug and should not be disguised as a "piecemeal" implementation of something different.
-Scott

Ok .. How hard is it to do all the stuff your asking for ?

Daniel

Scott Wood

2016-07-11 18:27 UTC #17 lore raw

On 07/11/2016 12:10 PM, Daniel Walker wrote:=0A=

Quoted 10 lines

On 07/11/2016 09:55 AM, Scott Wood wrote:=0A=
Quoted 9 lines
On 07/11/2016 11:36 AM, Daniel Walker wrote:=0A=
Quoted 8 lines
On 07/08/2016 06:12 PM, Scott Wood wrote:=0A=
Quoted 7 lines
On 07/07/2016 06:48 PM, Daniel Walker wrote:=0A=
Quoted 6 lines
On 07/07/2016 03:37 PM, Scott Wood wrote:=0A=
Quoted 5 lines
On 07/07/2016 05:01 PM, Daniel Walker wrote:=0A=
Quoted 4 lines
On 07/07/2016 02:59 PM, Scott Wood wrote:=0A=
Quoted 3 lines
On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=
Quoted 2 lines
On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=
Quoted 1 line
I suspect that add the usage of cspr_ext into the driver would f=

ix the=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
issue we have. It reads like you would find that acceptable ?=0A= What specifically is the problem you're having? Is it that CSPR=

_EXT is=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
not getting written to, and thus the device does not appear at t=

he=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
address that it should?=0A=
=0A=
Or is the driver matching incorrectly? The only way the driver'=

s lack=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
of using CSPR_EXT to match would be a problem would be if you ha=

ve=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
multiple chipselects with the same address in the lower 32 bits,=

and=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
only CSPR_EXT distinguishing them. Since you proposed a device =

tree=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
binding that assumes all devices have the same CSPR_EXT, I doubt=

that's=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the case, so I doubt adding CSPR_EXT matching to the driver will=

solve=0A=

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
your problem.=0A=
=0A=
-Scott=0A=
=0A=
I didn't do the debug on this. From my perspective it's either fl=

ash=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
works, or it doesn't work. We need the code below for it to work,=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Adding CSPR_EXT matching to the driver will not accomplish the sam=

e=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
thing as that code.=0A=
=0A=
So from u-boot perspective, the values in the device tree under "ra=

nges"=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
or parts of it, are place into the cspr and cspr_ext ? Is that how =

it's=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
suppose to work ?=0A=
U-Boot writes values that are hardcoded in the board config header.=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
These values (as well as the area covered by the IFC LAW) need to ma=

tch=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the address in the device tree, but U-Boot doesn't get them from the=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
device tree.=0A=
=0A=
I was suggesting the values it writes are the same as the ones inside=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the device tree. So we could have both csrp and csrp_ext written from=

=0A=

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
the driver and the values would=0A=
come from the ranges property.=0A=
There's more to CSPR than just the address. The driver should either =

be=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
able to assume that all of CSPR/CSOR has been correctly initialized, o=

r=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
it should assume none of that has been initialized -- which again,=0A= requires the attribute information to be in the device tree. If you'r=

e=0A=

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
doing something in between, then that's a board quirk rather than a=0A= general solution.=0A=
=0A=
-Scott=0A=
=0A=
It would seems like a good idea to add it then. I think it can be piece=

=0A=

Quoted 1 line

Quoted 1 line
Quoted 1 line
mail, rather than all or nothing tho. How difficult is adding the other=

=0A=

Quoted 2 lines

Quoted 2 lines
Quoted 1 line
part to the driver , v.s. just the cspr_ext ?=0A=
Writing only cspr_ext is a hack to work around a bug and should not be=

=0A=

Quoted 5 lines

Quoted 3 lines
disguised as a "piecemeal" implementation of something different.=0A= =0A=
-Scott=0A=
=0A=
Ok .. How hard is it to do all the stuff your asking for ?=0A=

=0A=
It shouldn't be hard. Add properties (at the chipselect level -- not=0A= IFC controller) that hold the non-address portions of CSPR, CSOR, and=0A= (if applicable to the IFC version) CSOR_EXT, as well as the chipselect=0A= number. If the chipselect property is present then the others must be=0A= as well. Code to handle this should go in drivers/memory/fsl_ifc.c=0A= rather than the NAND driver.=0A=
=0A=
If the chipselect property is absent, then the driver will assume that=0A= the loader has set up all chipselect registers correctly.=0A= =0A=
-Scott=0A=
=0A=