t1040 IFC flash driver Extended Chip Select

[Scott Wood]

On 07/11/2016 12:10 PM, Daniel Walker wrote:
> On 07/11/2016 09:55 AM, Scott Wood wrote:
>> On 07/11/2016 11:36 AM, Daniel Walker wrote:
>>> On 07/08/2016 06:12 PM, Scott Wood wrote:
>>>> On 07/07/2016 06:48 PM, Daniel Walker wrote:
>>>>> On 07/07/2016 03:37 PM, Scott Wood wrote:
>>>>>> On 07/07/2016 05:01 PM, Daniel Walker wrote:
>>>>>>> On 07/07/2016 02:59 PM, Scott Wood wrote:
>>>>>>>> On 07/07/2016 04:49 PM, Daniel Walker wrote:
>>>>>>>>> On 07/07/2016 02:23 PM, Scott Wood wrote:
>>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>> 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 ?
>>>>>> 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.
>>>> 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 ?
>> 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 ?

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

If the chipselect property is absent, then the driver will assume that
the loader has set up all chipselect registers correctly.

-Scott