[LinuxCNC/linuxcnc PR#48] PktUART driver for MESA

#1 – SebKuzminsky 于 2016-03-26

Hi Boris, thanks for the PR.

Is there a matching branch for the hostmot2-firmware repo that implements the FPGA side of this?

#2 – sirop 于 2016-03-26

I used 6i25 in 5i25 mode with 7i75 as a daughter board.

For the test component one needs only one PktUART instance, the instance stride is the “usual” one:
0x4 as opposed to the instance stride of UART which is 0x10.

So what shall I do? Upload TopPCIHostMot2.vhd, 5i25.ucf and the pin out file?
To src/hal/drivers/mesa-hostmot2/firmware/5i25 ?

#3 – SebKuzminsky 于 2016-03-26

I just pushed our hostmot2-firmware to a new github repo, please make a PR there: https://github.com/LinuxCNC/hostmot2-firmware

Also, can you give a general description of what this PR does, what the use cases are, and why it’s a good idea? Help us understand what it’s about.

#4 – sirop 于 2016-03-26

Updated my first comment.

Shall I first prove that the build process as described in https://github.com/LinuxCNC/hostmot2-firmware can be reproduced for my pin out file?
As I simply used Xilinx ISE 14.7 WebPack to make a bit file…

#5 – jepler 于 2016-03-27

My review is more “quick” than “thorough”, but I didn’t identify any big issues with the code. I did attach a few notes as you can see above in this issue. Thank you in particular for providing manpage docuentation for the new APIs!

I don’t think we need to get a particular ordering between adding this to linuxcnc and adding the supporting code to hostmot2-firmware, but it would be best to have both in the end.

#6 – jepler 于 2016-04-23

@sirop thanks for addressing my review comments. I am content with the revised version.
@SebKuzminsky are we putting this in 2.7 or master?

#7 – sirop 于 2016-05-10

https://github.com/LinuxCNC/linuxcnc/pull/48/commits/59847ca427716c23fae63cf331d3040eb82d666a:
A closing if-bracket came too late.
See also https://forum.linuxcnc.org/forum/24-hal-components/30262-reading-uart-rx-count-register-on-mesa-5i25-6i25?start=20#74416

#8 – jepler 于 2016-05-10

I just noticed that the commits on your branch do not follow our policy. You must provide a signed-off-by line in the commit message, to certify that you are submitting your patches under our license, GPLv2+. For more information, http://linuxcnc.org/docs/2.7/html/code/contributing-to-linuxcnc.html#signedoffbypolicy

If you need a hand doing this (the way I do it involves using “git rebase”), please let me know and I’ll walk you through it.

After you’ve done this I don’t see anything else stopping us from accepting this in our master branch.

Thank you for your time on this!

#9 – sirop 于 2016-05-10

Yes, I’d like to know if there is an elegant way to add a signed-off-by line to each commit of mine.

#10 – jepler 于 2016-05-10

I use something like this:

git rebase -i --exec="git commit --amend -CHEAD -s" origin/master

which assumes that you call linuxcnc.org’s master branch origin/master.

In your next push to sirop:PktUART_LinuxCNC you will expect to have to use “git push -f”, because you have rewritten history with “git rebase”.

#11 – sirop 于 2016-05-10

Ok, it was git rebase -i HEAD~3 --exec="git commit --amend -CHEAD -s" PktUART_LinuxCNC .
with git 2.6.2 .

#12 – andypugh 于 2023-03-08

I noticed that there seems to be an error in this driver.
“`
// Test the exported hm2getpktuart function
retval = hm2getpktuart(&hm2, name);
if (retval!=0)
{
rtapiprintmsg(RTAPIMSGERR, "%s hm2getpktuart: smth. is wrong. \n", name);
return -1;
}
“
The hm2getpktuart() function returns the index of the found pktuart (not for any good reason) or -1 on faiilure. So the driver here can only ever work with pktuart number 0. (which is probably all that it was tested with?)
Do you see any reason not switch the condition to:
“
if (retval < 0) ```

#13 - sirop 于 2023-03-09

@andypugh
You are right, https://github.com/LinuxCNC/linuxcnc/blob/9e28b3d8fe23fff0e08fc0f8d232c96be04404a6/src/hal/components/mesapktgyrotest.comp was tested only with pktuart number 0 as this component was rather a test example for the underlying functions.

So I do not mind if someone changes/rewrite this comp.

#14 - petterreinholdtsen 于 2023-03-09

[Boris Skegin]
> @andypugh You are right,
> https://github.com/LinuxCNC/linuxcnc/blob/9e28b3d8fe23fff0e08fc0f8d232c96be04404a6/src/hal/components/mesapktgyrotest.comp
> was tested only with pktuart number 0 as this component was rather a
> test example for the underlying functions.
>
> So I do not mind if someone changes/rewrite this comp.

Perhaps someone could write a script in tests/ to detect this problem
and ensure the component work? I understand this can be prohibiting
hard if special hardware is needed.

--
Happy hacking
Petter Reinholdtsen

#15 - andypugh 于 2023-03-09

PktUART is extremely niche. It can't be used at all without writing your own driver, and this error was in a sample/test driver for a specific piece of hardware.
Even then, it requires a Mesa card with a firmware supporting multiple PktUART instances.

In this case, it's really not worth testing.