Re: [Linuxptp-devel] [PATCH 2/2] phc_ctl: use PTP_CLOCK_GETCAPS2 ioctl if available

2023-06-22 Thread Jacob Keller


On 12/8/2022 1:20 PM, Geva, Erez wrote:
> On Wed, 2022-12-07 at 06:59 -0800, Richard Cochran wrote:
>> On Thu, Nov 17, 2022 at 02:15:23PM -0800, Jacob Keller wrote:
>>> On 11/17/2022 1:34 PM, Geva, Erez wrote:
>>
 The problem is the fallback works only on build.
 But if the build system is newer than the running system, the
 fallback
 will fail, as you will use the PTP_CLOCK_GETCAPS2 which does not
 exist
 on the running old system.

>>>
>>> Fair. We likely have the same problem with some of the other "2"
>>> ioctls,
>>> since they're handled in a similar way. I think we do the Right(TM)
>>> thing
>>> for the sysoff.c where we probe the kernel at run-time. This could
>>> be done
>>> here but is probably not really worth it considering that
>>> PTP_CLOCK_GETCAPS
>>> functions the same way as PTP_CLOCK_GETCAPS2 for all kernels I
>>> checked... So
>>> I guess this is somewhat less likely.
>>
>> Jacob, do you want to have phc_ctl fall back to PTP_CLOCK_GETCAPS at
>> run time if PTP_CLOCK_GETCAPS2 fails?
> 
> We can use run-time fallback.
> But personalty, I do not think it worth it.
> Using PTP_CLOCK_GETCAPS2 is only done for one new field in a debug
> tool.
> We can simply wait till PTP_CLOCK_GETCAPS become obsolete or we have a
> new PTP_CLOCK_GETCAPS3 to handle.
> 
>>
>>> I'm not sure if our other PTP ioctls are checked properly like this
>>> at run
>>> time...
>>
>> Maybe, but only because of sloppiness.  Better to support older
>> kernels at run time, as this is more user friendly.
>>
>> Working on embedded systems over the years, I've have often been that
>> user, and, believe me, it is super annoying when the latest greatest
>> App isn't backwards compatible.
> 
> My view too :-)
> 
>>
>> Thanks,
>> Richard
> 
> 
> Erez

Agreed. Since there's no difference to using PTP_CLOCK_GETCAPS2 at least
currently, it makes sense to just keep using PTP_CLOCK_GETCAPS.


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH v2] remove C99 style loop variable declarations

2023-06-22 Thread Jacob Keller
When building the tlv.c file, the following warning may occur when
operating in C89 mode:

  $make EXTRA_CFLAGS=-std=gnu89
  tlv.c: In function ‘mgt_post_recv’:
  tlv.c:374:17: error: ‘for’ loop initial declarations are only allowed in C99 
or C11 mode
374 | for (int i = 0; i < umtn->actual_table_size; i++) {
| ^~~
  tlv.c:374:17: note: use option ‘-std=c99’, ‘-std=gnu99’, ‘-std=c11’ or 
‘-std=gnu11’ to compile your code
  tlv.c: In function ‘mgt_pre_send’:
  tlv.c:551:17: error: ‘for’ loop initial declarations are only allowed in C99 
or C11 mode
551 | for (int i = 0; i < umtn->actual_table_size; i++) {
| ^~~
  make: *** [: tlv.o] Error 1
  make: *** Waiting for unfinished jobs
  pmc.c: In function ‘pmc_show’:
  pmc.c:559:17: error: ‘for’ loop initial declarations are only allowed in C99 
or C11 mode
559 | for (int i = 0; i < umtn->actual_table_size; i++) {
| ^~~

This occurs because initial loop declarations are only supported in C99 or
newer. LinuxPTP source still prefers to keep with the GNU C89 convention of
variables being at the top of the function. Newer versions of GCC have
begun defaulting to a newer C standard, which explains how these slipped
in. Note that directly enforcing "-std=c89" does not work because we rely
on some GNU extensions of C89.

For the tlv.c file it turns out that the case statements which declare the
variables in loops already have suitable iterator variables bound at the
top of the function. For the iterator in pmc_show, we need to add the
declaration at the top of the function.

Reported-by: Jakub Raczyński 
Signed-off-by: Jacob Keller 
---

 pmc.c | 4 ++--
 tlv.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pmc.c b/pmc.c
index 00e691f0c244..bc870585960e 100644
--- a/pmc.c
+++ b/pmc.c
@@ -181,8 +181,8 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
uint64_t next_jump;
struct portDS *p;
struct TLV *tlv;
+   int action, i;
uint8_t *buf;
-   int action;
 
if (msg_type(msg) == SIGNALING) {
pmc_show_signaling(msg, fp);
@@ -591,7 +591,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
"BM", "identity", "address", "state",
"clockClass", "clockQuality", "offsetScaledLogVariance",
"p1", "p2");
-   for (int i = 0; i < umtn->actual_table_size; i++) {
+   for (i = 0; i < umtn->actual_table_size; i++) {
ume = (struct unicast_master_entry *) buf;
pmc_show_unicast_master_entry(ume, fp);
buf += sizeof(*ume) + ume->address.addressLength;
diff --git a/tlv.c b/tlv.c
index 79400126cbc4..9b82bd9d3fb2 100644
--- a/tlv.c
+++ b/tlv.c
@@ -444,7 +444,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t 
data_len,
umtn->actual_table_size =
ntohs(umtn->actual_table_size);
buf = (uint8_t *) umtn->unicast_masters;
-   for (int i = 0; i < umtn->actual_table_size; i++) {
+   for (i = 0; i < umtn->actual_table_size; i++) {
len += sizeof(struct unicast_master_entry);
if (data_len < len)
goto bad_length;
@@ -643,7 +643,7 @@ static void mgt_pre_send(struct management_tlv *m, struct 
tlv_extra *extra)
case MID_UNICAST_MASTER_TABLE_NP:
umtn = (struct unicast_master_table_np *)m->data;
buf = (uint8_t *) umtn->unicast_masters;
-   for (int i = 0; i < umtn->actual_table_size; i++) {
+   for (i = 0; i < umtn->actual_table_size; i++) {
ume = (struct unicast_master_entry *) buf;
// update pointer before the conversion
buf += sizeof(*ume) + ume->address.addressLength;
-- 
2.41.0.1.g9857a21e0017.dirty



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] remove C99 style loop variable declarations

2023-06-22 Thread Jacob Keller


On 6/22/2023 8:49 PM, Richard Cochran wrote:
> On Mon, Jun 12, 2023 at 10:18:07AM +0200, Jakub Raczyński wrote:
>> In master version and 4.0 this patch was not applied. Please apply this when 
>> possible.
> 
> That patch does not apply (as I said back on 07 Dec 2022)
> 
> If you want to have it, then please clean it up and re-submit.
> 
> Thanks,
> Richard

Apologies I must have missed that message. Anyways, I will propose an
updated version that applies now.

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] remove C99 style loop variable declarations

2023-06-22 Thread Jacob Keller



On 12/7/2022 6:43 AM, Richard Cochran wrote:
> On Wed, Nov 16, 2022 at 11:48:32AM -0800, Jacob Keller wrote:
>> When building the tlv.c file, the following warning may occur when
>> operating in C89 mode:
> 
> Jacob, this patch doesn't apply any more.  Can you rebase it please?
> 
> Thanks,
> Richard


I will rebase this.

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Use the 802.1AS peer delay computation when transportSpecific is 1

2023-06-22 Thread Richard Cochran
On Tue, Jun 06, 2023 at 02:25:44PM -0400, Dylan Robinson wrote:
> If the transportSpecific value is configured to be 1, compute t3c using
> the 802.1AS math instead of the 1588 math.
> 
> The 1588 defined computation using variable names from this project is:
> D = [(t4 - t1) - (t3 - t2) - c1 - c2]/2
> 
> The existing code combines the corrections into a variable t3c which we
> can get the value of by isolating t3, c1 and c2:
> t3c = t3 + c1 + c2
> 
> The 802.1AS defined computation is:
> D = [(t4 - t1) - (t3 - t2) + c1 - c2]/2
> 
> Again, isolating t3, c1 and c2 for 802.1AS:
> t3c = t3 + c2 - c1
> 
> This has been tested against the MOTU Switch based on the KSZ9567 with
> non-zero correction fields based on the 802.1AS equation as well as MOTU
> audio interfaces that don't utilize the correction fields.
> 
> Signed-off-by: Dylan Robinson 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/5] General improvements for linuxptp focused around phase adjustment

2023-06-22 Thread Richard Cochran
On Thu, Jun 22, 2023 at 10:09:50AM -0700, Rahul Rameshbabu via Linuxptp-devel 
wrote:
> On Thu, 22 Jun, 2023 12:11:05 +0200 Erez  wrote:
> > Just add a version tag, please. Whether you add an RFC or not.
> > If you really must , you can add a note in the cover letter for Richard.
> 
> Noted for future submissions. Thanks for the clarification.

If you started with "RFC", then the next post can be "v1".

In any case, put something in the Subject: that helps us keep track of
your work.

Thanks,
Richard




___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Resolve false hybrid_e2e warning

2023-06-22 Thread Richard Cochran
On Mon, May 01, 2023 at 02:22:57PM +0300, Eyal Itkin via Linuxptp-devel wrote:
> When delay_mechanism is set to "E2E" mode in the [global] section,
> it is applied only to non-UDS ports as UDS ports will override the
> delay_mechanism with "DM_AUTO". Still the UDS ports will be checked
> for hybrid_e2e and generate a false warning of: "hybrid_e2e only
> works with E2E".
> 
> Update the check so it would only cover non-UDS ports.
> 
> Signed-off-by: Eyal Itkin 
> Reviewed-by: Rahul Rameshbabu 

Applied.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] remove C99 style loop variable declarations

2023-06-22 Thread Richard Cochran
On Mon, Jun 12, 2023 at 10:18:07AM +0200, Jakub Raczyński wrote:
> In master version and 4.0 this patch was not applied. Please apply this when 
> possible.

That patch does not apply (as I said back on 07 Dec 2022)

If you want to have it, then please clean it up and re-submit.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/5] General improvements for linuxptp focused around phase adjustment

2023-06-22 Thread Rahul Rameshbabu via Linuxptp-devel
On Thu, 22 Jun, 2023 12:11:05 +0200 Erez  wrote:
> On Wed, 21 Jun 2023 at 01:13, Rahul Rameshbabu  wrote:
>
>  Hi Erez,
>
>  On Wed, 21 Jun, 2023 00:33:28 +0200 Erez  wrote:
>  > Hi,
>  >
>  > You already submitted the patch seria.
>  > Has it changed?
>
>  Yes, I took feedback from the RFC (request for comments) I sent out and
>  applied it in this submission. In the git notes field in the first patch
>  of the series, I document the change made since the RFC. Since the first
>  submission was an RFC, I did not treat it as a formal submission, so I
>  did not consider it a v1 but rather a draft. I submitted as an RFC due
>  to pending kernel side changes.
>
>  > If so, please mark it with version 2. "git format-patch -v 2".
>
>  I agree with this if a formal v1 was submitted. However since my
>  previous submission was an RFC, is the practice still to increment the
>  official submission as v2?
>
> Rahul, all patches are for review.
> Do not make our life too much sophisticated.

Ack.

> Just add a version tag, please. Whether you add an RFC or not.
> If you really must , you can add a note in the cover letter for Richard.

Noted for future submissions. Thanks for the clarification.

Thanks,

Rahul


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 0/5] General improvements for linuxptp focused around phase adjustment

2023-06-22 Thread Erez
On Wed, 21 Jun 2023 at 01:13, Rahul Rameshbabu 
wrote:

> Hi Erez,
>
> On Wed, 21 Jun, 2023 00:33:28 +0200 Erez  wrote:
> > Hi,
> >
> > You already submitted the patch seria.
> > Has it changed?
>
> Yes, I took feedback from the RFC (request for comments) I sent out and
> applied it in this submission. In the git notes field in the first patch
> of the series, I document the change made since the RFC. Since the first
> submission was an RFC, I did not treat it as a formal submission, so I
> did not consider it a v1 but rather a draft. I submitted as an RFC due
> to pending kernel side changes.
>
> > If so, please mark it with version 2. "git format-patch -v 2".
>
> I agree with this if a formal v1 was submitted. However since my
> previous submission was an RFC, is the practice still to increment the
> official submission as v2?
>

Rahul, all patches are for review.
Do not make our life too much sophisticated.
Just add a version tag, please. Whether you add an RFC or not.
If you really must , you can add a note in the cover letter for Richard.

Linux works with several groups.
Linuxptp has only one developer group, it is not that big :-)

Thanks
  Erez


>
> > If not, why do you send it again?
>
> Ignoring the fact that I applied changes based on feedback from the RFC
> submission, isn't the normal practice to submit an RFC as a normal
> submission to the mailing list to indicate readiness for applying the
> patches onto the target?
>
> > I think Richard wanted to close version 4 first.
>
> Sure, that makes sense. I am hoping RFC patches are not considered for
> merging into releases or the default branch of the project. At the time,
> the needed kernel side changes were not merged into net-next of the
> linux netdev tree.
>
> Apologize in advance if we are implementing the practice of closing
> submissions during release windows (similar to what net-next does in the
> linux netdev tree). I did not see a mention of that on the mailing list.
> Might have missed it.
>
> Thanks,
>
> Rahul Rameshbabu
>
> >
> > Erez
> >
> > On Tue, 20 Jun 2023 at 19:39, Rahul Rameshbabu via Linuxptp-devel <
> linuxptp-devel@lists.sourceforge.net> wrote:
> >
> >  The main focus of this submission is adding support for testing
> ADJ_OFFSET with
> >  phc_ctl and querying the maximum supported ADJ_OFFSET adjustment that a
> device
> >  is capable of. Some other minor cleanups are also included in the
> submission.
> >
> >  That patch the introduces support for querying the maximum offset
> supported by
> >  ADJ_OFFSET depends on a kernel patch series (linked below) that is
> targeted for
> >  kernel 6.5. Previously, sent this series out as an RFC to inquire
> feedback early
> >  on. Have incorporated that feedback into this submission.
> >
> >  Link: https://sourceforge.net/p/linuxptp/mailman/message/37854603/
> >  Link:
> https://lore.kernel.org/netdev/20230612211500.309075-1-rrameshb...@nvidia.com/
> >
> >  Rahul Rameshbabu (5):
> >Rename NSEC2SEC as NSEC_PER_SEC and refactor to util.h
> >phc_ctl: Add phase command to support ADJ_OFFSET
> >phc_ctl: Add maximum offset capability
> >phc_ctl: Use pr_notice instead of pr_err for displaying adjusted
> >  frequency
> >phc_ctl: Handle errors returned by various clockadj helpers
> >
> >   missing.h  |  9 +++---
> >   phc_ctl.8  |  4 +++
> >   phc_ctl.c  | 77 ++
> >   port.c | 14 -
> >   port_private.h |  3 +-
> >   servo.c|  3 +-
> >   tc.c   |  6 ++--
> >   util.h |  2 ++
> >   8 files changed, 82 insertions(+), 36 deletions(-)
> >
> >  --
> >  2.40.1
> >
> >  ___
> >  Linuxptp-devel mailing list
> >  Linuxptp-devel@lists.sourceforge.net
> >  https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel