Using a PTP wide enum 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 | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
index 56b2d7d66177..83024220cb42 100644
--- a/include/linux/ptp_classify.h
+++ b/include/linux/ptp_classify.h
@@ -93,6 +93,13 @@ unsigned int ptp_classify_raw(const struct sk_buff *skb);
*/
struct ptp_header *ptp_parse_header(struct sk_buff *skb, unsigned int type);
+enum ptp_msg_type {
+ PTP_MSGTYPE_SYNC = 0x0,
+ PTP_MSGTYPE_DELAY_REQ = 0x1,
+ PTP_MSGTYPE_PDELAY_REQ = 0x2,
+ PTP_MSGTYPE_PDELAY_RESP = 0x3,
+};
+
/**
* ptp_get_msgtype - Extract ptp message type from given header
* @hdr: ptp header
@@ -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 */
@@ -132,13 +139,13 @@ static inline struct ptp_header *ptp_parse_header(struct sk_buff *skb,
{
return NULL;
}
-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)
{
/* 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
Use new return type of ptp_get_msgtype(). Remove usage of magic number.
Signed-off-by: Christian Eggers <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: Kurt Kanzenbach <[email protected]>
---
drivers/net/phy/dp83640.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index f2caccaf4408..bb68db3518bc 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -88,7 +88,7 @@ struct rxts {
unsigned long tmo;
u64 ns;
u16 seqid;
- u8 msgtype;
+ enum ptp_msg_type msgtype;
u16 hash;
};
@@ -803,7 +803,7 @@ static int decode_evnt(struct dp83640_private *dp83640,
static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
{
struct ptp_header *hdr;
- u8 msgtype;
+ enum ptp_msg_type msgtype;
u16 seqid;
u16 hash;
@@ -815,7 +815,7 @@ static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
msgtype = ptp_get_msgtype(hdr, type);
- if (rxts->msgtype != (msgtype & 0xf))
+ if (rxts->msgtype != msgtype)
return 0;
seqid = be16_to_cpu(hdr->sequence_id);
@@ -964,7 +964,7 @@ static void decode_status_frame(struct dp83640_private *dp83640,
static int is_sync(struct sk_buff *skb, int type)
{
struct ptp_header *hdr;
- u8 msgtype;
+ enum ptp_msg_type msgtype;
hdr = ptp_parse_header(skb, type);
if (!hdr)
@@ -972,7 +972,7 @@ static int is_sync(struct sk_buff *skb, int type)
msgtype = ptp_get_msgtype(hdr, type);
- return (msgtype & 0xf) == 0;
+ return msgtype == PTP_MSGTYPE_SYNC;
}
static void dp83640_free_clocks(void)
--
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
Use new return type of ptp_get_msgtype(). 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 | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index cf9400a9886d..7e6084124f8f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -617,7 +617,7 @@ static int dpaa2_eth_consume_frames(struct dpaa2_eth_channel *ch,
}
static int dpaa2_eth_ptp_parse(struct sk_buff *skb,
- u8 *msgtype, u8 *twostep, u8 *udp,
+ enum ptp_msg_type *msgtype, u8 *twostep, u8 *udp,
u16 *correction_offset,
u16 *origintimestamp_offset)
{
@@ -659,7 +659,7 @@ static void dpaa2_eth_enable_tx_tstamp(struct dpaa2_eth_priv *priv,
{
struct ptp_tstamp origin_timestamp;
struct dpni_single_step_cfg cfg;
- u8 msgtype, twostep, udp;
+ u8 twostep, udp;
struct dpaa2_faead *faead;
struct dpaa2_fas *fas;
struct timespec64 ts;
@@ -684,9 +684,11 @@ static void dpaa2_eth_enable_tx_tstamp(struct dpaa2_eth_priv *priv,
faead->ctrl = cpu_to_le32(ctrl);
if (skb->cb[0] == TX_TSTAMP_ONESTEP_SYNC) {
+ enum ptp_msg_type msgtype;
+
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;
}
@@ -1195,7 +1197,7 @@ static void dpaa2_eth_tx_onestep_tstamp(struct work_struct *work)
static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
{
struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
- u8 msgtype, twostep, udp;
+ u8 twostep, udp;
u16 offset1, offset2;
/* Utilize skb->cb[0] for timestamping request per skb */
@@ -1210,9 +1212,11 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
/* TX for one-step timestamping PTP Sync packet */
if (skb->cb[0] == TX_TSTAMP_ONESTEP_SYNC) {
+ enum ptp_msg_type msgtype;
+
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
Use new return type of ptp_get_msgtype(). Remove driver internal defines
for this.
Signed-off-by: Christian Eggers <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: Kurt Kanzenbach <[email protected]>
---
drivers/ptp/ptp_ines.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/ptp/ptp_ines.c b/drivers/ptp/ptp_ines.c
index 4700ffbdfced..4836c9c35604 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);
@@ -442,7 +437,7 @@ static bool ines_match(struct sk_buff *skb, unsigned int ptp_class,
{
struct ptp_header *hdr;
u16 portn, seqid;
- u8 msgtype;
+ enum ptp_msg_type msgtype;
u64 clkid;
if (unlikely(ptp_class & PTP_CLASS_V1))
@@ -675,7 +670,7 @@ static void ines_txtstamp_work(struct work_struct *work)
static bool is_sync_pdelay_resp(struct sk_buff *skb, int type)
{
struct ptp_header *hdr;
- u8 msgtype;
+ enum ptp_msg_type msgtype;
hdr = ptp_parse_header(skb, type);
if (!hdr)
@@ -683,26 +678,26 @@ 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;
}
}
-static u8 tag_to_msgtype(u8 tag)
+static enum ptp_msg_type 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
Hi Christian,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-ptp-introduce-enum-ptp_msg_type/20201118-033828
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 72308ecbf33b145641aba61071be31a85ebfd92c
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c4e4cfcabe3201e2ece90cc9025894e4ed08f069
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christian-Eggers/net-ptp-introduce-enum-ptp_msg_type/20201118-033828
git checkout c4e4cfcabe3201e2ece90cc9025894e4ed08f069
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
>> drivers/ptp/ptp_ines.c:690:26: error: conflicting types for 'tag_to_msgtype'
690 | static enum ptp_msg_type tag_to_msgtype(u8 tag)
| ^~~~~~~~~~~~~~
drivers/ptp/ptp_ines.c:178:11: note: previous declaration of 'tag_to_msgtype' was here
178 | static u8 tag_to_msgtype(u8 tag);
| ^~~~~~~~~~~~~~
>> drivers/ptp/ptp_ines.c:178:11: warning: 'tag_to_msgtype' used but never defined
drivers/ptp/ptp_ines.c:690:26: warning: 'tag_to_msgtype' defined but not used [-Wunused-function]
690 | static enum ptp_msg_type tag_to_msgtype(u8 tag)
| ^~~~~~~~~~~~~~
vim +/tag_to_msgtype +690 drivers/ptp/ptp_ines.c
689
> 690 static enum ptp_msg_type tag_to_msgtype(u8 tag)
691 {
692 switch (tag) {
693 case MESSAGE_TYPE_SYNC:
694 return PTP_MSGTYPE_SYNC;
695 case MESSAGE_TYPE_P_DELAY_REQ:
696 return PTP_MSGTYPE_PDELAY_REQ;
697 case MESSAGE_TYPE_P_DELAY_RESP:
698 return PTP_MSGTYPE_PDELAY_RESP;
699 case MESSAGE_TYPE_DELAY_REQ:
700 return PTP_MSGTYPE_DELAY_REQ;
701 }
702 return 0xf;
703 }
704
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Wed, Nov 18, 2020 at 07:17:41AM +0800, kernel test robot wrote:
> >> drivers/ptp/ptp_ines.c:690:26: error: conflicting types for 'tag_to_msgtype'
> 690 | static enum ptp_msg_type tag_to_msgtype(u8 tag)
> | ^~~~~~~~~~~~~~
> drivers/ptp/ptp_ines.c:178:11: note: previous declaration of 'tag_to_msgtype' was here
> 178 | static u8 tag_to_msgtype(u8 tag);
> | ^~~~~~~~~~~~~~
Wait for the patches to simmer a little bit more before resending. And
please make sure to create a cover letter when doing so.
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