On Tue, Nov 23, 2021 at 02:14:16AM +0200, Vladimir Oltean wrote:

> +static enum port_state ts2phc_normalize_state(enum port_state state)
> +{
> +     if (state != PS_MASTER && state != PS_SLAVE &&
> +         state != PS_PRE_MASTER && state != PS_UNCALIBRATED) {
> +             /* treat any other state as "not a master nor a slave" */
> +             state = PS_DISABLED;
> +     }
> +     return state;
> +}

Please make this explicit by using switch/case over all the enumerated values.

> +static int auto_init_ports(struct ts2phc_private *priv)
> +{
> +     int number_ports, timestamping, err;
> +     struct ts2phc_clock *clock;
> +     struct ts2phc_port *port;
> +     enum port_state state;
> +     char iface[IFNAMSIZ];
> +     unsigned int i;
> +
> +     while (1) {
> +             if (!is_running())
> +                     return -1;
> +             err = pmc_agent_query_dds(priv->agent, 1000);
> +             if (!err)
> +                     break;
> +             if (err == -ETIMEDOUT)
> +                     pr_notice("Waiting for ptp4l...");
> +             else
> +                     return -1;
> +     }
> +
> +     number_ports = pmc_agent_get_number_ports(priv->agent);
> +     if (number_ports <= 0) {
> +             pr_err("failed to get number of ports");
> +             return -1;
> +     }
> +
> +     err = pmc_agent_subscribe(priv->agent, 1000);
> +     if (err) {
> +             pr_err("failed to subscribe");
> +             return -1;
> +     }
> +
> +     for (i = 1; i <= number_ports; i++) {
> +             err = pmc_agent_query_port_properties(priv->agent, 1000, i,
> +                                                   &state, &timestamping,
> +                                                   iface);
> +             if (err == -ENODEV) {
> +                     /* port does not exist, ignore the port */
> +                     continue;
> +             }
> +             if (err) {
> +                     pr_err("failed to get port properties");
> +                     return -1;
> +             }
> +             if (timestamping == TS_SOFTWARE) {
> +                     /* ignore ports with software time stamping */
> +                     continue;
> +             }
> +             port = ts2phc_port_add(priv, i, iface);
> +             if (!port)
> +                     return -1;
> +             port->state = ts2phc_normalize_state(state);
> +     }
> +     if (LIST_EMPTY(&priv->clocks)) {
> +             pr_err("no suitable ports available");
> +             return -1;
> +     }
> +     LIST_FOREACH(clock, &priv->clocks, list) {
> +             clock->new_state = ts2phc_clock_compute_state(priv, clock);
> +     }
> +
> +     return 0;
> +}

This is very similar to the code in phc2sys.  Can't you refactor to
share that logic?

Thanks,
Richard


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

Reply via email to