t1040 IFC flash driver Extended Chip Select

[Scott Wood]

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?

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.

>>> 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.

-Scott