Zach Welch wrote: > On Fri, 2009-05-08 at 13:49 +0200, Magnus Lundin wrote: > >> I'll give it a go, and I do think I understand what is going on under >> the hood in some detail. >> > > Thank you for providing your explanations. This has been the most > constructive attempt to address the technical issues, and I appreciate > your focusing on it from this perspective. > > >> Also there are a lot of cleanup to be done. >> >> And the documentation about some finer technical points is somewhere >> between bad, nonexsistant or can be found in Dominics thesis paper. >> > > Which was last updated when? I will make it a priority to grok this > document and regurgitate anything useful that I find to the list, but I > just want to point out that the lack of up-to-date backing documentation > raises questions about the objectivity of anyone's current opinions. > I believe internals should be fair game for being rewritten at any time. > > >> Zach Welch wrote: >> > [snip] > >>> * Are you _certain_ the current bugs being experienced cannot be the >>> result of other bugs that were uncovered by the removal of the >>> in_handler code? If they are, Øyvind just did us a service. >>> >>> >> The bugs I have seen have all been caused by pushing unteted code into >> hede. Basically to quick and not careful enough. There were some ideas >> but this was not a carefull thought out architectural resturcturing. I >> hope to show that the old code is definetly a carefully planned archtecture. >> >> The architecture he proposes will work if cleanly implementd. >> > > You are saying his ideas can be implemented successfully? Okay... the > only problem here is that you introduced ambiguity in the pronoun 'he'. > I can read your reply such that it refers to either Dominic or Øyvind; > your intention is not perfectly clear from the context. > > Wrong reading, I am saying that Övinds ideas, as I interpret them can be implemented and made to work. Problem is that they lack more precise substance than : We shall mkove things and it will became better, cleaner and faster. He does not state how to do it or in any detal why it will be faster, but he is given the benfit of a doubt >>> * Are you _positive_ the "lost performance" will not be regained by any >>> other means? If it can, then that argument is insufficient. >>> >>> >> It might be regained, but in that case I am positive that we we have >> either built the same asych callback functionality somwhere else, or >> trimmed the USB layer, in which case even more performance can be gained >> by reverting. >> > > I expect the functionality to be re-implemented in a way that the > community will support, even if that means adding back nearly identical > code. If I were in Øyvind's shoes, I would rather see the in_handler > functionality re-implemented cleanly than just put it back as it was. > True, but nobody has argued against reimplementing cleanly, but that is not retiring functionality and changing the API.
>>> * How _exactly_ does the in_handler callback fit into it? Again, this >>> needs to be explained in fairly complete technical detail. Or maybe >>> someone can link to the on-line documentation.... >>> >>> >> Actually we are talking about two different functionalities here. >> >> - The handling of asychronous reads from the jtag, and the extra data >> treansformation done by the in_handler funtions. >> >> In order to issue several read requests where the result should be >> stored in different locations and avoid doing a full roundtrip for each >> call, the address of the receiving location is stored in the in_variable >> field when issuing a scan request to the jtag layer. When a >> jtag_execute_que() is completed the all oustanding such queued reads are >> completed. >> So the calling sequence is >> - issue as many in scans as possible, >> - when we need the return walue, call jtag_execute_queue(), if this >> succeds we are guaranteed that the values are in place. >> > > I follow you so far. We want to decouple reads of results from the > writes, so both can happen asynchronously. jtag_execute_queue() is a > sync point that ensures all queued write/read pairs have completed. > > >> This introduces the potential of receiving value going out of scope, >> this happens if we ask for a value to be stored in a local variable and >> exit before calling jtag_execute_scan, which is a type of an "unused >> variable assignment" >> > > I am okay up until ", which is...". The last phrase is confusing. > > If you ask the jtag layer for a variable, it should be be used before going out of scope, otherwise it is a type of unused varable assignment problem. >> The natural place for this functionality is in the jtag layer. >> > > This is not entire clear just yet, but I will assume this for now since > you seem to be mostly on-track so far. > > >> The in_handler functionality tells the jtag layer to apply an in_handler >> supplied with the inscan command to the received value before storing >> it in the receiving location. This transformation is something that must >> be done somewhere to adaptet the received jtag data to the data format >> of the receiving variable. So some part of the code must handle this >> transformation. And it must be done after receiving the jtag in data and >> before the result is used. Possible choises are jtag or target variant >> specific code. >> > > Right: in_handler is/was a scan-specific "value fix-up" callback. > > >> Prime example of this is indata from ARM7 scanchain 1. This data is >> bitswapped. Something users that target read register funtions are not >> aware of. So the issuer of the Scanchain 1 scan structure tells the >> jtag layer about this by adding the relevant function to the in_handler >> field. When the data is received the code has long since move away form >> this function and we are in the upper layer function that simply >> expects a u32 value. >> > > Okay. This upper layer that expects the u32... how do the functions > that use this value and the functions that setup the scan to retrieve it > relate to each other? Specifically, are those functions that setup > scans and those that use the results tightly coupled together? > The upper layer users are general functions that does things like calls to target->type->read_memory(), The scan setup comes from calling the exact architecture specific calls that can read from a certain target. for ARM7TDMI the call chain is as follows arm7_9_read_memory() -> arm7_9->load_word_regs() -> arm7tdmi_load_word_regs() -> arm7tdmi_clock_out(). Here in arm7tdmi_clock_out(). the in_handler is set to flip the bits and then a dr scan is added > To start, I will assume they are. If this is the case, the situation > with in_handler seemed to be this: > > * We want a u32, so setup a scan to get it, with an in_handler to fix > the data when it comes in. > * At some point, jtag_execute_queue() will retrieve this value and call > the in_handler to convert it. > * Later yet, the function that needs that value uses it. > > Likewise, the new implementation may simply look like: > > * We want a u32, so setup a scan to get it. > * At some point, jtag_execute_queue() will retrieve this value. > * Layer yet, the function that needs that value can ask to retrieve it. > * A helper function takes the former in_handler and lazily converts it. > > You ar right that we can do lazy conversions we have thrown away the information about which handler to use. The upper layer does not know which handler to call without doing some target specific look up of which handler to use. Basically going down the call chain to look this up again. > Now, one might assume the opposite: that the scan setup/usage functions > are not as tightly coupled. If that is the case, then your very example > seems to be a layering violation: you want target-specific fix-ups to be > processed by the jtag layer? > > Am I thoroughly confused here, or is this a correct interpretation of > the situation? > > Since target debug unit uses diffent mappings between scanchain bits and the memory data we are trying to read it is tightly coupled. >>> * Where are the patches to finish implementing this work? My impression >>> has been that this feature is mostly "for future use", and that is not a >>> sufficient reason to keep it -- unless it is being actively worked on. >>> >>> >>> >> The stuff above is finished, it has been used for a long time, cleanup >> and polishing is always good but this is a core functionlity that is >> working. >> > > Right. I do agree that future changes of this magnitude should be > completed in advance of being committed to the head of the trunk. > I think r1607 should have been floated for at least 24 hours before > further work was done, and Øyvind might have been wise to stop until the > community was satisfied with the path forward. > > I apologize to everyone that finds this difficult, but breakage _should_ > occasionally happen in the trunk or we are being too conservative in our > development practices. [[Not this much breakage though!]] Still, it > helps to have clear plans in the public view for a decent amount of time > for consideration, then one can just say "I tried to warn you". > > I have a series of big patches pending, but they should not break > anything for anyone. Even in my completely non-functional changes, > there will be room for controversy though, so stay tuned. :) > > >>> Show me the "lost work" by answering the above, and I will defend you. >>> If this is not forthcoming, then it is in our interest to continue on >>> the current path, despite any further objections. >>> >>> >>> >> Hope this explains the background. >> > > It does provide more context, but I do not think that it explains why > the implementation cannot be improved. The resistance to change in this > community worries me. The code is not in _any_ shape to be "preserved" > in its current state (unless we are talking about archiving backups). > OpenOCD needs to grow and evolve or it will not survive. > > If change is painful, then evolution is an outright killer. > > C'est la vie, > > Zach > > I am tired of hearing this argument about resisting change. It is simply not true. I hear arguments about improving architecture, great but then there should be an new architectural plan at least for the affected parts. But changes done without understanding the code is not evolution. Big experiments should be done as experiments and then evaluated. Regards Magnus _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
