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