Re: t1040 IFC flash driver Extended Chip Select

[Daniel Walker <danielwa@cisco.com>]

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 ?

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