On Wed, Feb 17, 2021 at 9:20 PM Min Li [off-list ref] wrote:
I attached the G.8273.2 document, where chapter 6 is about supporting physical layer
frequency. And combo mode is Renesas way to support this requirement. Other companies
may come up with different ways to support it.
When EEC quality is below certain level, we would wanna turn off combo mode.
Maybe this is something that could be handled inside of the device driver then?
If the driver can use the same algorithm that is in your user space software
today, that would seem to be a nicer way to handle it than requiring a separate
application.
quoted
quoted
This function will read FCW first and convert it to FFO.
Is this related to the information in the timex->freq field? It sounds like this
would already be accessible through the existing
clock_adjtime() interface.
They are related, but dealing with timex->freq has limitations
1) Renesas SMU has up to 8 DPLLs and only one of the them would be ptp
clock and we want to be able to read any DPLL's FFO or state
Is this necessarily unique to Renesas SMU though? Not sure what
makes sense in terms of the phc/ptp interface. Could there just be
a separate instance for each DPLL in the phc subsystem even if it's
actually a ptp clock, or would that be an incorrect use?
2) timex->freq's unit is ppb and we want to read more precise ffo in smaller unit of ppqt
This also sounds like something that would not be vendor specific. If you
need a higher resolution, then at some point others would need it as well.
There is already precedence in 'struct timex' to redefine the resolution of
some fields based on a flag -- 'time.tv_usec' can either refer to microseconds
to nanoseconds.
If the range of the 'freq' field is sufficient to encode ppqt, you could add
another flag for that, otherwise another reserved field can be used.
3) there is no interface in the current ptp hardware clock infrastructure to read ffo back from hardware
Adding an internal interface is the easy part here, the hard part is defining
the user interface.
quoted
Wouldn't any PTP clock run in one of these modes? If this is just
informational, it might be appropriate to have another sysfs attribute for
each PTP clock that shows the state of the DPLL, and then have the PTP
driver either fill in the current value in 'struct ptp_clock', or provide a
callback to report the state when a user reads the sysfs attribute.
What you propose can work. But DPLL operating mode is not standardized
so different vendor may have different explanation for various modes.
If it's a string, that could easily be extended to further modes, as long
as the kernel documents which names are allowed. If multiple vendors
refer to the same mode by different names, someone will have to decide
what to call it in the kernel, and everyone afterwards would use the same
name.
Also, I thought sysfs is only for debug or informational purpose.
Production software is not supposed to use it for critical tasks?
No, you are probably thinking of debugfs. sysfs is one of multiple
common ways to exchange this kind of data in a reliable way.
An ioctl would probably work just as well though, usually sysfs
is better when the information makes sense to human operators
or simple shell scripts, while an ioctl interface is better if performance
is important, or if the information is primarily used in C programs.
Arnd
On Wed, 17 Feb 2021 22:30:14 +0100 Arnd Bergmann wrote:
On Wed, Feb 17, 2021 at 9:20 PM Min Li [off-list ref] wrote:
quoted
I attached the G.8273.2 document, where chapter 6 is about supporting physical layer
frequency. And combo mode is Renesas way to support this requirement. Other companies
may come up with different ways to support it.
When EEC quality is below certain level, we would wanna turn off combo mode.
Maybe this is something that could be handled inside of the device driver then?
If the driver can use the same algorithm that is in your user space software
today, that would seem to be a nicer way to handle it than requiring a separate
application.
Other points sound more time than networking, so no suggestions
from me, but on using PHC for L1 freq - that seems like a good
fit for ethtool?
-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: February 17, 2021 4:30 PM
To: Min Li <redacted>
Cc: Derek Kiernan <redacted>; Dragan Cvetic
[off-list ref]; Arnd Bergmann [off-list ref]; gregkh
[off-list ref]; linux-kernel@vger.kernel.org; Networking
[off-list ref]; Richard Cochran [off-list ref]
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization
Management Unit (SMU) support
On Wed, Feb 17, 2021 at 9:20 PM Min Li [off-list ref] wrote:
quoted
I attached the G.8273.2 document, where chapter 6 is about supporting
physical layer frequency. And combo mode is Renesas way to support
this requirement. Other companies may come up with different ways to
support it.
quoted
When EEC quality is below certain level, we would wanna turn off combo
mode.
Maybe this is something that could be handled inside of the device driver
then?
If the driver can use the same algorithm that is in your user space software
today, that would seem to be a nicer way to handle it than requiring a
separate application.
Hi Arnd
What is the device driver that you are referring here?
In summary of your reviews, are you suggesting me to discard this change and go back to PTP subsystem
to find a better place for things that I wanna do here?
Min
On Thu, Feb 18, 2021 at 4:28 AM Min Li [off-list ref] wrote:
quoted
If the driver can use the same algorithm that is in your user space software
today, that would seem to be a nicer way to handle it than requiring a
separate application.
Hi Arnd
What is the device driver that you are referring here?
In summary of your reviews, are you suggesting me to discard this change
and go back to PTP subsystem to find a better place for things that I wanna
do here?
Yes, I mean doing it all in the PTP driver.
Arnd
-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: February 18, 2021 5:51 AM
To: Min Li <redacted>
Cc: Derek Kiernan <redacted>; Dragan Cvetic
[off-list ref]; Arnd Bergmann [off-list ref]; gregkh
[off-list ref]; linux-kernel@vger.kernel.org; Networking
[off-list ref]; Richard Cochran [off-list ref]
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization
Management Unit (SMU) support
On Thu, Feb 18, 2021 at 4:28 AM Min Li [off-list ref] wrote:
quoted
quoted
If the driver can use the same algorithm that is in your user space
software today, that would seem to be a nicer way to handle it than
requiring a separate application.
Hi Arnd
What is the device driver that you are referring here?
In summary of your reviews, are you suggesting me to discard this
change and go back to PTP subsystem to find a better place for things
that I wanna do here?
Yes, I mean doing it all in the PTP driver.
Arnd
Hi Arnd
The APIs I am adding here is for our development of assisted partial timing support (APTS),
which is a Global Navigation Satellite System (GNSS) backed by Precision Time Protocol (PTP).
So it is not part of PTP but they can work together for network timing solution.
What I am trying to say is the things that I am adding here doesn't really belong to the PTP world.
For example, timex->freq is different from the ffo that I am reading from this driver since the DPLL is
Working in different mode. For PTP, DPLL is working in DCO mode. In DCO mode, the DPLL
control loop is opened and the DCO can be controlled by a PTP clock recovery servo running on an
external processor to synthesize PTP clocks. On the other hand for GNSS timing, the ffo I am reading here is when DPLL is
in locked mode. In Locked the long-term output frequency accuracy is the same as the long-term
frequency accuracy of the selected input reference.
For our GNSS APTS development, we have 2 DPLL channels, one channel is locked to GNSS and another channel is PTP channel.
If GNSS channel is locked, we use GNSS's channel to support network timing. Otherwise, we switch to PTP channel.
To think about it, our device is really an multi functional device (MFD), which is why I am submitting another review for our MFD driver
on the side. We have our PTP driver and we have this for GNSS APTS and other misc functions.
So can you take a look at this again and see if it makes sense to keep this change simply because the change is not part of PTP subsystem.
They sound like they are related. But when it comes to technicality, there is really no place in PTP to hold stuff that I am doing here.
Thanks
Min
-----Original Message-----
From: Min Li
Sent: February 18, 2021 11:14 AM
To: 'Arnd Bergmann' <arnd@kernel.org>
Cc: Derek Kiernan <redacted>; Dragan Cvetic <redacted>; Arnd Bergmann <arnd@arndb.de>; gregkh <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; Networking <redacted>; Richard Cochran <richardcochran@gmail.com>
Subject: RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: February 18, 2021 5:51 AM
To: Min Li <redacted>
Cc: Derek Kiernan <redacted>; Dragan Cvetic
[off-list ref]; Arnd Bergmann [off-list ref]; gregkh
[off-list ref]; linux-kernel@vger.kernel.org; Networking
[off-list ref]; Richard Cochran [off-list ref]
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization
Management Unit (SMU) support
On Thu, Feb 18, 2021 at 4:28 AM Min Li [off-list ref] wrote:
quoted
quoted
If the driver can use the same algorithm that is in your user
space software today, that would seem to be a nicer way to handle
it than requiring a separate application.
Hi Arnd
What is the device driver that you are referring here?
In summary of your reviews, are you suggesting me to discard this
change and go back to PTP subsystem to find a better place for
things that I wanna do here?
Yes, I mean doing it all in the PTP driver.
Arnd
Hi Arnd
The APIs I am adding here is for our development of assisted partial timing support (APTS), which is a Global Navigation Satellite System (GNSS) backed by Precision Time Protocol (PTP).
So it is not part of PTP but they can work together for network timing solution.
What I am trying to say is the things that I am adding here doesn't really belong to the PTP world.
For example, timex->freq is different from the ffo that I am reading from this driver since the DPLL is Working in different mode. For PTP, DPLL is working in DCO mode. In DCO mode, the DPLL control loop is opened and the DCO can be controlled by a PTP clock recovery servo running on an external processor to synthesize PTP clocks. On the other hand for GNSS timing, the ffo I am reading here is when DPLL is in locked mode. In Locked the long-term output frequency accuracy is the same as the long-term frequency accuracy of the selected input reference.
For our GNSS APTS development, we have 2 DPLL channels, one channel is locked to GNSS and another channel is PTP channel.
If GNSS channel is locked, we use GNSS's channel to support network timing. Otherwise, we switch to PTP channel.
To think about it, our device is really an multi functional device (MFD), which is why I am submitting another review for our MFD driver on the side. We have our PTP driver and we have this for GNSS APTS and other misc functions.
So can you take a look at this again and see if it makes sense to keep this change simply because the change is not part of PTP subsystem.
They sound like they are related. But when it comes to technicality, there is really no place in PTP to hold stuff that I am doing here.
Thanks
Min
Hi Arnd
This is Min again.
I just got off the meeting with our Architect and I would like to correct some confusing information that I delivered before
I said this driver supports 1588/PTP related functionalities. This is not correct. In fact, this driver is developed to support our
"GNSS Assisted Partial Timing Support" feature, which is not in PTP domain.
So I would categorize Renesas SMU (synchronization management unit) device as a multi-functional device and we are actually having a
separate review with Lee Jones for the MFD driver alone. Above the MFD driver, we have PHC driver to support PTP and this
driver for miscellaneous functions like GNSS APTS stuff.
I also attached the diagram drawing, where left column is the old way we are doing things and right column is how we wanna do it
through MFD now.
Hopefully, this will help you reconsider my proposal. Please do not hesitate to get back to me for this issue.
Thanks
Min
-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: February 18, 2021 5:51 AM
To: Min Li <redacted>
Cc: Derek Kiernan <redacted>; Dragan Cvetic
[off-list ref]; Arnd Bergmann [off-list ref]; gregkh
[off-list ref]; linux-kernel@vger.kernel.org; Networking
[off-list ref]; Richard Cochran [off-list ref]
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization
Management Unit (SMU) support
On Thu, Feb 18, 2021 at 4:28 AM Min Li [off-list ref] wrote:
quoted
quoted
If the driver can use the same algorithm that is in your user space
software today, that would seem to be a nicer way to handle it than
requiring a separate application.
Hi Arnd
What is the device driver that you are referring here?
In summary of your reviews, are you suggesting me to discard this
change and go back to PTP subsystem to find a better place for things
that I wanna do here?
Yes, I mean doing it all in the PTP driver.
Arnd