PCMHammer P04

[antus]

Yeah it certainly been a team effort!

I added some debug code so I could take a look at the counter register in the AMD write routing on P12 and it looked fine. So I tested again and it worked properly. At this point I think I may have run an old kernel or something by accident doing late night development work. I can't reproduce the fault.

So I got this new P12b I've got now and hooked that up. Confirmed it was a 2mb P12b by reading it and tested write, it reads and writes as it should. So that is all the PCMs I currently have in my possession are testing OK.

[Gampy]

Antus, You won't listen privately, so ...

Sorry, I'm blunt, I speak(write) poorly, and I'm definitely inelegant with words, so please bear with me!

I don't even know what to say to you anymore, I was absolutely stunned flat when I read your last PM stating you just stumbled on Mike Morton's M68k article.

I don't know when I first read that article, I do know it was over two years ago, and it could be as much as 5 years.
I have had the original Byte Magazine article (68000 Tricks and Traps by Mike Morton) in PDF form on my documentation screen for the last two years (among others), and I reference them frequently!
Not all PRM's are alike ...

Pretty sure I've pointed this out, but I'm not surprised you missed it, considering the source, me!

I know how align works, I've tried to tell you, and many other things you are just figuring out, that is why I had to trick you into finding out how come it wouldn't align, and yes, I did trick
you into doing that, you wouldn't do it just because I asked you to, and I even pleaded! That's what I call respect! NOT!
I needed a second set of eyes, I knew I was missing something stupid ... Turns out I was!
Although I do admit, I really did think it was VPWReceive, I own that ... However that is irrelevant, it was the alignment I was trying to get you to understand!

Pretty horrible way to have to communicate IMO ...

There are other (better) alignment choices ... However, it's apparent I can't tell (teach) you anything, so I'll waste no more time, and say what I came here to say.

Your Kernel code in your PR is a bomb waiting to explode, I won't even run it as is for there are serious bugs in it, trying to fix them shows it's still on the knife edge ... Sure be nice if we could figure that one out!
BTW, This Knife edge issue, is only with the P04, it does not affect any other units.

Yes, you did find my fault causing the alignment issue.

And I beg to differ, we are no closer to a release then before ...

Sorry, those are the facts as I see them!

I do not code especially for size, I code for consistent, concise, clean, clear, proper and efficient code, styled so it's easily followed without having to look all over the screen to follow the logic!

I currently have a good solid Kernel working on all supported and currently targeted PCM's (A delicate balance I must say). (i.e.: P01, P04 AMD and Intel, P08, P10, P12, P12b, P59 AMD and Intel, E54)
However, as far as the P04 is concerned, it is on the knife edge, and code changes not done correctly will knock it off and break it.
One must understand and be extremely careful!

To me this is unacceptable, IMO it's wrong to consider this public ready, definitely not professional quality, even though it is the best we've got so far.
It does run through my rigorous test course flawlessly, not one single issue on hundreds of Os changes, on 5 different test stations (PC to PCM) and multiple of each PCM except the expensive E54.

-Enjoy

[antus]

The 68k Article, yes I found a clear article so I shared. I was aware of the alignment issues before that, Kur4o described them on this forum, and it had been touched upon before. For everyone else, its an interesting read. http://www.easy68k.com/paulrsm/doc/trick68k.htm

If you want to send a merge request for your code, send it and include a technical description of why you have done it that way. We can debate the details (if we need to) in the merge request. If its clean and stable and it achieves the goals without introducing too much tech debt (my primary concern is added tech debt) I'll probably accept it. I am happy to rebase my other pieces on to that in a new branch and I have a bunch of things I'd still like to add for the next stage - (256kb v6, more OSIDs).

The first time you sent it, it was not on github and you would not describe the design choices or changes present and you told me to read it and learn from it. Unfortunately I am not retired and this is a hobby. I'd rather be working on my cars in my spare time (which I did most of this weekend) but I want to see this code completed. I love to learn but you need to provide a quick summary of what's changed and why. I don't have time to read and consider everything without a description of what I am looking it. That is an inefficient use of my time and I find attempting to do so frustrating. I like a clear goal. It's also quite likely that I'll miss your point if you don't state it then you'll send PMs like the above that still don't expand on the technical discussion I am looking for as frustration rises. You are not good with words - I get that, so be it. There is a lot of emotion in your post. But if we can keep it technical discussing specific aspects of the code or the design choices then that works for me, and I'd much rather look at merge requests, even for work in progress, for the purpose of discussion, when labeled as such than attachments in PMs. We might even attract more developers to the project that way.

I have a list of tasks that I think we need to solve before release, but several items are my own, like work on my bench setup to solve an electrical noise problem because the VPW bus got too long and has too many heads. Also add or make another another test bench for the P10 that has arrived for regression testing on that one. If your working code base is a good step then getting that in is probably the main thing now. Please send the merge request and I'll work on my bench signal quality issue then I'll be able to provide testing of nearly all the supported platforms and interfaces. I wish we could automate this testing, as you know it takes a long time to work through them all for each change now and lots of physical swapping connectors which means it cant all be done remotely (I often work on this away from the test bench). The number of times I've run through it just to find a new issue on a specific platform, add a change, then need to start testing again is a real time killer. I am rapidly loosing the belief that having one code base for all PCMs (especially the older ones) is an achievable goal, or even one that is worth chasing. It sounds like your solution might be the better one.

[Gampy]

Emotions, you bet, I am very passionate about my work, I devote a lot of time to it and take it very seriously, it is me, it's my face, it makes me whom I am.

I'm just trying to get this resolved as well, but you are making statements and changes that are 100% incorrect and make absolutely no sense at all.
My gawd, can't you wait a few months, at least until I'm dead!

When I tell you what you need to do, it is ignored, which is evident in the irrelevant and or incorrect changes you continue to make.

I can't make headway if I have to keep cleaning up behind you and wasting time defending my work all because you do not have time to do your due diligence and homework.
It goes with my mantra, what goes in, comes out, thus garbage in, garbage out, or intelligence in, intelligence out ...

I have spent the last two years fairly steady (some breaks here and there to allow soak in) studying M68k Assembly, I have been through a M68k course, I have worked through Motorola tutorials as well as Intel and AMD materials, I have probably 1000 to 1500 commits of my testing and playing to understand all matters of things involving various parts of this process.

All those commits are me doing my homework, figuring out how things like the DBcc instructions unroll, so I know how to properly and accurately use them.
And I've done this with all the core instructions and some pertinent directives, I do not care about most features, I have zero desires to code for vehicles.
My interests are strictly reading and writing flash.

You say you are an RE, I say I'm a wanna be programmer ... How about we do this.
You do the disassembly work and let me do the assembly work!

You want technical descriptions of what I do and why in English, please do so, let us start with the big white elephant in your work, register d0 and why it all of a sudden broke causing you to not understand why you needed to change it to register d7 ... ??AMD??

Code: Select all

IntelFlashUnLock:
.
. Wacked for brevity
.
    | Enable +12v Vpp
#if defined P04
    clr.w   %d0                        | P04 needs this syntax <<<<<<<<<<<<<<<<<<<<<<<<-- Because you are overwriting it (the count)!
    bset    #VPP_BIT, %d0 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<-- Because you are overwriting it (the count)!
    move.w  %d0, (HARDWARE_IO).w <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<-- Because you are overwriting it (the count)!
#elif defined P08
    bset    #VPP_BIT, (HARDWARE_IO).w  | P08 needs this syntax
#else
    move.w  (HARDWARE_IO).w, %d1       | Move Vpp Latch Address value to d1
    bset    #VPP_BIT, %d1              | Set Vpp Latch Bit
    move.w  %d1, (HARDWARE_IO).w       | Move modified value back to Latch Address
#endif

Code: Select all

IntelFlashLock:
.
. Wacked for brevity
.
    | Disable +12v Vpp
#if defined P04
    clr.w   %d0                        | P04 needs this syntax <<<<<<<<<<<<<<<<<<<<<<<<<-- Because you are overwriting it (the return code)!
    move.w  %d0, (HARDWARE_IO).w <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<-- Because you are overwriting it (the return code)!
#elif defined P08
    bclr    #VPP_BIT, (HARDWARE_IO).w  | Does not work on P01, needed for P08

It's obviously not going to work correctly with the byte count being overwritten between the time it's loaded into the register and used, and the return code cleared ...

As I have stated many times before the P04 does not require that construct in the first place, it works perfectly with the construct under #else.
Thus, changes just to make changes with nothing to back them up ...

Do your homework, RTFM, do proper testing.

Are the following bugs, yes, are they harmful, no, not really, but they are incorrect that's for sure ...
As my comment stated that you removed, there is no need for this sub, it is only needed in one place. You not only removed the comment, you incorrectly followed the clear usability comment in the first place creating one of the next red flags that shows how much homework you have not done.

Code: Select all

LongWaitWithWatchdog:
    movem.l %d5, -(%sp)                | Save d5 <<<<<<<<<<<<<<<<<<<-- RTFM
    move.w  #0x8000, %d1               | 32768 loops <<<<<<<<<<<<<<<-- ONE OFF ERROR (Mostly irrelevant in this case), it's actually 32769 loops, thus poor timing math as is!
LongWaitLoop:
    bsr.w   ResetWatchdog              | Scratch the dog
    bsr.w   WasteTime                  | Twiddle thumbs
    dbf     %d1, LongWaitLoop | If False Decrement and Branch
    movem.l (%sp)+, %d5                | Restore d5 <<<<<<<<<<<<<<<<-- RTFM
    rts

Using hex for counts and indexes is human error prone for one off errors, not to mention most compilers/preprocessors convert them to decimal.
Now I know this isn't harmful, my point is, I have fixed multiples of your one off bugs due to this, that and alignment is also the original works main downfall!

Again, changes just to make changes with nothing to back them up ...

Code: Select all

IntelEraseSector:
.
. Wacked for brevity
.
IntelEraseSectorNotReady:
    bsr.w   LongWaitWithWatchdog <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<-- Poor design!
    move.w  #READ_STATUS_REGISTER, (%a0)  | 0x7070 Read Status Register <<<<<<<<<<<<<<<<<<<-- RTFM
    move.w  (%a0), %d1                    | Move Status to d1
    btst    #0x007, %d1                   | Test Status
    bne.s   IntelEraseSectorReady         | Status Success - Jump to IntelEraseSectorReady
    subq.l  #1, %d2                       | Decrement index
    bne.s   IntelEraseSectorNotReady      | Fail Status Check Test again

First off, it's a Wait For loop, having a delay in a wait for loop is poor design, increase the timeout to accommodate, and allow it to finish according to the performance of the current unit.

I was taught that delays are typically the sign of poor code design, split the process up with other processes in between them ... however, sometimes that is not possible and delays are required.

In this case, A delay is required after enabling Vpp, and ONLY WHEN ENABLING IT ... Therefore the sub LongWaitWithWatchdog is nothing more then bloat!

Again, well proven unnecessary changes, plus logically unnecessary.

Code: Select all

IntelFlashLock:
.
. Wacked for brevity
.
#endif

    bsr.w   LongWaitWithWatchdog <<<<<<<<<<<<<<<<<<<<<-- It's done, this does nothing but slow the process down for zero reasons!
    rts

Again, making changes just to make changes with nothing to back them up ...

-Enjoy

[Tazzi]

P04's are not all the same.
You could have 3 different P04's there, and it wont match someone elses.

In fact, even the operating system run on the P04's affects how a kernel reacts. Disabling interrupts SHOULD prevent this from happening, but there are other silicon based settings and options that can still be running in the background that will affect how a kernel will operate when running.

Pete and I were dumb founded that we could swap ECU OS's and the VPW timing would literally change between the two, and so would the time it took to read/write. One of the OS's (On both units when switched) would also occasionally fail to erase... which leads me to the entire point of this message.

Adding larger delays after setting VPP or unlocking the flash is better then trying to optimize it for as fast as possible. By extending the time on both of these, I fixed the issue I was getting with erasing, and it all came down to differences in the background operating system of the P04, not the actual hardware.

Id recommend leaving extending timings for those, there is nothing being harmed in doing so other then allowing for a buffer zone for OS/hardware that may be slower.

[antus]

Thanks, both.

I can't make headway if I have to keep cleaning up behind you and wasting time defending my work all because you do not have time to do your due diligence and homework.

This is the difference between an RE and a programmer. My process is that I take a selected reference point from my branch in git, and I apply changes and observe the result. If it still doesn't work and I am still learning I make more changes and try and resolve the issue. If I do resolve the issue I copy the branch locally, then I use the diff tool meld to cherry pick changes and try and get the minimal set of changes back in to have it work. By this stage it should be clear why it was improved, though with undocumented hardware and some unexplained differences in the hardware/os this is not always the case. Then when I am ready to call it done I will review the changes again and fix up any loose ends, comments etc. I did read all the comments but to re-trace your steps entirely nothing new will be found. I likely removed the comment when I got lucky with something undocumented in the P04 and the effect was not from the cause I expected. Work in progress, cleanup when understood.

You don't have to nor should you clean up behind me. I guess I have left that merge request open for a long time on this branch with just a comment that it still needs work. I am partly tempted to go and close it but it does provide a single place to see the net changes between a branch and develop. In my day job work in progress merge requests are often used to see the net sum of changes on a branch and to get feedback before the code is ready.

antus/p08 is an experiment, and is a work in progress. It has only been 50% successful and has uncovered more problems. At this point, if you have a branch that implements different kernels for different P04s (hopefully from a single Kernel.S still) I would rather go to that branch and then cherry pick from mine, if there is any value there at all. Though before I give up on it now, while I wait for your branch if you say you can submit it, I will check the things you have flagged above.

As Tazzi mentioned, the delays are in the search of stability. Definitely the same code doesn't work on all PCMs, we know that. The angle is that I am testing if more delay will make it stable on more platforms. As above when it works they can be taken out again for another round of testing and see if all platforms work without it or not. Once we have the minimal amount of delays that work on all platforms, I would document which platform needs what for maintainability so that no future developer takes one of them out and breaks an obscure platform they don't own and can't test.

I'll look at that d0 vs d5 usage in the lock routines. I've obviously wasted more than a few nights chasing my tail on that one.

Let me know if you have a branch you'd like to submit that is working properly on all platforms, or just send the merge request. I'd rather accept that than duplicate work cleaning up antus/p08. I'd love to see P04, 95/96 V6 and P08 working properly plus the platforms we are flashing already with the C kernel.

Then we can stop adding platforms for a while and I'd like to start looking at the app structure internally and its data handing. I think we've outgrown that mega case statement, and a lot of that data should be stored against PCM type, not OSID.
I think the UI could use another look too. Now that we support OSs that dont respect the param block (95/96 V6), and we have targets where we cant flash the slave (P10, P12, P12b) I think we need to stop naming sector purpose against flash chip, or need something to abstract it so that the type is decided on a combination of PCM and flash chip together. But this is later after the kernel is good.

[Gampy]

Thank you for your response and confirming exactly what I predicted your response to be.

You asked me for English explanations, and you respond with excuses.

I agree, I shouldn't have to clean up after anybody, but that is whom I am, when there is a mess I clean it up, and I don't sweep it under the carpet when no one is looking!

I think my message was crystal clear pertaining to the issues I described.

We are worlds apart, you are the type that does things in a haphazzard random manner with the hope that some amount of success may be encountered, the problem with your technique as a programmer is most of the stuff you throw makes a mess and affects the whole chain.

I am a person that thinks logically and works through issues with logic.

A PR (Pull Request) is a request to the maintainer of the project that you have a COMPLETED feature ready to be merged into the project, it is not a request for development.
When I look at your PR, I see a mess that should not be merged, and that is next to impossible to properly vet, I pointed out some of the issues and you respond with more excuses as to why you have created the issues.

At your request, we had already agreed not to add any more PCM's until we were caught up, and now you've broken that agreement (someone gifted you some PCM's) and have added another target.
One I have already done most of, but not added, because of our agreement, and don't try to explain it as it's a P04, I suspect it is probably not, doesn't matter, we agreed no more until we caught up. PERIOD!

We have the same goal, I just cannot figure out how we can work together, you're wasting my time I don't have, you say you don't have time, the hell you don't you have a lifetime of time, I on the other hand do not, I'll be dead within months, maybe weeks if not days ... I need to get this Kernel done (Working properly), I cannot afford constantly cleaning up messes and defending my work.

If you were truly making progress there would be ZERO issues, but your not, you are not accomplishing anything other than your own education at my expense. And you won't allow me to help bring you up to speed faster.

I on the the other hand am making progress, and yes, I will push a PR when I have done my due diligence, not until, so don't waste your time asking, I will do it when I am ready, hopefully before I am dead!
But if not, it'll still get pushed by those that I have prepared to do so ...

I'm done with this conversation, I've presented the facts and continuance is pointless.

-Enjoy

[antus]

I'll wait for your PR and stop developing my branch.

I will add that git is a tool, and you can use it however you want. There is gitflow which does not include work in progress commits, but it is not required and we have not agreed to do gitflow. Work in progress commits are a thing in many projects, organizations and git workflows including at my employer. I am not the worlds best coder, nor do I claim to be, but I often get results with my methods where others cannot.

[Tazzi]

For an application being done in everyones spare time... there appears to be alot of argument about formatting or testing.

If code is not being committed regularly to git, theres no way for others to check or overlook for problems.
The only way they can progress is by pushing forward on their own, I see Antus has done exactly that.

If its that much of an issue.. Fork the project and host/commit/run it however you like. Easy solution.

[Gampy]

I know I said I wasn't gong to continue this conversation, however I feel there needs to be some clarity.

Tazzi,
Please, don't try and deflect the facts to benign reasons and poor paths forward.

As I have always said, I am here for PCM Hammer (and to keep my mind active), that is all, I'm not here to make friends or learn how to do this stuff for any other reason then PCM Hammer (and to keep my mind active), what good is it going to do me as worm food, it's not like it's going to make the worms smarter!

Making friends is a bonus.

If I did my own thing in my fork, it would only hurt PCMHammer, and 99.9% of the changes would be incorporated into PCMHammer anyways, except for the industry standard professional words I use like VCI (Vehicle Communication Interface) over cable, tool, device, or any of the benign over used words that are completely unclear as to what they really are.

but I often get results with my methods where others cannot.

I will whole heartily agree with that, you are an excellent RE, something I do respect you very much for!
That is the exact reason I come to you for that sort of info or knowledge, I'm no electronics RE, and I'm nowhere near as good as you at disassembling binaries, I can do it, it takes me 10 times longer, it's not something I do, it's not my idea of fun, as it is to you ...
My father taught me if you want to learn something right, pick the best and learn from them! I feel I made the right choice on the RE topic!

I have NEVER said you were a bad programmer, NEVER!, what I have been saying is that you have been arguing with me (and ignoring me) over things that you clearly have not even read the manuals on.
That was my only real issue ... PERIOD!

I personally feel we have become pretty close, closer then any other internet person I communicate with that is for sure!
I believe you've shared things with me that not everyone knows, in my case only immediate family knows a lot of the things I have shared with you.
I'm not a person that makes friends easy, I am picky whom I allow into my inner circle, extremely picky, I don't associate with just anybody.

That is the only reason I've spent ANY time whatsoever trying to work through this!

-Enjoy