On Thu, Jan 06, 2022 at 06:19:01PM +0800, Hangbin Liu wrote:
> Latest Linux kernel has supported getting PHC from bond directly. This would
> help topology like VLAN over bond to get PHC natively. To achieve this,
> the new hwtstamp flag is needed when passing hwtstamp_config to kernel.

> @@ -1001,10 +1001,18 @@ struct clock *clock_create(enum clock_type type, 
> struct config *config,
>       c->timestamping = timestamping;
>       required_modes = clock_required_modes(c);
>       STAILQ_FOREACH(iface, &config->interfaces, list) {
> -             memset(ts_label, 0, sizeof(ts_label));
> -             if (!rtnl_get_ts_device(interface_name(iface), ts_label))
> -                     interface_set_label(iface, ts_label);
> -             interface_get_tsinfo(iface);
> +#ifdef HWTSTAMP_FLAG_BONDED_PHC_INDEX
> +             if (interface_get_tsinfo(iface) ||
> +                 (interface_tsinfo_valid(iface) &&
> +                  !interface_tsmodes_supported(iface, required_modes))) {
> +#endif

Why is this and the other parts in the patch wrapped in the ifdef?
I'd expect the kernel feature to matter only in sk.c. If not using
bonding, the code should work the same no matter the kernel has a new
flag, right?

> +                     rtnl_link_status(fd, p->name, port_link_status, p);
> +
>               if (p->link_status == (LINK_UP|LINK_STATE_CHANGED)) {
>                       return EV_FAULT_CLEARED;
>               } else if ((p->link_status == (LINK_DOWN|LINK_STATE_CHANGED)) ||
> -                        (p->link_status & TS_LABEL_CHANGED)) {
> +                        (p->link_status & TS_LABEL_CHANGED) ||
> +                        (p->link_status & PHC_INDEX_CHANGED)) {

I think this and the other instances can be simplified to
                           (p->link_status & 
(TS_LABEL_CHANGED|PHC_INDEX_CHANGED))

> @@ -41,6 +41,7 @@ enum link_state {
>       LINK_UP  = (1<<1),
>       LINK_STATE_CHANGED = (1<<3),
>       TS_LABEL_CHANGED  = (1<<4),
> +     PHC_INDEX_CHANGED  = (1<<5),
>  };

Just wondering, is the new state necessary? Would it be simpler to
handle it as a special case of TS_LABEL_CHANGED, where the label
"changed" to the same value, only refreshing the PHC index?

-- 
Miroslav Lichvar



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

Reply via email to