Re: [Linuxptp-devel] [PATCH 1/4] pmc_agent: Convert the method that queries TAI-UTC offset into the canonical form.
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
Reviewed-by: Jacob Keller
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, );
> - 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, );
> + 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;
> +}
> +
[Linuxptp-devel] [PATCH 1/4] pmc_agent: Convert the method that queries TAI-UTC offset into the canonical form.
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
---
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, );
- 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, );
+ 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
