t1040 IFC flash driver Extended Chip Select

[Scott Wood]

On 07/07/2016 03:52 PM, Daniel Walker wrote:
> On 07/07/2016 01:34 PM, Scott Wood wrote:
>> On 07/07/2016 02:44 PM, Daniel Walker wrote:
>>> 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 ?
>> 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 ..
> 
>>
>>>>>> 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?
>> 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 ?

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.

-Scott