Re: t1040 IFC flash driver Extended Chip Select

On 07/11/2016 12:10 PM, Daniel Walker wrote:=0A=

On 07/11/2016 09:55 AM, Scott Wood wrote:=0A=

quoted

On 07/11/2016 11:36 AM, Daniel Walker wrote:=0A=

quoted

On 07/08/2016 06:12 PM, Scott Wood wrote:=0A=

quoted

On 07/07/2016 06:48 PM, Daniel Walker wrote:=0A=

quoted

On 07/07/2016 03:37 PM, Scott Wood wrote:=0A=

quoted

On 07/07/2016 05:01 PM, Daniel Walker wrote:=0A=

quoted

On 07/07/2016 02:59 PM, Scott Wood wrote:=0A=

quoted

On 07/07/2016 04:49 PM, Daniel Walker wrote:=0A=

quoted

On 07/07/2016 02:23 PM, Scott Wood wrote:=0A=

quoted

I suspect that add the usage of cspr_ext into the driver would f=

ix the=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

issue we have. It reads like you would find that acceptable ?=0A=
What specifically is the problem you're having?  Is it that CSPR=

_EXT is=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

not getting written to, and thus the device does not appear at t=

he=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

address that it should?=0A=
=0A=
Or is the driver matching incorrectly?  The only way the driver'=

s lack=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

of using CSPR_EXT to match would be a problem would be if you ha=

ve=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

multiple chipselects with the same address in the lower 32 bits,=

 and=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

only CSPR_EXT distinguishing them.  Since you proposed a device =

tree=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

binding that assumes all devices have the same CSPR_EXT, I doubt=

 that's=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

the case, so I doubt adding CSPR_EXT matching to the driver will=

 solve=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

your problem.=0A=
=0A=
-Scott=0A=
=0A=

I didn't do the debug on this. From my perspective it's either fl=

ash=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

works, or it doesn't work. We need the code below for it to work,=

=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

Adding CSPR_EXT matching to the driver will not accomplish the sam=

e=0A=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

thing as that code.=0A=
=0A=

So from u-boot perspective, the values in the device tree under "ra=

nges"=0A=

quoted

quoted

quoted

quoted

quoted

quoted

or parts of it, are place into the cspr and cspr_ext ? Is that how =

it's=0A=

quoted

quoted

quoted

quoted

quoted

quoted

suppose to work ?=0A=

U-Boot writes values that are hardcoded in the board config header.=

=0A=

quoted

quoted

quoted

quoted

quoted

These values (as well as the area covered by the IFC LAW) need to ma=

tch=0A=

quoted

quoted

quoted

quoted

quoted

the address in the device tree, but U-Boot doesn't get them from the=

=0A=

quoted

quoted

quoted

quoted

quoted

device tree.=0A=
=0A=

I was suggesting the values it writes are the same as the ones inside=

=0A=

quoted

quoted

quoted

quoted

the device tree. So we could have both csrp and csrp_ext written from=

=0A=

quoted

quoted

quoted

quoted

the driver and the values would=0A=
come from the ranges property.=0A=

There's more to CSPR than just the address.  The driver should either =

be=0A=

quoted

quoted

quoted

able to assume that all of CSPR/CSOR has been correctly initialized, o=

r=0A=

quoted

quoted

quoted

it should assume none of that has been initialized -- which again,=0A=
requires the attribute information to be in the device tree.  If you'r=

e=0A=

quoted

quoted

quoted

doing something in between, then that's a board quirk rather than a=0A=
general solution.=0A=
=0A=
-Scott=0A=
=0A=

It would seems like a good idea to add it then. I think it can be piece=

=0A=

quoted

quoted

mail, rather than all or nothing tho. How difficult is adding the other=

=0A=

quoted

quoted

part to the driver , v.s. just the cspr_ext ?=0A=

Writing only cspr_ext is a hack to work around a bug and should not be=

=0A=

quoted

disguised as a "piecemeal" implementation of something different.=0A=
=0A=
-Scott=0A=

=0A=
Ok .. How hard is it to do all the stuff your asking for ?=0A=

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