2020-11-20 08:44:01

by Christian Eggers

[permalink] [raw]
Subject: [PATCH net-next v3 0/3] net: ptp: introduce common defines for PTP message types

This series introduces commen defines for PTP event messages. Driver
internal defines are removed and some uses of magic numbers are replaced
by the new defines.

Changes v2 --> v3
------------------
- extend commit description for ptp_ines (Jacob Keller)

Changes v1 --> v2
------------------
- use defines instead of an enum (Richard Cochran)
- no changes necessary for dp63640
- add cover message (Vladimir Oltean)



2020-11-20 08:47:19

by Christian Eggers

[permalink] [raw]
Subject: [PATCH net-next v3 1/3] net: ptp: introduce common defines for PTP message types

Using PTP wide defines will obsolete different driver internal defines
and uses of magic numbers.

Signed-off-by: Christian Eggers <[email protected]>
Cc: Kurt Kanzenbach <[email protected]>
---
include/linux/ptp_classify.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
index 56b2d7d66177..cc0da0b134a4 100644
--- a/include/linux/ptp_classify.h
+++ b/include/linux/ptp_classify.h
@@ -31,6 +31,11 @@
#define PTP_CLASS_V2_VLAN (PTP_CLASS_V2 | PTP_CLASS_VLAN)
#define PTP_CLASS_L4 (PTP_CLASS_IPV4 | PTP_CLASS_IPV6)

+#define PTP_MSGTYPE_SYNC 0x0
+#define PTP_MSGTYPE_DELAY_REQ 0x1
+#define PTP_MSGTYPE_PDELAY_REQ 0x2
+#define PTP_MSGTYPE_PDELAY_RESP 0x3
+
#define PTP_EV_PORT 319
#define PTP_GEN_BIT 0x08 /* indicates general message, if set in message type */

@@ -138,7 +143,7 @@ static inline u8 ptp_get_msgtype(const struct ptp_header *hdr,
/* The return is meaningless. The stub function would not be
* executed since no available header from ptp_parse_header.
*/
- return 0;
+ return PTP_MSGTYPE_SYNC;
}
#endif
#endif /* _PTP_CLASSIFY_H_ */
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

2020-11-20 08:48:00

by Christian Eggers

[permalink] [raw]
Subject: [PATCH net-next v3 2/3] dpaa2-eth: use new PTP_MSGTYPE_* define(s)

Remove usage of magic numbers.

Signed-off-by: Christian Eggers <[email protected]>
Cc: Ioana Ciornei <[email protected]>
Cc: Ioana Radulescu <[email protected]>
Cc: Yangbo Lu <[email protected]>
---
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index cf9400a9886d..a0a30c721fe7 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -686,7 +686,7 @@ static void dpaa2_eth_enable_tx_tstamp(struct dpaa2_eth_priv *priv,
if (skb->cb[0] == TX_TSTAMP_ONESTEP_SYNC) {
if (dpaa2_eth_ptp_parse(skb, &msgtype, &twostep, &udp,
&offset1, &offset2) ||
- msgtype != 0 || twostep) {
+ msgtype != PTP_MSGTYPE_SYNC || twostep) {
WARN_ONCE(1, "Bad packet for one-step timestamping\n");
return;
}
@@ -1212,7 +1212,7 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
if (skb->cb[0] == TX_TSTAMP_ONESTEP_SYNC) {
if (!dpaa2_eth_ptp_parse(skb, &msgtype, &twostep, &udp,
&offset1, &offset2))
- if (msgtype == 0 && twostep == 0) {
+ if (msgtype == PTP_MSGTYPE_SYNC && twostep == 0) {
skb_queue_tail(&priv->tx_skbs, skb);
queue_work(priv->dpaa2_ptp_wq,
&priv->tx_onestep_tstamp);
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

2020-11-20 08:48:38

by Christian Eggers

[permalink] [raw]
Subject: [PATCH net-next v3 3/3] ptp: ptp_ines: use new PTP_MSGTYPE_* define(s)

Remove driver internal defines for this. Masking msgtype with 0xf is
already done within ptp_get_msgtype().

Signed-off-by: Christian Eggers <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: Kurt Kanzenbach <[email protected]>
---
drivers/ptp/ptp_ines.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/ptp/ptp_ines.c b/drivers/ptp/ptp_ines.c
index 4700ffbdfced..6c7c2843ba0b 100644
--- a/drivers/ptp/ptp_ines.c
+++ b/drivers/ptp/ptp_ines.c
@@ -108,11 +108,6 @@ MODULE_LICENSE("GPL");
#define MESSAGE_TYPE_P_DELAY_RESP 3
#define MESSAGE_TYPE_DELAY_REQ 4

-#define SYNC 0x0
-#define DELAY_REQ 0x1
-#define PDELAY_REQ 0x2
-#define PDELAY_RESP 0x3
-
static LIST_HEAD(ines_clocks);
static DEFINE_MUTEX(ines_clocks_lock);

@@ -683,9 +678,9 @@ static bool is_sync_pdelay_resp(struct sk_buff *skb, int type)

msgtype = ptp_get_msgtype(hdr, type);

- switch ((msgtype & 0xf)) {
- case SYNC:
- case PDELAY_RESP:
+ switch (msgtype) {
+ case PTP_MSGTYPE_SYNC:
+ case PTP_MSGTYPE_PDELAY_RESP:
return true;
default:
return false;
@@ -696,13 +691,13 @@ static u8 tag_to_msgtype(u8 tag)
{
switch (tag) {
case MESSAGE_TYPE_SYNC:
- return SYNC;
+ return PTP_MSGTYPE_SYNC;
case MESSAGE_TYPE_P_DELAY_REQ:
- return PDELAY_REQ;
+ return PTP_MSGTYPE_PDELAY_REQ;
case MESSAGE_TYPE_P_DELAY_RESP:
- return PDELAY_RESP;
+ return PTP_MSGTYPE_PDELAY_RESP;
case MESSAGE_TYPE_DELAY_REQ:
- return DELAY_REQ;
+ return PTP_MSGTYPE_DELAY_REQ;
}
return 0xf;
}
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

2020-11-20 13:39:14

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/3] net: ptp: introduce common defines for PTP message types

On Fri, Nov 20, 2020 at 09:41:03AM +0100, Christian Eggers wrote:
> This series introduces commen defines for PTP event messages. Driver
> internal defines are removed and some uses of magic numbers are replaced
> by the new defines.

Nice cleanup!

Reviewed-by: Richard Cochran <[email protected]>

Thanks,
Richard

2020-11-20 22:33:36

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 1/3] net: ptp: introduce common defines for PTP message types

On Fri, Nov 20, 2020 at 09:41:04AM +0100, Christian Eggers wrote:
> Using PTP wide defines will obsolete different driver internal defines
> and uses of magic numbers.
>
> Signed-off-by: Christian Eggers <[email protected]>
> Cc: Kurt Kanzenbach <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2020-11-20 22:35:50

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 2/3] dpaa2-eth: use new PTP_MSGTYPE_* define(s)

On Fri, Nov 20, 2020 at 09:41:05AM +0100, Christian Eggers wrote:
> Remove usage of magic numbers.
>
> Signed-off-by: Christian Eggers <[email protected]>
> Cc: Ioana Ciornei <[email protected]>
> Cc: Ioana Radulescu <[email protected]>
> Cc: Yangbo Lu <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2020-11-20 22:37:14

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 3/3] ptp: ptp_ines: use new PTP_MSGTYPE_* define(s)

On Fri, Nov 20, 2020 at 09:41:06AM +0100, Christian Eggers wrote:
> Remove driver internal defines for this. Masking msgtype with 0xf is
> already done within ptp_get_msgtype().
>
> Signed-off-by: Christian Eggers <[email protected]>
> Cc: Richard Cochran <[email protected]>
> Cc: Kurt Kanzenbach <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2020-11-20 22:41:01

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/3] net: ptp: introduce common defines for PTP message types

On Fri, Nov 20, 2020 at 09:41:03AM +0100, Christian Eggers wrote:
> This series introduces commen defines for PTP event messages. Driver
> internal defines are removed and some uses of magic numbers are replaced
> by the new defines.
>
> Changes v2 --> v3
> ------------------
> - extend commit description for ptp_ines (Jacob Keller)
>
> Changes v1 --> v2
> ------------------
> - use defines instead of an enum (Richard Cochran)
> - no changes necessary for dp63640
> - add cover message (Vladimir Oltean)

I understand that you don't want to spend a lifetime on this, but I see
that there are more drivers which you did not touch.

is_sync() in drivers/net/phy/dp83640.c can be made to
return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;

this can be removed from drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h:
enum {
MLXSW_SP_PTP_MESSAGE_TYPE_SYNC,
MLXSW_SP_PTP_MESSAGE_TYPE_DELAY_REQ,
MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_REQ,
MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_RESP,
};

Either way, this can also be applied as-is, since there's nothing wrong
with it.

2020-11-23 22:05:28

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/3] net: ptp: introduce common defines for PTP message types

On Fri, 20 Nov 2020 09:41:03 +0100 Christian Eggers wrote:
> This series introduces commen defines for PTP event messages. Driver
> internal defines are removed and some uses of magic numbers are replaced
> by the new defines.

Applied, thanks!