raspberrypi / linux

Kernel source tree for Raspberry Pi-provided kernel builds. Issues unrelated to the linux kernel should be posted on the community forum at https://forums.raspberrypi.com/
Other
11k stars 4.95k forks source link

CM4 is missing IEEE1588-2008 support through BCM54210PE #4151

Open tschiemer opened 3 years ago

tschiemer commented 3 years ago

Is this the right place for my bug report? Yes: I asked on the forum, I asked the raspberry foundation through the contact form but didn't get any response - it is very much kernel related as is explained in the forum post.

Describe the bug The CM4 has an undocumented inofficial PHY (BCM54210PE) that supports hardware timestamping (and a hardware clock, actually??), but there seems to be no driver supporting these features.

So CM4 does actually not provide hardware based IEEE1588-2008 support as opposed to what the raspberry pi foundation actually communicates in the CM4 datasheet.

To reproduce sudo ethtool -T eth0 only shows software timestamping capabilities.

Expected behaviour Depends - the raspberry pi foundation never communicated what IEEE1588-2008 features are supported such that is is unclear what exactly can be expected; also the PHY documentation is not available such that possibilities (ie optimal expected behaviour) can not be described.

Expected (according to https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/s1-using_ptp):

The CM4 datasheet lists SYNC_IN, SYNC_OUT pins ment for IEEE1588-2008 support but has not further specified this. It actually is a question wether these pins are needed in the first place - because the PHY would seem to have a hardware-clock itself. Further it must be clear wether the PHY's internal clock (if any) is routed to a CM4 pin in any form and if not this should be fixed in future revisions because custom boards for the CM4 might require tight synchronization to a hardware clock.

Actual behaviour sudo ethtool -T eth0 only shows software timestamping capabilities. There is no (documented) synchronizable IEEE1588-2008 clock signal going from the CM4.

System Raspberry Pi Compute Module 4

Raspberry Pi OS Lite, January 11th 2021, Kernel version: 5.4 Ubuntu Server 20.04.2 LTS,

Logs none

Additional context BCM54210PE documentation is required to implement to see actual features provided and configuration options - IEEE802.3-2018 seems to define said capabilities etc. Possibly also required are CM4 schematics and BCM2711 documentation. Also see IEEE802.3-2018 Section 90. Ethernet support for time synchronization protocols

I can try to take care of the implementation, but I would like to have the mentioned documents.

marcohabeck commented 2 years ago

https://www.microchip.com/en-us/product/LAN9360

PHY with AVB support. But is also under NDA. Here you could build a board for the RPI.

ghollingworth commented 2 years ago

There is no issue with releasing the code we have already placed into these sources. The software license that both Raspberry Pi and @cfdez-tech have with Broadcom allows us to use the information to write open-sourced drivers.

KyleJudd commented 2 years ago

Thans JamesH65. How do you think we should proceed? Are there formal request that can be made? The missing info we need is really in Broadcom documentation. I know Raspberry Pi and Broadcom are separate entities, but I am sure you guys know each other. I am sure most people in this discussion understand that people at Raspberry Pi and Broadcom intend to be helpful in the products they make, and we are only asking for a little help consistent with that spirit.

ghollingworth commented 2 years ago

What is the missing information?

KyleJudd commented 2 years ago

I think Carlos' code was using the wrong register for the running time. The register is accurate, but slow. Other older bcm chips (with public documentation and no NDA) have a heartbeat register but also what's called EAV registers and modes. It seems EAV was a predecessor to AVB, and those registers (in older chips) do exactly what we need. Does anybody know about EAV registers, or perhaps BCM has changed the name? There may be modes to enable these registers, etc. That's what we are missing.

KyleJudd commented 2 years ago

P.S. Thanks ghollingworth I believe you, and if I can confirm what you are saying is true, I'd be happy to create a public repository for all to see. But I still think there may be register/mode info that would help us (EAV / see above).

ydirson commented 2 years ago

I'm not familiar with the Maaxboard/iMX8 stuff. What nic/mac/phy do they use? I didn't see that info on there product page.

It's using a stmmac, which has hwts support in the linux driver. According to the Hardware Manual, the PHY is an Atheros AR8035.

KyleJudd commented 2 years ago

To elaborate on the register problem, what I see on my CM4 is that the registers used to get the hardware timestamps for PTP SYNC/PDRQ messages works correctly. If you use these timestamps in communications with AVB switches (ExtremeNetworks, MOTU, etc.) these switches will report peer delay values of about 200 ns, which seems correct or at least good enough for these switches to "open the gate" for other necessary AVB protocols. I believe that much is working properly.

The problem is that when you use gettime to get the current time, unrelated to PTP messages, it takes up to 4 milliseconds for the device to respond, which is no better than using software timestamps. Thus, in certain software calculations, I get the effect of software timestamping, not because the PHY isn't working, but because gettime is taking so long to respond. I don't have this problem when using the i210. In the past, I've used the same software calculations with other BCM products, and did not have the problem, and those chips worked as well as the i210. But I wasn't using the heart-beat register for those bcm chips, I was using another set of registers to get the current time.

I suspect there is a way to do with this bcm chip what can be done with other bcm chips, but the documentation for that isn't public yet. Everything I know about this is based on public documentation still available from Broadcoms own website.

jdimpson commented 2 years ago

TSN support is still a long way off. Except for a few experiments, I don't see any projects that would be really viable. Especially since this support has to be in the kernel. This has been included in macOS since 2013, and little has changed under Linux for years.

I don't disagree with this assessment, but in hopes of improving that outlook by increasing awareness, there is some amount of TSN support in the OpenIL (industrial linux) project. It's been too complicated for me to have figured out how much functionality is in the kernel vs in hardware, both in general for TSN and in particular for this implementation, which AFAIK is only functional for NXP hardware and thus is not helpful here.

https://github.com/openil/tsntool

marcohabeck commented 2 years ago

A variety of things are necessary, some of which have to run in the kernel and in userspace. All time critical parts must be in the kernel. IEEE1722.1, PTP,SRP,Qos(PHY) for control can run in userspace. The stream data must also be processed and, if necessary, read or output on hardware.

As long as the kernel developers do not seriously discover the topic of TSN, many individual projects will remain. All of which work more or less poorly. It is also due to the fact that such things are usually implemented in FPGAs. You can buy these solutions ready-made and only need the Linux board to control the components.

Most products work the same way. The time-critical part runs in an FPGA, the embedded board controls it via an interface. You need a PTP capable PHY and switch. Marvell offers everything together, Switch Core, ARM including PTP enabled PHY and Stream Core. Of course only with NDA, manufacturers make the topic a secret. The reason is the strong focus on industry. You can earn money with this and it also has the ability to write information from the NDA driver.

you can see that in the AVB support of the switches. There are no affordable switches with more than 5 or 6 ports. Motu etc. all have the same Marvell chip inside. If you need a big switch it gets really expensive and you get bad AVB support.

KyleJudd commented 2 years ago

thanks marcohabeck. I think your explanation is accurate and helpful to those who want to learn how to write AVB software. Much of the open-source work available was written for demo purposes and isn't retail ready yet. So I took the time to hand-roll my own code, test it with real-world equipment from EN, MOTU, PreSonus, Apple and others. Each of those devices has their own quirks that have to be dealt with as well.

I've spent a lot of time on my software only implementation, and it works with the equipment I've mentioned. It works with several Intel i-series nics It even works with a different bcm card, and maybe one I forgot :) It works on a CM4, with a i-210. It just doesn't work YET with the stock CM4 nic, and that is just a shame. Everything you said above is true, but I/we are so close to changing that.

But you are also right that these companies like Broadcom and Intel supply the companies that make AVB switches, and there may be existing contracts that prevent these companies from open-sourcing too much of their product. I respect that.

But there are other ways to skin this cat. I never planned on polling the nic 8000 times per second, it was just a quick proof of concept that happened to work with the i210. I'm sure I'll get my thing to do what I want it to do :)

neggles commented 2 years ago

Guys, you're not going to get in any trouble for releasing an open-source driver, especially not one that's been written without access to the documentation. It's something that has to happen eventually for the spec sheet to be telling the truth, and I'd like to believe the Pi Foundation would've removed it from said specs already if it can't be done.

Whether HW timestamping alone is adequate for proper TSN/AVB/etc support doesn't matter - there are plenty of use cases where HW timestamping alone is all that's necessary (e.g. low cost PTPv2 grandmaster for AES67 & LTE/NR small cells).

Half the reason I bought a CM4 in the first place was the HW timestamping support called out in the datasheet; it has largely sat and gathered dust since I completed the IO board fan controller driver, waiting for HW TS support to materialise.

Regardless of whether it works particularly well, it works; once source for a functional driver is out there, someone will pick it up and experiment and improve it - that's the point of open source! None of us is as good of a programmer as all of us.

But as @cfdez-tech has now pulled down their repo, there's nothing for any of us to work with - even if I wanted to do it myself, without a datasheet I can't get or a halfway-functional driver to start from, I doubt I'll get anywhere.

To be clear, I'm not blaming anyone in particular, certainly nobody in this thread - I understand why they've made the decisions they have - but this whole situation just doesn't feel great.


[edit]: I guess I'm just really sick and tired of things being hidden behind NDAs that vendors won't let an individual sign, won't even detail the criteria to be allowed the privilege - and for what? What could possibly be in an ethernet PHY datasheet / programming guide that's so critically important to hide from the competition?

Broadcom are not the only vendor which is quite happy to make money from the open source community without actually participating with it in a constructive fashion, Marvell and Qualcomm are just as bad, but, still.

Also, the AR8035 explicitly does not have hardware timestamping in the PHY. The AR8031 does, and the 8033 appears to have it as well despite the spec sheet saying otherwise, but the previously mentioned i.MX board does not have PHY timestamping. The MAC in several of the i.MX chips has HWTS, but that's not nearly as accurate as PHY timestamping without a lot of calibration.

Sorry for the rant.

KyleJudd commented 2 years ago

I hear your frustration neg2led. I feel the same way, but I haven't stopped working on this either. It may take a day or two to resolve some of the concerns, but I think it will happen. I know some of the performance concerns I have don't apply to what others intend to do with PTP, so I intend to share my solution even if we don't find ways to improve that issue.

The only remaining issue is that I haven't seen any documentation that shows that the register information we have was publicly released for open-source purposes. ghollingworth stated above that he believes it was. Perhaps he or someone else is trying to gather that information as we speak.

Just because something wasn't released yesterday doesn't mean it can't be released tomorrow. Its Friday night and I think the people who make those decisions have stopped work for the day. I think our best option is to remain patient and polite.

I'm still looking for other places where this code may have been released, and if I can find that, my concerns will be resolved.

Its funny how the biggest problem with AVB is that it really isn't very open-source at all, is it? :)

KyleJudd commented 2 years ago

I take that back. The AVNU people have been very helpful to me. I would point out to any AVB manufacturer that if we could go to Best Buy and buy consumer grade AVB speakers and consumer grade AVB media players, they would sell a whole lot more AVB switches. I believe there should be an AVB switch in every household, but who needs and AVB switch if there is nothing to plug into it?

Being able to plug a Raspberry Pi into an AVB switch would increase the sales of AVB switches. That's the point that Broadcom and other companies need to understand. If you want to sell a million AVB switches, you'll also need about a million AVB audio sources, and about a million AVB speakers. The Raspberry Pi is an excellent way for people to learn about AVB/TSN/PTP etc, and it is likely these people who would make the speakers and media players that plug into the AVB switches. Providing PTP/AVB support on an open platform like Raspberry Pi would only increase the marketplace for AVB products.

neggles commented 2 years ago

The only remaining issue is that I haven't seen any documentation that shows that the register information we have was publicly released for open-source purposes. ghollingworth stated above that he believes it was. Perhaps he or someone else is trying to gather that information as we speak.

The register maps and their details are definitely not public information, or there would be a datasheet / programming guide available publicly - but that is not relevant to whether an open-source driver can be released. Quite a few of the PHYs supported in the kernel have no publicly available register maps / datasheets; for example, the BCM54210E (this PHY's cousin from the Pi 4), and the Marvell/Aquantia AQR1 series both have little to no public information available, and the AR803x series only very recently became public IIRC.

Whether the full details of the register map etc. are available publicly or not, it is impossible to write an open-source driver without revealing at least some details of what the registers do. That's why I find Broadcom et. al.'s approach to be so intensely frustrating. There is no way to keep information about how to interface with a device secret if it's supported in the mainline Linux kernel, and as @ghollingworth pointed out:

There is no issue with releasing the code we have already placed into these sources. The software license that both Raspberry Pi and @cfdez-tech have with Broadcom allows us to use the information to write open-sourced drivers.

Draw your own conclusion, I guess 😝

ghollingworth commented 2 years ago

FFS... As Chief Product Officer of Raspberry Pi, and an acting director for the company, I have read and signed (and made Broadcom modify) our software license agreement which allows me (or anyone at Raspberry Pi) to take the information contained within their document and develop open-source drivers.

I cannot show you the contract, because it's also an NDA and I cannot show you the original document (unless you work for Raspberry Pi or are a contractor for us). If anyone else questions whether I am allowed to release the information, I will delete their messages

You've been warned...

On other things, if someone would like to develop a driver under contract for me, then I'm sure we can work something out. Then I can provide you the documentation...

jwbowen commented 2 years ago

Oh yay, frustration and threats toward people who want to make use of your product. How very endearing.

pelwell commented 2 years ago

A threat to delete misinformation is not a threat - it's a public service.

KyleJudd commented 2 years ago

My bad ghollingworth, I just didn't understand who you were. I was just trying to err on the side of caution. I'll start putting together a public repository where others can view my changes. It may take a day or two.

KyleJudd commented 2 years ago

OK, Game On ...

I created my forked-repository, checked in my code, pushed it up, cloned it down, compiled it, tested it... and it still works.

I intended to keep the repo private until later tonight so that I can clean the code a bit and do more testing, but it seems I cannot make a fork private as I intended. I'd like to test it with other PTP apps before other people really dig into it, but for those people who must absolutely have it now:

https://github.com/KyleJudd/linux/tree/cm4-ptp-kylejudd

I only compile and test it locally in 64 bit mode using the instructions : https://www.raspberrypi.com/documentation/computers/linux_kernel.html

Remember that you have to do 'make menuconfig' to enable Phy Timestamping, PTP and the 54210PE Phy driver. I can make clear instructions for that later, but I do not have time to do that right now.

Thanks to cfdez-tech, ghollingworth, neg2led, and everyone else who helped. If I need to add anyone else's name to the top of the pages, please let me know.

lasselj commented 2 years ago

@KyleJudd Kudos. This is cool and you definitely did the right thing in respect of all the legal BS which is completely safe to ignore. I am going to test this with Timebeat (https://timebeat.app) and let you know the results. 5/5. You made my Christmas card list.... :-)

All the best.

Lasse

KyleJudd commented 2 years ago

PTP Testing ... Can someone recommend some common and simple PTP software to test with?

I've tested with my AVB switches (EN/MOTU/Netgear), but I know other people use PTP for other things. I'd like to start with the simple stuff everyone expects PTP to do, and work my way up to the more sophisticated things everyone wants it to do.

One concern I have is that my current solution might not work well as a GMC (if you use gptp for example), and I don't really have any way of testing the quality of a GMC or comparing the accuracy to other GMC's. I think that is where the heart-beat counter issue is going to be noticable. But I could be wrong, and this could just work well out of the box - we'll find out...

KyleJudd commented 2 years ago

I forgot, for the moment this will likely only work with Layer 2 PTP since I am specifically filtering those messages by ether_type. I'm sure Layer 3 will work equally well or poorly once I find good apps to test with. Sorry :)

The current version of my solution only demonstrates that the time stamp registers on the phy are working. I'll keep working on a polished, retail-ready solution.

ghollingworth commented 2 years ago

Kyle, email me, address up above, then can answer some questions about the registers

lasselj commented 2 years ago

That's cool. Timebeat does layer 2 PTP as well. (forwardable and non-forwardable MACs). In case anyone decides to configure here is a timebeat.yml snippet:

timebeat yml - layer2

The arm64 version can be downloaded here: https://timebeat.app/assets/packages/timebeat-1.3.18-arm64.deb

60 day licenses available from the website (https://timebeat.app/download.php), but if you are just doing something cool with open source and want to play then email me at lasse@timebeat.app and I will send you a free license that lasts until the end of the century :-)

Also, Timebeat does cool stuff like the below and in fact when we first started doing our PTP+Squared thing we did it on "Pi210s" :

https://www.icloud.com/iclouddrive/007oKLD6nh8SbpgxcrWauHzNw#Timebeat_PTP+Squared_video

All the best,

Lasse

ainguraXmarquiegui commented 2 years ago

FFS... As Chief Product Officer of Raspberry Pi, and an acting director for the company, I have read and signed (and made Broadcom modify) our software license agreement which allows me (or anyone at Raspberry Pi) to take the information contained within their document and develop open-source drivers.

I cannot show you the contract, because it's also an NDA and I cannot show you the original document (unless you work for Raspberry Pi or are a contractor for us). If anyone else questions whether I am allowed to release the information, I will delete their messages

You've been warned...

On other things, if someone would like to develop a driver under contract for me, then I'm sure we can work something out. Then I can provide you the documentation...

A piece of constructive advice: If you don't want this to happen again you should fix your github account so that it displays who you work for and what your position is there. If you don't want to do that, next time you say something of relevance make sure to explain why what you say should be taken in consideration.

KyleJudd commented 2 years ago

Thanks jnk19, awesome that people are trying to test things, I appreciate it.

In the code you downloaded, there are several issues that I have already resolved that would prevent gptp, ptp4l, and others from working properly. I am almost done fixing issues with gptp, I assume ptp4l will work much better once I post those changes. If there are still problems, we will fix what's necessary for ptp4l next. I'll also make sure it works with timebeat.app (see above).

I'll post here when there is an updated version, maybe in the next 24 hours. Thanks again for working on this.

cfdez-tech commented 2 years ago

Ok, So @KyleJudd it seems that there's no need to create my repo again. I'll attach and contribute again to the project ASAP. Thanks for your efforts!

mlichvar commented 2 years ago

If you are testing with ptp4l, you will need the -2 option to select the L2 transport (default is UDPv4). There is also an example config for the gPTP profile in the configs directory.

From the code that was posted it seems to me the hardware is functionally closer to the hardware supported by the bnx2x driver rather than the tg3 driver. I'm curious to see what limitations they share. Do you know if the BCM54210PE can timestamp unicast PTP packets?

KyleJudd commented 2 years ago

So everyone knows... Most of the code I posted was not written by me, but Carlos I believe, give him credit not me. I wouldn't know the difference between the bnx2x or the tg3 driver, or anything specific about the chip. I have a certain expertise in getting PTP stuff to work for the sake of AVB audio, but my real expertise is audio/video software at the user level.

KyleJudd commented 2 years ago

I learned a few things about alsa drivers from an AES67 driver. Was that you jnk19 ?

KyleJudd commented 2 years ago

New code is available on my repo !!! git clone -b cm4-ptp-kylejudd https://github.com/KyleJudd/linux.git

This version is working rock-solid in my own ptp daemon, but I am still seeing issues in gptp and ptp4l. I haven't tried timebeat yet, but I probably will in the next day or two. Just because gptp and ptp4l aren't working yet doesn't mean that the driver is not working correctly. It may be that the code for gptp or ptp4l may need to be adjusted, so I will try to explain why I am thinking that.

PTP nic/drivers aren't yet like AC97 soundcards or USB Keyboard / Mice where the hardware is designed to match an existing driver so that software is guaranteed to work with it. Much of the development for ptp has been focused around the Intel i210 and other Intel products. Looking at the driver for the i210, they describe their time stamping process as being more single-threaded than what we are doing in our driver. I've noticed in my own ptp daemon that sometimes ptp messages come out-of-sequence, and I have modified that code to handle that issue. Independent Linux documentation warns that the the kernel can re-order the packets and software should be written to handle this issue. Errors I see in gptp look like they are not handling out-of-order packets. I see similar errors in ptp4l, but I am less familiar with that program. This is to be expect from gptp because it was written years ago, mostly for use with the i210, where the single-thread nature guarantees that packets will arrive in order.

I've learned the proper Linux solution is to fork gptp, see if I can make reasonable changes, then do a pull request :) It may be the same way with ptp4l or other software. But some software may work out of the box as well. Its worth testing things, but a failure doesn't necessarily mean there is a bug in the driver.

So, for example, if you are a developer working on timebeat, it may be worth your while to look at the code and see what can be done to make things work with this version of the driver. If there really is a bug in the driver, or implementation is impossible, please let me/us know.

All this driver can do is copy timestamp data from hardware registers and paste them to software buffers that the kernel will push up the stack. There are things we can do to optimize the driver code, but that won't necessarily make the timestamps more accurate, but they are plenty accurate right now. The only issue in the driver is whether all packets are timestamped as intended, and I think we are pretty close to that.

If you've been able to follow my logic so far, I think I can say that this version of the driver demonstrates that the CM4 does have a PHY that does do time stamping, the time stamp registers are not disabled, and that the time stamp registers are plenty accurate. Hence the CM4 can do what it is supposed to do. If the time stamping works slightly differently than the intel i210 or other cards, it doesn't mean that it is not usable or wrong, it just means that some adjustments in software may be necessary.

Thanks for your help, I'll keep working on this. Like everyone else, I have a day job and other obligations, so it may be a few days before any new code changes.

KyleJudd commented 2 years ago

To simply what I said above for software developers...

When sending peer-delay requests, software might assume that it will receive timestamps in this order T1, then T2 & T4, T3. For threading reasons in this driver, you might receive T2 & T4 first, T3 next, then T1. Or sometimes T1, T3, then T2 & T4. It's possible the driver is sending the packets to the kernel in correct order, but the kernel is reordering them.

The phenomenon may be fixable in the driver, but if you take the time to handle this situation in software, your software might be able to work on the Raspberry Pi CM4, where other software can not.

wowczarek commented 2 years ago

To simply what I said above for software developers...

This is why I always opted for a different timestamp delivery method in the Linux kernel, rather than socket messages... I envisaged something along the lines of /dev/tspipeN where the driver would simply stream the set of timestamps as and when it received them, in a common format: flags (what identification fields are available i.e. domain, seq,msgtype,direction, or "not PTP", or otherwise event type such as 1PPS in where available) , then timestamp, then ID fields, or even the payload, for those cases where a NIC allows timestamping all packets. This would unburden the driver from managing internal queues of fixed lengths, which often results in the dreaded TX timestamp misses, and it could grab the timestamps off the h/w FIFO as they came, and the PTP stack would pick those up and act accordingly.

There was just never time to do this, and the current API is what it is, and in my opinion it hurts the achievable timestamping throughput, especially on TX. And yes, the s/w stack should not expect a specific order, instead wait for pairs: HAVE_T1|HAVE_T2, HAVE_T3|HAVE_T4, and update the respective side of the equations when one of the pairs is received - in effect, decouple PTP messages from attached timestamps in a sort of control plane / data plane manner.

Great work anyhow; I still haven't gotten my hands on the CM4 but It's definitely next on my list for if and when I get back to developing / rewriting ptpd.

KyleJudd commented 2 years ago

Thanks for the insight wowczarek. Do you have some advice as to which product would be easiest to debug through to see if everything is working correctly? I am most familiar with gptp, but I know the code inside is fairly elaborate. Would either ptpd or ptp4l be easier to understand in this circumstance? Is there any well known sample code that would work well for testing?

I do think there is something inherently wrong when a driver has to post a legitimate message to the error queue. :)

KyleJudd commented 2 years ago

Did I hear you correctly - Its working with Layer 2? I'll check it out tonight if it is. Anything IP4 or IP6 just isn't implemented yet. I should have explained that. The code writing shouldn't be difficult, but I've never used IP4 or IP6 PTP, so someone might have to explain how I can test that as I implement it.

jnk19 commented 2 years ago

Yes L2 is working I see packet movement, I do not have the correct hardware to test it but is working. The UDPV4 and UDPV6 need a revisit. Check it with ptp4l.

KyleJudd commented 2 years ago

I'll check that out then. Thanks.

KyleJudd commented 2 years ago

If I want to test IP4 & IP6, do I just need 2 computers running through a normal switch? Do I need a switch with special features? Is IP4/IP6 designed to sync with clocks on the internet instead of a local LAN?

ghollingworth commented 2 years ago

Kyle, can you explain why you are taking so many readings from the hardware clock? You should only really need to read the hardware clock to create and maintain the local time...

Are you trying to timestamp the packets in software, rather than in hardware?

KyleJudd commented 2 years ago

Let me answer this in an email. I'm not sure I understand the question, but I'll try to say what I can here. The reason why I was doing this was to fake a real-time timer callback, but I know that eventually I will implement my software using real-time features of linux and I won't have to worry about faking this timer callback. So for me this is not a real requirement, but I was surprised to see gptp (written by Intel) polling the hardware time so frequently, so I figured this might be something we may need to support anyways. Do I need this to implement my software? But what I saw with the Intel-gptp implies that other people may need to poll the clock frequently in order to do whatever it is they are trying to do.

I agree with you that all you should have to do is use the system clock with frequency adjustment, but that doesn't work in real life with some of the equipment I work with. I'll explain that in an email because that doesn't relate to PTP and what we are doing here.

KyleJudd commented 2 years ago

I'm making great progress, but need help with one issue. I need to verify that setting the frequency of the clock on the NIC /PHY is working correctly. I know the function I need to use to test is clock_adjtime() and I understand its usage. What I don't understand is changing frequency using PPM or PPB. I deal with frequency with floats where 1.0 = normal, 0.5 = half, 2.0 = 2 x normal, etc.

If I want to change a clock's frequency so that it runs at 50% or 200% (0.5 or 2.0 in my world) how would I calculate the necessary PPM to pass to clock_adjtime() ? Is there good documentation/explanation for this somewhere?

KyleJudd commented 2 years ago

Is there a usual limit for how much you can change the frequency? Can I really change it 50% or 200%, or is it more like 1%?

mlichvar commented 2 years ago

The maximum depends on the hardware and driver. Usually it's at least few percent. For example, on I219-LM it is ~60% as reported by phc_ctl:

# phc_ctl /dev/ptp0 
phc_ctl[13314.393]: 
capabilities:
  599999999 maximum frequency adjustment (ppb)
  0 programable alarms
  0 external time stamp channels
  0 programmable periodic signals
  0 configurable input/output pins
  doesn't have pulse per second support
  has cross timestamping support

The clock_adjtime() call follows the original ntp_adjtime() and Linux-specific adjtimex(). https://www.man7.org/linux/man-pages/man2/adjtimex.2.html

The frequency is set in a fixed-point format using 16-bit fraction in parts per million (ppm). If you wanted to set it to run 50% faster (i.e. at 150% of its nominal speed), timex.freq would be 500000 * 65536 = 32768000000. The type is long, so such a large frequency offset can be set only on 64-bit systems.

KyleJudd commented 2 years ago

Thanks mlichvar. That makes sense, the fixed-point format is what I was missing. lasselj is helping look into this and he has tools far better than mine.

KyleJudd commented 2 years ago

I just posted new code, don't get too excited. This basically just fixes an issue where CPU Core #2 was constantly running at 50-100%. If you have already downloaded previous code, please download the current code to replace what you have. If you haven't downloaded the code yet, don't worry about download this, its not ready yet.

We are definitely making progress, but there are some known issues that we are working to resolve. Those issues would prevent the driver from working in any PTP app. Good testing is being done, so I'll let you know when we figure it out.

KyleJudd commented 2 years ago

Nope. I'm now turning my attention to that, as well as re-working the threads and the queues. I might actually have to buy me a cheap new dumb-switch. All of the switches I have are designed to do the 802.1AS style PTP, and it is not necessarily easy to disable the Layer 2 packets from being sent.

Your welcome to work on that as well, if it is quick and easy for you. Don't worry if you can't, I know we all have other obligations.

KyleJudd commented 2 years ago

I just meant I didn't want to stand in your way if you wanted to do it, that's all. With your expertise, you can probably do in a few minutes what might take me all day, so if you get bored this weekend, feel free to knock that out. But no worries if you can't, you've already been very helpful.

lasselj commented 2 years ago

Hi Everybody, (@KyleJudd @ghollingworth @cfdez-tech)

I had a look. As you may or may not be aware, a particular Ethernet controller driver registers itself using the ptp_clock_info structure which is filled with some different function pointers. In the draft code so far, it looks like this:

static const struct ptp_clock_info bcm54210pe_clk_caps = { .owner = THIS_MODULE, .name = "BCM54210PE_PHC", .max_adj = S32_MAX, .n_alarm = 0, .n_pins = 0, .n_ext_ts = 0, .n_per_out = 0, .pps = 0, .adjtime = &bcm54210pe_adjtime, .adjfine = &bcm54210pe_adjfine, .gettime64 = &bcm54210pe_gettime, .settime64 = &bcm54210pe_settime, };

I am persuaded that both bcm54210pe_gettime and bcm54210pe_settime work fine. This allow reading the PHC clock time and stepping the PHC clock. Stepping is useful if a clock is off by more than 500ms.

However, in order to adjust the PHC very accurately (or "finely") the (bcm54210pe_)adjfine function must also work. When a PTP application calls clock_adjtime(2) this is in the case of this particular controller the function that gets called (actually, clock_adjtime in glibc just wraps a syscall, but this is not really important to understand).

Anyhow, the point is that this function does not work. If you observe the Gist below is does not matter what are the values of hi and lo are, I have tried the entire spectrum of both hi and lo from uint16 max to 0 and it produces no change in the drift of the clock in respect of the hardware timestamped PTP. (...which now works, still just layer 2, but at least e2e delay request scheme is supported in addition to p2p - l2/p2p is "quite" an unusual config... :-)) :

https://gist.github.com/lasselj/26f8ae88ff175108a09fcda7e9002225

I have tried to swap around the write sequence of the MSB and LSB registers as some controllers are particular about this. It makes no difference.

So, we can add support for layer 4 PTP and tidy up ioctl handling, add gettimex64 functions for some sweet PTP_SYS_OFFSET_EXTENDED functionality etc.etc.etc., but until we know that we understand the correct registers for the adjfine stuff, this does not seem like a good use of time...

Here is a fancy diagram from Timebeat's front-end showing how frequency changes does not impact the drift rate:

CM4 offset and frequency

In parallel I was observing the values of hi and lo with some printk debug statements.

Incidentally, if someone reading this has access to the relevant Broadcom controller register information under NDA needed to fix this problem and their agreement with Broadcom allows for open source software to be released on the basis of that information, then I am available for hire as a consultant for an annual rate of £1 to work on this specific driver. That is "somewhat" below my normal rate... :-)

All the best,

Lasse

KyleJudd commented 2 years ago

I agree with lasselj and I think I am seeing the same phenomenon in ptp4l. The timestamping works, but the clock drifts and ptp4l can't seem, to correct it.

There is one last thing to try. I noticed that after Settime(), the first time you call Gettime() you will get the wrong value, then after that you get the correct value. So in Settime(), I call Gettime() before returning so that the next call from the user app will be correct. I am not sure I understand what kind of latching is going on there, but perhaps something similar needs to be done in AdjFine. Maybe the registers in AdjFine aren't truely set until they are read again.

I'm using my time today to see if I can improve the performance of the driver code, trying different threading ideas. I'll probably also try to implement the IP4/IP6 stuff today, but after that, there isn't much more for me to do.

I agree that having access to the docs is critical at this point, even if just to verify what we've done so far is correct. If lasselj 's rates are too steep, I am willing to work for $1 :)