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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
