Hi Miroslav, thank you for your review. I updated the patch as suggested. 
Hopefully it is ok now...

Viliam Lejcik

> -----Original Message-----
> From: Miroslav Lichvar [mailto:[email protected]]
> Sent: 17 January 2017 10:36
> To: Lejcik Viliam
> Cc: [email protected]
> Subject: Re: [PATCH] ptp4l: Make UTC offset configurable.
> 
> On Mon, Jan 16, 2017 at 06:31:33PM +0100, Viliam Lejcik wrote:
> > Currently UTC offset is defined as a constant - CURRENT_UTC_OFFSET,
> > and if a leap second is added, that constant is no longer valid. Ptp4l
> > was updated to read the UTC offset from configuration instead.
> 
> Good idea.
> 
> > @@ -681,7 +681,7 @@ static void clock_update_slave(struct clock *c)
> >     if (!(c->tds.flags & PTP_TIMESCALE)) {
> >             pr_warning("foreign master not using PTP timescale");
> >     }
> > -   if (c->tds.currentUtcOffset < CURRENT_UTC_OFFSET) {
> > +   if (c->tds.currentUtcOffset < config_get_int(c->config, NULL,
> > +"utc_offset")) {
> 
> Would it make sense to add a new field to the clock structure instead of 
> calling
> config_get_int() every time the fallback offset is needed?
> 
> > +++ b/ptp4l.8
> > @@ -327,6 +327,10 @@ The default is 0xFFFF.
> >  The domain attribute of the local clock.
> >  The default is 0.
> >  .TP
> > +.B utc_offset
> > +The current number of leap seconds.
> > +The default is 37.
> 
> This should probably say it's the offset between TAI and UTC. The actual 
> number
> of leap seconds is smaller by 10 and this could be confusing.
> 
> --
> Miroslav Lichvar

Attachment: 0001-ptp4l-make-utc-offset-configurable.patch
Description: 0001-ptp4l-make-utc-offset-configurable.patch

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to