On 11/28/2020 9:08 AM, Richard Cochran wrote:
> This patch renames the function to have the module prefix and corrects the
> return code semantics.
> 
> The active word in the function's name is "query" rather that "get" in
> order to distinguish methods that send and receive over the network
> from those that merely return a cached value.
> 
> Signed-off-by: Richard Cochran <richardcoch...@gmail.com>

Reviewed-by: Jacob Keller <jacob.e.kel...@intel.com>

I appreciate the function rename too, it seems a bit more clear. (Plus,
if this were a project with many patches in flight, renaming the
function helps prevent potential non-text merge conflicts where a new
user was added but didn't update with the new return semantics).

> ---
>  phc2sys.c   |  8 +++----
>  pmc_agent.c | 63 +++++++++++++++++++++++++++--------------------------
>  pmc_agent.h | 16 ++++++++++++--
>  3 files changed, 50 insertions(+), 37 deletions(-)
> 
> diff --git a/phc2sys.c b/phc2sys.c
> index 999d20e..725a25c 100644
> --- a/phc2sys.c
> +++ b/phc2sys.c
> @@ -720,7 +720,7 @@ static int do_loop(struct phc2sys_private *priv, int 
> subscriptions)
>                       if (priv->state_changed) {
>                               /* force getting offset, as it may have
>                                * changed after the port state change */
> -                             if (run_pmc_get_utc_offset(priv->node, 1000) <= 
> 0) {
> +                             if (pmc_agent_query_utc_offset(priv->node, 
> 1000)) {
>                                       pr_err("failed to get UTC offset");
>                                       continue;
>                               }
> @@ -910,7 +910,7 @@ static int auto_init_ports(struct phc2sys_private *priv, 
> int add_rt)
>       }
>  
>       /* get initial offset */
> -     if (run_pmc_get_utc_offset(priv->node, 1000) <= 0) {
> +     if (pmc_agent_query_utc_offset(priv->node, 1000)) {
>               pr_err("failed to get UTC offset");
>               return -1;
>       }
> @@ -1305,8 +1305,8 @@ int main(int argc, char *argv[])
>               }
>  
>               if (!priv.forced_sync_offset) {
> -                     r = run_pmc_get_utc_offset(priv.node, 1000);
> -                     if (r <= 0) {
> +                     r = pmc_agent_query_utc_offset(priv.node, 1000);
> +                     if (r) {
>                               pr_err("failed to get UTC offset");
>                               goto end;
>                       }
> diff --git a/pmc_agent.c b/pmc_agent.c
> index 22af306..5e066ac 100644
> --- a/pmc_agent.c
> +++ b/pmc_agent.c
> @@ -228,36 +228,6 @@ int run_pmc_wait_sync(struct pmc_agent *node, int 
> timeout)
>       }
>  }
>  
> -int run_pmc_get_utc_offset(struct pmc_agent *node, int timeout)
> -{
> -     struct ptp_message *msg;
> -     int res;
> -     struct timePropertiesDS *tds;
> -
> -     res = run_pmc(node, timeout, TLV_TIME_PROPERTIES_DATA_SET, &msg);
> -     if (res <= 0)
> -             return res;
> -
> -     tds = (struct timePropertiesDS *) management_tlv_data(msg);
> -     if (tds->flags & PTP_TIMESCALE) {
> -             node->sync_offset = tds->currentUtcOffset;
> -             if (tds->flags & LEAP_61)
> -                     node->leap = 1;
> -             else if (tds->flags & LEAP_59)
> -                     node->leap = -1;
> -             else
> -                     node->leap = 0;
> -             node->utc_offset_traceable = tds->flags & UTC_OFF_VALID &&
> -                                          tds->flags & TIME_TRACEABLE;
> -     } else {
> -             node->sync_offset = 0;
> -             node->leap = 0;
> -             node->utc_offset_traceable = 0;
> -     }
> -     msg_put(msg);
> -     return 1;
> -}
> -
>  int run_pmc_get_number_ports(struct pmc_agent *node, int timeout)
>  {
>       struct ptp_message *msg;
> @@ -376,6 +346,37 @@ int pmc_agent_get_sync_offset(struct pmc_agent *agent)
>       return agent->sync_offset;
>  }
>  
> +int pmc_agent_query_utc_offset(struct pmc_agent *node, int timeout)
> +{
> +     struct timePropertiesDS *tds;
> +     struct ptp_message *msg;
> +     int res;
> +
> +     res = run_pmc(node, timeout, TLV_TIME_PROPERTIES_DATA_SET, &msg);
> +     if (is_run_pmc_error(res)) {
> +             return run_pmc_err2errno(res);
> +     }
> +
> +     tds = (struct timePropertiesDS *) management_tlv_data(msg);
> +     if (tds->flags & PTP_TIMESCALE) {
> +             node->sync_offset = tds->currentUtcOffset;
> +             if (tds->flags & LEAP_61)
> +                     node->leap = 1;
> +             else if (tds->flags & LEAP_59)
> +                     node->leap = -1;
> +             else
> +                     node->leap = 0;
> +             node->utc_offset_traceable = tds->flags & UTC_OFF_VALID &&
> +                                          tds->flags & TIME_TRACEABLE;
> +     } else {
> +             node->sync_offset = 0;
> +             node->leap = 0;
> +             node->utc_offset_traceable = 0;
> +     }
> +     msg_put(msg);
> +     return 0;
> +}
> +
>  void pmc_agent_set_sync_offset(struct pmc_agent *agent, int offset)
>  {
>       agent->sync_offset = offset;
> @@ -405,7 +406,7 @@ int pmc_agent_update(struct pmc_agent *node)
>               if (node->stay_subscribed) {
>                       renew_subscription(node, 0);
>               }
> -             if (run_pmc_get_utc_offset(node, 0) > 0) {
> +             if (!pmc_agent_query_utc_offset(node, 0)) {
>                       node->pmc_last_update = ts;
>               }
>       }
> diff --git a/pmc_agent.h b/pmc_agent.h
> index 483a21b..050d52d 100644
> --- a/pmc_agent.h
> +++ b/pmc_agent.h
> @@ -40,8 +40,6 @@ void run_pmc_events(struct pmc_agent *agent);
>  int run_pmc_port_properties(struct pmc_agent *agent, int timeout,
>                           unsigned int port, int *state,
>                           int *tstamping, char *iface);
> -int run_pmc_get_utc_offset(struct pmc_agent *agent, int timeout);
> -
>  
>  /**
>   * Creates an instance of a PMC agent.
> @@ -69,6 +67,20 @@ int pmc_agent_get_leap(struct pmc_agent *agent);
>   */
>  int pmc_agent_get_sync_offset(struct pmc_agent *agent);
>  
> +/**
> + * Queries the TAI-UTC offset and the current leap adjustment from the
> + * ptp4l service.
> + *
> + * In addition:
> + *
> + * - The port state notification callback might be invoked.
> + *
> + * @param agent  Pointer to a PMC instance obtained via @ref 
> pmc_agent_create().
> + * @param timeout  Transmit and receive timeout in milliseconds.
> + * @return         Zero on success, negative error code otherwise.
> + */
> +int pmc_agent_query_utc_offset(struct pmc_agent *agent, int timeout);
> +
>  /**
>   * Sets the TAI-UTC offset.
>   * @param agent  Pointer to a PMC instance obtained via @ref 
> pmc_agent_create().
> 


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

Reply via email to