Messages in this thread | | | Date | Wed, 18 Nov 2020 05:18:47 -0800 | From | Richard Cochran <> | Subject | Re: [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type |
| |
On Tue, Nov 17, 2020 at 08:31:21PM +0100, Christian Eggers wrote: > Using a PTP wide enum will obsolete different driver internal defines > and uses of magic numbers.
I like the general idea, but ...
> +enum ptp_msg_type { > + PTP_MSGTYPE_SYNC = 0x0, > + PTP_MSGTYPE_DELAY_REQ = 0x1, > + PTP_MSGTYPE_PDELAY_REQ = 0x2, > + PTP_MSGTYPE_PDELAY_RESP = 0x3, > +};
I would argue that these should be #defines. After all, they are protocol data fields and not an abstract enumerated types.
For example ...
> @@ -103,10 +110,10 @@ struct ptp_header *ptp_parse_header(struct sk_buff *skb, unsigned int type); > * > * Return: The message type > */ > -static inline u8 ptp_get_msgtype(const struct ptp_header *hdr, > +static inline enum ptp_msg_type ptp_get_msgtype(const struct ptp_header *hdr, > unsigned int type) > { > - u8 msgtype; > + enum ptp_msg_type msgtype; > > if (unlikely(type & PTP_CLASS_V1)) { > /* msg type is located at the control field for ptp v1 */
msgtype = hdr->control; } else { msgtype = hdr->tsmt & 0x0f;
This assigns directly from protocol data into an enumerated type.
}
return msgtype;
Thanks, Richard
| |