hm2_eth: Fix invalid soft error

[BsAtHome]

The hm2_eth driver registers a soft error when using hm2_modbus. This was tracked down to a problem in the hm2_eth driver when non-queued writes precede the first queued read without having had a queued write.

Full details of the problem and analysis in #4322.

The solution uses a flag to ensure that a queued write has set the proper data in the board before its read back value is used in the check.

Fixes #4322.

[hdiethelm]

If it helps, i can test it tomorrow but without modbus.
Otherwhise it looks good, thanks for fixing!

[BsAtHome]

If it helps, i can test it tomorrow but without modbus. Otherwhise it looks good, thanks for fixing!

Thanks, already tested :-)

I have a RPi5 + 7i96s on my desk with a modbus device attached. I wouldn't have been able to reproduce the problem, let alone track the error if I hadn't.

_{BTW, hm2_modbus driver author talking here ;-)}

[grandixximo]

Something else I also noticed, AFAIU the driver counts every write it sends to the board (write_cnt). But only one kind of write (the queued per-cycle one) also stores that count on the board, where it can be read back and compared.

So the check "does the board's copy of the count match mine?" breaks if any other kind of write happens in between: the driver's count goes up by one, the board's copy doesn't, mismatch, false error.

That other kind of write can in theory happen mid-run, e.g. a userspace tool poking the board while threads are active. Then you'd get one bogus soft error even with this PR applied.

A possible fix (if it's even worth fixing) is to keep two counters, count only queued writes in write_cnt, since that's the only count the board ever sees, and if necessary a second direct_write_cnt that counts the non-queued writes for debug/logging.

If agreed this is a real issue, probably better as a follow up, but leaving the decision to you.

[hdiethelm]

If it helps, i can test it tomorrow but without modbus. Otherwhise it looks good, thanks for fixing!

Thanks, already tested :-)

I have a RPi5 + 7i96s on my desk with a modbus device attached. I wouldn't have been able to reproduce the problem, let alone track the error if I hadn't.

BTW, hm2_modbus driver author talking here ;-)

Ah yes, now I see it in the header. Do RPi5 work well with real time? I have one lying around. I could try to get the Xenomai4 kernel working on it... ;-) The only thing missing here is a second mesa card. The only one I have is mounted deep inside the machine. Let's see, would be anyway nice for developing and as a spare.

That other kind of write can in theory happen mid-run, e.g. a userspace tool poking the board while threads are active. Then you'd get one bogus soft error even with this PR applied.

It would be probably be a bad idea to throw some normal reads/writes between the real time cyclic queued read/writes.

[grandixximo]

It would be probably be a bad idea to throw some normal reads/writes between the real time cyclic queued read/writes.

The current bogus errors certainly deterr the idea, silencing them completely would seem like a free pass, and I'm not advocating for that, we could leave them in consciously. I only see the current wording as possibly confusing during debug, a separate count with a clear deterr warning seems more appropriate, but let's see what Bertho thinks...

[BsAtHome]

Something else I also noticed, AFAIU the driver counts every write it sends to the board (write_cnt). But only one kind of write (the queued per-cycle one) also stores that count on the board, where it can be read back and compared.

Yes, it does. Same for the synchronous read, but that you don't detect because it is rewritten in the queued read, which is checked in the second part of the read.

So the check "does the board's copy of the count match mine?" breaks if any other kind of write happens in between: the driver's count goes up by one, the board's copy doesn't, mismatch, false error.

It would be probably be a bad idea to throw some normal reads/writes between the real time cyclic queued read/writes.

If you intersperse queued operation with synchronous operation, then you cannot properly run realtime anymore. Doing so would risk killing your timing (even hm2_modbus uses queued recovery).

The only way you can do so is if a registered hm2 driver calls synchronous functions, and there are (luckily) not many. Normal addf read/write for hm2_eth are the queue wrappers.

If you have code that actually does synchronous calls while in the normal thread cycle, then I think that marking packet errors is not the most obvious thing to do. OTOH, I would consider using a synchronous write while the the thread cycle an error. So, removing it from unqueued write would hide the scenario and keeping it results in a weird error.

[BsAtHome]

Do RPi5 work well with real time? I have one lying around. I could try to get the Xenomai4 kernel working on it... ;-)

These RPi5 boards are really fast. Also the RPi4 (and RPi3) have been successfully used to run RT.
For systems that do not require huge other computing power or other interfacing methods, like 95% of the time, I'd actually suggest a RPi5. It surely beats a normal PC on power and price. The RPi5/7i96s combi + modbus digital I/O box in my modbus test setup uses less than 10W running (without a screen and not counting mill/lath/... stuff).

The only downside I have seen, with all the RPi boards, is that you need to have a good SD card or it will get destroyed (setting noatime is a must). You can simply make a backup copy, but Real Men don't make backups, Real Men cry. ;-)