On Tue, Oct 25, 2016 at 03:58:49AM +0000, Punnaiah Choudary Kalluri wrote: > > I have hacked the v7 patchset enough to work on 4.8 and hacked it some > > more to work on my hardware. The driver appears to be in no better > > shape than the 3.12 out-of-tree Xilinx release I was using previously.. > > The driver in Xilinx tree works with 4.6 kernel and adopted the > required
I mean, the driver from the 3.12 Xilinx tree that I last looked at years ago. This is at v7 now and is still needs lots of work, I did some fixing, including making parts of it work on 4.8. You can copy it out of the patch I sent you. > Changes (may release in 1-2 weeks). Still it need some rework, now a days > I am seeing many requests from different people for this driver and some of > Them are using different version of IP as you know this IP has four variants > and > Xilinx is using the pl353 variant. Well, I am using Xilinx Zynq hardware and care about making that configuration actually work. This is the last driver I require to use Zynq with mainline. It clearly needs more work than just forward porting to 4.8, so please let me know if you are able to tackle this. > Let's wait for the next series of patches and Get the patches tested > with others as well along with the review comments. You now have 10 review comments from me, please respond to all of them in your v8 patchset - no sense in continuing to drag this out. Please cc me on future series. Jason > Regards, > Punnaiah > > I've attached the ugly, ugly patch I made, it may save you some time > > when preparing v8. > > > > Commentary: > > 1) nand_chip already includes mtd_info, no reason for a second one in > > the pl353_nand_info struct. The standard accessors mtd_to_nand/etc > > should be used instead of priv > > 2) I hacked out all the ECC stuff from the driver since I don't use > > it and it was broken.. Obviously some has to come back, but fixing > > other things on this list first will make that much easier and better.. > > 3) pl353_nand_write_page_swecc/pl353_nand_read_page_swecc are pure > > outdated copies of the core routines and should not exist in a > > driver. This needs to be fixed so nand_scan_tail sets them. > > This seems to be a side effect of #9 ? > > 4) The hacky stuff to detect 2 vs 3 address cycle NAND doesn't work, > > and doesn't work without ONFI (see patch for possible fix). The > > driver should probably use the same scheme the core code uses.. > > But this is all a problem because of #10 > > 5) The driver assumes alignment of the iomap, needs to use + not | > > when constructing the address. (yes, this blows up on my system) > > 6) Driver unconditionally sets timing to ONFI mode 0 (slow!) > > Maybe timing should be common code driven by DT.. > > 7) Driver unconditionally selects a BBT format, and an ECC layout > > (neither match what my system uses, but I guess that is a core mtd > > issue these days :/) > > 8) Driver unconditionally forces a chip-delay (wrong for my > > system) maybe this should be common code driven by DT.. > > 9) This buisness with pl353_nand_ecc_init and the > > special functions to do PL353_NAND_LAST_TRANSFER_LENGTH stuff > > is just horrid. I'd expect that is a big NAK. > > > > The driver needs to implement read_buf *properly* so the core > > routines can be used instead of trying to 'fix' the call sites > > to do the PL353_NAND_LAST_TRANSFER_LENGTH stuff. > > 10) pl353_nand_cmd_function is a wonky copy of nand_command_lp, > > maybe > > this driver should use cmd_ctrl, or the core code should be > > refactored some more..