Re: t1040 IFC flash driver Extended Chip Select

On 07/07/2016 03:52 PM, Daniel Walker wrote:=0A=

On 07/07/2016 01:34 PM, Scott Wood wrote:=0A=

quoted

On 07/07/2016 02:44 PM, Daniel Walker wrote:=0A=

quoted

It seems natual that if cspr is in the device tree, you would also want=

=0A=

quoted

quoted

cspr_ext because both are used to identify the device. The fact that=0A=
it's missing to me is strange. As I said in my prior email, even if=0A=
uboot sets those, you could have cases when it's wrong. Why would I not=

=0A=

quoted

quoted

be able to simply change the device tree to correct it ?=0A=

CSPR is not in the device tree.  The physical address of each chipselect=

=0A=

quoted

is in the device tree (via the ranges property on the IFC node) and that=

=0A=

quoted

covers both the address portion of CSPR, and CSPR_EXT.=0A=
=0A=
What I do see missing from the driver is using CSPR_EXT to match the=0A=
device, most likely because the initial IFC version didn't have=0A=
CSPR_EXT.  Fixing that doesn't require a device tree change.=0A=

=0A=
Ok ..=0A=
=0A=

quoted

=0A=

quoted

quoted

quoted

quoted

The information that is missing from the device tree, that currently=

=0A=

quoted

quoted

quoted

quoted

quoted

must come from boot software programming the registers, is the vario=

us=0A=

quoted

quoted

quoted

quoted

quoted

attributes that get programmed in CSPR/CSOR.=0A=
=0A=

Like I said mine doesn't do this, so it's required that it be set in =

an=0A=

quoted

quoted

quoted

quoted

alternative way. The only alternative we have currently is adding som=

e=0A=

quoted

quoted

quoted

quoted

code to manually set the values but it's not ideal (and not upstreama=

ble).=0A=

quoted

quoted

quoted

I wouldn't have a problem merging code in a platform board file that=

=0A=

quoted

quoted

quoted

writes a single register that a hard-to-update bootloader forgot to wr=

ite.=0A=

quoted

quoted

I can submit it to you, but I would much prefer a general solution that=

=0A=

quoted

quoted

others can use without having to create board files. Our goal has been=

=0A=

quoted

quoted

to reduce board files as much as possible, do you not agree with that?=

=0A=

quoted

I do agree that board files are not ideal, but they're still a=0A=
reasonable place to put board-specific quirks.=0A=
=0A=
I don't want to put a half-measure into the main driver and pretend it's=

=0A=

quoted

a general solution.  If the driver is to set the address, it should also=

=0A=

quoted

set the rest of CSPR/CSOR, which requires that information to be added=

=0A=

quoted

to the device tree.  If you want to propose the latter I have no problem=

=0A=

quoted

with that, as long as compatibility is maintained.=0A=

=0A=
I suspect that add the usage of cspr_ext into the driver would fix the =

=0A=

issue we have. It reads like you would find that acceptable ?=0A=

=0A=
What specifically is the problem you're having?  Is it that CSPR_EXT is=0A=
not getting written to, and thus the device does not appear at the=0A=
address that it should?=0A=
=0A=
Or is the driver matching incorrectly?  The only way the driver's lack=0A=
of using CSPR_EXT to match would be a problem would be if you have=0A=
multiple chipselects with the same address in the lower 32 bits, and=0A=
only CSPR_EXT distinguishing them.  Since you proposed a device tree=0A=
binding that assumes all devices have the same CSPR_EXT, I doubt that's=0A=
the case, so I doubt adding CSPR_EXT matching to the driver will solve=0A=
your problem.=0A=
=0A=
-Scott=0A=
=0A=