On 07/07/2016 12:26 PM, Scott Wood wrote:
On 07/07/2016 10:48 AM, Daniel Walker wrote:
On 07/06/2016 05:57 PM, Scott Wood wrote:
On 07/06/2016 03:23 PM, Daniel Walker wrote:
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.
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.
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 ?
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
--- a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
@@ -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.
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()
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).
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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
