* [PATCH net-next 2/4] net: phy: dp83640: use enum ptp_msg_type
2020-11-17 19:31 [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type Christian Eggers
@ 2020-11-17 19:31 ` Christian Eggers
2020-11-17 19:31 ` [PATCH net-next 3/4] dpaa2-eth: " Christian Eggers
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Christian Eggers @ 2020-11-17 19:31 UTC (permalink / raw)
To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
linux-kernel, Christian Eggers, Richard Cochran, Kurt Kanzenbach
Use new return type of ptp_get_msgtype(). Remove usage of magic number.
Signed-off-by: Christian Eggers <ceggers@arri.de>
Cc: Richard Cochran <richard.cochran@omicron.at>
Cc: Kurt Kanzenbach <kurt@linutronix.de>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next 3/4] dpaa2-eth: use enum ptp_msg_type
2020-11-17 19:31 [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type Christian Eggers
2020-11-17 19:31 ` [PATCH net-next 2/4] net: phy: dp83640: use " Christian Eggers
@ 2020-11-17 19:31 ` Christian Eggers
2020-11-17 19:31 ` [PATCH net-next 4/4] ptp: ptp_ines: " Christian Eggers
2020-11-18 13:18 ` [PATCH net-next 1/4] net: ptp: introduce " Richard Cochran
3 siblings, 0 replies; 7+ messages in thread
From: Christian Eggers @ 2020-11-17 19:31 UTC (permalink / raw)
To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
linux-kernel, Christian Eggers, Ioana Ciornei, Ioana Radulescu,
Yangbo Lu
Use new return type of ptp_get_msgtype(). Remove usage of magic numbers.
Signed-off-by: Christian Eggers <ceggers@arri.de>
Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
Cc: Ioana Radulescu <ruxandra.radulescu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next 4/4] ptp: ptp_ines: use enum ptp_msg_type
2020-11-17 19:31 [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type Christian Eggers
2020-11-17 19:31 ` [PATCH net-next 2/4] net: phy: dp83640: use " Christian Eggers
2020-11-17 19:31 ` [PATCH net-next 3/4] dpaa2-eth: " Christian Eggers
@ 2020-11-17 19:31 ` Christian Eggers
2020-11-17 23:17 ` kernel test robot
2020-11-18 13:18 ` [PATCH net-next 1/4] net: ptp: introduce " Richard Cochran
3 siblings, 1 reply; 7+ messages in thread
From: Christian Eggers @ 2020-11-17 19:31 UTC (permalink / raw)
To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
linux-kernel, Christian Eggers, Kurt Kanzenbach
Use new return type of ptp_get_msgtype(). Remove driver internal defines
for this.
Signed-off-by: Christian Eggers <ceggers@arri.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Kurt Kanzenbach <kurt@linutronix.de>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next 4/4] ptp: ptp_ines: use enum ptp_msg_type
2020-11-17 19:31 ` [PATCH net-next 4/4] ptp: ptp_ines: " Christian Eggers
@ 2020-11-17 23:17 ` kernel test robot
2020-11-17 23:24 ` Vladimir Oltean
0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2020-11-17 23:17 UTC (permalink / raw)
To: Christian Eggers, Richard Cochran, Andrew Lunn, Heiner Kallweit,
Jakub Kicinski
Cc: kbuild-all, Vladimir Oltean, Russell King, David S . Miller,
netdev, linux-kernel, Christian Eggers
[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]
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 <lkp@intel.com>
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/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53484 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next 4/4] ptp: ptp_ines: use enum ptp_msg_type
2020-11-17 23:17 ` kernel test robot
@ 2020-11-17 23:24 ` Vladimir Oltean
0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Oltean @ 2020-11-17 23:24 UTC (permalink / raw)
To: Christian Eggers
Cc: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski,
Russell King, David S. Miller, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type
2020-11-17 19:31 [PATCH net-next 1/4] net: ptp: introduce enum ptp_msg_type Christian Eggers
` (2 preceding siblings ...)
2020-11-17 19:31 ` [PATCH net-next 4/4] ptp: ptp_ines: " Christian Eggers
@ 2020-11-18 13:18 ` Richard Cochran
3 siblings, 0 replies; 7+ messages in thread
From: Richard Cochran @ 2020-11-18 13:18 UTC (permalink / raw)
To: Christian Eggers
Cc: Andrew Lunn, Heiner Kallweit, Jakub Kicinski, Vladimir Oltean,
Russell King, David S . Miller, netdev, linux-kernel,
Kurt Kanzenbach
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
^ permalink raw reply [flat|nested] 7+ messages in thread