CeDeROM wrote: > First, we should not modify anything, but discuss what is the status > of the work, what and how we should change anything, if necessary.
Yes and no. It's fine that Simon's changes touch code which is relevant also for libswd. The important thing (and I think this is actually what you mean) is that the changes are going somehow in the same direction so as to support both Versaloon and libswd code. > > I found both libswd and my SWD patch modify 2 main functions in > > arm_adi_v5.c, which are ahbap_debugport_init and mem_ap_read_buf_u32. > > LibSWD im far more complex than that - it only modify OpenOCD code > where absolutely necessary. This is also why it takes so long to > make it work, because library needs to fix design errors of the > application. I disagree very strongly with this. Design errors in OpenOCD must be fixed in OpenOCD and not worked around in libswd, otherwise we are going in the wrong direction. Did you review Simon's patch in detail? Do you see some parts of it which makes the situation within OpenOCD worse, or does it make things better, or does it simply not change the design errors at all? > The debugport_init was changes because sticky flags handling is > different for jtag and swd. Different how? > We should create additional transport independent function set to > check and remove error conditions, or simply check what is the > transport and apply proper operation in place.. The first option > could become additional transport routine. This sounds good. > > ahbap_debugport_init should obviously be modified to initialize > > SWD if SWD transport is sellected. > > ..or we can create transport independent sticky flags handling, > because rest of the initialization is the same, see above :-) How much is actually common between JTAG and SWD? > > And mem_ap_read_buf_u32 should not call adi_jtag_dp_scan, or implement > > mem_ap_read_buf_u32_jtag and mem_ap_read_buf_u32_swd. > > mem_ap_read_buf_u32 has a dirty hack that hardcodes use of jtag > functions. This is why I simply created a wrapper to call existing > funtion if jtag is used (backward compatibility) and/or call new > function that use swd transport. This sounds fine. > The problem with acessing the MEM-AP is somehow more complex, > especially when sticky error handling is active. I guess this will be handled in Versaloon firmware and libswd, respectively. > Note few important details: > 1. This is different than abiguous specification. Argh. :( > 2. OpenOCD does not have retry mechanism of any kind, so I had to > implement it in LibSWD. I think this is a good thing actually. It's important to not lock OpenOCD to relatively stupid bitbanging interfaces, but to go in the direction of more intelligent interfaces like the Versaloon, which can accept high-level commands and do retry if neccessary. I'm happy that libswd uses the same approach, I think this is exactly the right place when using the simpler interfaces. > 3. OpenOCD can queue more read write operations and then flush the > queue. Again it does not check where was the error, simply crashes if > execution failed, so I had to add error handling as everything went > smooth in LibSWD (imagine working with such thing at queue level). This must be changed in OpenOCD, and can be changed immediately if I understand correctly? The change would mostly be when flushing, and obviously knowing where the error was, and act accordingly. But first just return an error instead of crashing. > 4. Enqueueing read operations will dramatically increase efficiency, > but it also complicates code that does not use double pointers, also > the error handling is far more complex. Double pointers are just tools, there's no reason to avoid them when they do what we want. > 5. We dont know what will happen if TAR autoincrement is active, this > solution might not work... What decides if autoincrement is active? > work is somehow slow because noone want to support it recently with > their investment and/or resources, everyone just want the results > for free, and I need to work. This is perfectly fine. Of course your contributions are very much appreciated. For me, one part of the problem is that I still know too few details to really contribute effectively. Hence all my questions. > Recently the work was intensified somehow with Akos we are pushing > forward, the retry mechanism seems to be working at last, I need to > clean up the code and push it to the repo, Please rebase the work on current openocd.git before making further changes. The more time that passes, the more work there will be to do the rebase. Getting the changes rebased has IMO higher priority than any other progress. > > 2. In libswd code, 2 functions are implemented for mem_ap_read_buf_u32, > > which are mem_ap_read_buf_u32_jtag and mem_ap_read_buf_u32_swd. > > And they are called in mem_ap_read_buf_u32 according to the transport > > sellected. > > The mem_ap_read_buf_u32 should use transport independent routine > provided by transport layer and we should get rid of the jtag-specific > dirty hack. This is not libswd change itself but fix in the OpenOCD This also sounds like it can be done immediately? What do you propose? > Please just do not interfere with my current work, This is not an excellent comment. As you know, Simon has done a lot of work to support SWD using the Versaloon in OpenOCD. We all want the same thing, but the Versaloon firmware has quite different capabilities compared with the FT2232 based adapters which I think you focus on in libswd so you have to be prepared to make changes on your side that will allow more intelligent adapters to be used by OpenOCD, maybe directly without libswd, maybe somehow using only parts of libswd. > we need well planned work not another wave of dirty patches This is also not an excellent comment, and the libswd git repo is also not exactly squeaky clean, if it were then it might well have been merged into openocd.git already. Fact is, Simon does an amazing job maintaining the Versaloon SWD patch for OpenOCD, and it works. I agree that the patch should be split into a number of smaller changes, but this should be the easy part. > - what you proposed is obvious but the problem is somewhere else > when you look at details :-) As long as the change is going in the right direction I don't think it's bad to make it, quite the opposite actually, since it is already finished. > You better tell me Simon how did you manage the mem-ap read retry > on ack==wait, this would help me a lot to clarify that mechanism > and finish the job faster :-) Did you look at the Versaloon firmware? I guess this can be seen there. http://vsprog.googlecode.com/svn/trunk/dongle/firmware/Interfaces/APP/SWD/SWD.c //Peter ------------------------------------------------------------------------------ Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d _______________________________________________ OpenOCD-devel mailing list OpenOCD-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openocd-devel
