[PATCH net-next 0/3] net: ptp: use common defines for PTP message types in further drivers

[PATCH net-next 0/3] net: ptp: use common defines for PTP message types in further drivers

[Christian Eggers]

This series replaces further driver internal enumeration / uses of magic
numbers with the newly introduced PTP_MSGTYPE_* defines.

On Friday, 20 November 2020, 23:39:10 CET, Vladimir Oltean wrote:
> 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.
> > [...]
> 
> 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,
> };
I think that I have found an addtional one in the Microsemi VSC85xx PHY driver.

[PATCH net-next 1/3] net: phy: dp83640: use new PTP_MSGTYPE_SYNC define

[Christian Eggers]

Replace use of magic number with recently introduced define.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/phy/dp83640.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index f2caccaf4408..9757ca0d9633 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -964,15 +964,12 @@ 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;
 
 	hdr = ptp_parse_header(skb, type);
 	if (!hdr)
 		return 0;
 
-	msgtype = ptp_get_msgtype(hdr, type);
-
-	return (msgtype & 0xf) == 0;
+	return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
 }
 
 static void dp83640_free_clocks(void)
-- 
Christian Eggers
Embedded software developer

[PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Christian Eggers]

Use recently introduced PTP wide defines instead of a driver internal
enumeration.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Petr Machata <petrm@mellanox.com>
Cc: Jiri Pirko <jiri@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 8 ++++----
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 7 -------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index ca8090a28dec..d6e9ecb14681 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -828,10 +828,10 @@ struct mlxsw_sp_ptp_state *mlxsw_sp1_ptp_init(struct mlxsw_sp *mlxsw_sp)
 		goto err_hashtable_init;
 
 	/* Delive these message types as PTP0. */
-	message_type = BIT(MLXSW_SP_PTP_MESSAGE_TYPE_SYNC) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_DELAY_REQ) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_REQ) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_RESP);
+	message_type = BIT(PTP_MSGTYPE_SYNC) |
+		       BIT(PTP_MSGTYPE_DELAY_REQ) |
+		       BIT(PTP_MSGTYPE_PDELAY_REQ) |
+		       BIT(PTP_MSGTYPE_PDELAY_RESP);
 	err = mlxsw_sp_ptp_mtptpt_set(mlxsw_sp, MLXSW_REG_MTPTPT_TRAP_ID_PTP0,
 				      message_type);
 	if (err)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
index 8c386571afce..1d43a3755285 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -11,13 +11,6 @@ struct mlxsw_sp;
 struct mlxsw_sp_port;
 struct mlxsw_sp_ptp_clock;
 
-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,
-};
-
 static inline int mlxsw_sp_ptp_get_ts_info_noptp(struct ethtool_ts_info *info)
 {
 	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-- 
Christian Eggers
Embedded software developer

[PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines

[Christian Eggers]

Use recently introduced PTP_MSGTYPE_SYNC and PTP_MSGTYPE_DELAY_REQ
defines instead of a driver internal enumeration.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Quentin Schulz <quentin.schulz@bootlin.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_ptp.c | 14 +++++++-------
 drivers/net/phy/mscc/mscc_ptp.h |  5 -----
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index d8a61456d1ce..924ed5b034a4 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -506,9 +506,9 @@ static int vsc85xx_ptp_cmp_init(struct phy_device *phydev, enum ts_blk blk)
 {
 	struct vsc8531_private *vsc8531 = phydev->priv;
 	bool base = phydev->mdio.addr == vsc8531->ts_base_addr;
-	enum vsc85xx_ptp_msg_type msgs[] = {
-		PTP_MSG_TYPE_SYNC,
-		PTP_MSG_TYPE_DELAY_REQ
+	u8 msgs[] = {
+		PTP_MSGTYPE_SYNC,
+		PTP_MSGTYPE_DELAY_REQ
 	};
 	u32 val;
 	u8 i;
@@ -847,9 +847,9 @@ static int vsc85xx_ts_ptp_action_flow(struct phy_device *phydev, enum ts_blk blk
 static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
 			    bool one_step, bool enable)
 {
-	enum vsc85xx_ptp_msg_type msgs[] = {
-		PTP_MSG_TYPE_SYNC,
-		PTP_MSG_TYPE_DELAY_REQ
+	u8 msgs[] = {
+		PTP_MSGTYPE_SYNC,
+		PTP_MSGTYPE_DELAY_REQ
 	};
 	u32 val;
 	u8 i;
@@ -858,7 +858,7 @@ static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
 		if (blk == INGRESS)
 			vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
 						   PTP_WRITE_NS);
-		else if (msgs[i] == PTP_MSG_TYPE_SYNC && one_step)
+		else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
 			/* no need to know Sync t when sending in one_step */
 			vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
 						   PTP_WRITE_1588);
diff --git a/drivers/net/phy/mscc/mscc_ptp.h b/drivers/net/phy/mscc/mscc_ptp.h
index 3ea163af0f4f..da3465360e90 100644
--- a/drivers/net/phy/mscc/mscc_ptp.h
+++ b/drivers/net/phy/mscc/mscc_ptp.h
@@ -436,11 +436,6 @@ enum ptp_cmd {
 	PTP_SAVE_IN_TS_FIFO = 11, /* invalid when writing in reg */
 };
 
-enum vsc85xx_ptp_msg_type {
-	PTP_MSG_TYPE_SYNC,
-	PTP_MSG_TYPE_DELAY_REQ,
-};
-
 struct vsc85xx_ptphdr {
 	u8 tsmt; /* transportSpecific | messageType */
 	u8 ver;  /* reserved0 | versionPTP */
-- 
Christian Eggers
Embedded software developer

Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]

Hi Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
config: sparc-randconfig-r012-20201122 (attached as .config)
compiler: sparc64-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/78cc4b0e1739511ed9712c9466a48ddc6885d153
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
        git checkout 78cc4b0e1739511ed9712c9466a48ddc6885d153
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_cmp_init':
   drivers/net/phy/mscc/mscc_ptp.c:510:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     510 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
   drivers/net/phy/mscc/mscc_ptp.c:510:3: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/phy/mscc/mscc_ptp.c:511:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     511 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_conf':
   drivers/net/phy/mscc/mscc_ptp.c:851:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     851 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
>> drivers/net/phy/mscc/mscc_ptp.c:851:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:851:3: note: (near initialization for 'msgs[0]')
   drivers/net/phy/mscc/mscc_ptp.c:852:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     852 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c:852:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:852:3: note: (near initialization for 'msgs[1]')
>> drivers/net/phy/mscc/mscc_ptp.c:861:20: warning: comparison between pointer and integer
     861 |   else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
         |                    ^~

vim +851 drivers/net/phy/mscc/mscc_ptp.c

   846	
   847	static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
   848				    bool one_step, bool enable)
   849	{
   850		u8 msgs[] = {
 > 851			PTP_MSGTYPE_SYNC,
   852			PTP_MSGTYPE_DELAY_REQ
   853		};
   854		u32 val;
   855		u8 i;
   856	
   857		for (i = 0; i < ARRAY_SIZE(msgs); i++) {
   858			if (blk == INGRESS)
   859				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   860							   PTP_WRITE_NS);
 > 861			else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
   862				/* no need to know Sync t when sending in one_step */
   863				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   864							   PTP_WRITE_1588);
   865			else
   866				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   867							   PTP_SAVE_IN_TS_FIFO);
   868	
   869			val = vsc85xx_ts_read_csr(phydev, blk,
   870						  MSCC_ANA_PTP_FLOW_ENA(i));
   871			val &= ~PTP_FLOW_ENA;
   872			if (enable)
   873				val |= PTP_FLOW_ENA;
   874			vsc85xx_ts_write_csr(phydev, blk, MSCC_ANA_PTP_FLOW_ENA(i),
   875					     val);
   876		}
   877	
   878		return 0;
   879	}
   880	

---
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: 29585 bytes --]

Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4450 bytes --]

Hi Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
config: sparc-randconfig-r012-20201122 (attached as .config)
compiler: sparc64-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/78cc4b0e1739511ed9712c9466a48ddc6885d153
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
        git checkout 78cc4b0e1739511ed9712c9466a48ddc6885d153
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_cmp_init':
   drivers/net/phy/mscc/mscc_ptp.c:510:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     510 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
   drivers/net/phy/mscc/mscc_ptp.c:510:3: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/phy/mscc/mscc_ptp.c:511:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     511 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_conf':
   drivers/net/phy/mscc/mscc_ptp.c:851:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     851 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
>> drivers/net/phy/mscc/mscc_ptp.c:851:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:851:3: note: (near initialization for 'msgs[0]')
   drivers/net/phy/mscc/mscc_ptp.c:852:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     852 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c:852:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:852:3: note: (near initialization for 'msgs[1]')
>> drivers/net/phy/mscc/mscc_ptp.c:861:20: warning: comparison between pointer and integer
     861 |   else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
         |                    ^~

vim +851 drivers/net/phy/mscc/mscc_ptp.c

   846	
   847	static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
   848				    bool one_step, bool enable)
   849	{
   850		u8 msgs[] = {
 > 851			PTP_MSGTYPE_SYNC,
   852			PTP_MSGTYPE_DELAY_REQ
   853		};
   854		u32 val;
   855		u8 i;
   856	
   857		for (i = 0; i < ARRAY_SIZE(msgs); i++) {
   858			if (blk == INGRESS)
   859				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   860							   PTP_WRITE_NS);
 > 861			else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
   862				/* no need to know Sync t when sending in one_step */
   863				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   864							   PTP_WRITE_1588);
   865			else
   866				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   867							   PTP_SAVE_IN_TS_FIFO);
   868	
   869			val = vsc85xx_ts_read_csr(phydev, blk,
   870						  MSCC_ANA_PTP_FLOW_ENA(i));
   871			val &= ~PTP_FLOW_ENA;
   872			if (enable)
   873				val |= PTP_FLOW_ENA;
   874			vsc85xx_ts_write_csr(phydev, blk, MSCC_ANA_PTP_FLOW_ENA(i),
   875					     val);
   876		}
   877	
   878		return 0;
   879	}
   880	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29585 bytes --]

Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines

[Christian Eggers]

[-- Attachment #1: Type: text/plain, Size: 5694 bytes --]

This error will disappear as soon as my series



"net: ptp: introduce common defines for PTP message types" [1]



has been merged.



regards

Christian



[1] https://patchwork.ozlabs.org/project/netdev/list/?series=215721



On Sunday, 22 November 2020, 10:42:31 CET, kernel test robot wrote:

> Hi Christian,

>

> I love your patch! Perhaps something to improve:

>

> [auto build test WARNING on net-next/master]

>

> url:

> https://github.com/0day-ci/linux/commits/Christian-Eggers/net-ptp-use-commo

> n-defines-for-PTP-message-types-in-further-drivers/20201122-163319 base:

> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git

> f9e425e99b0756c1479042afe761073779df2a30 config:

> sparc-randconfig-r012-20201122 (attached as .config)

> compiler: sparc64-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/78cc4b0e1739511ed9712c9466a48ddc688

> 5d153 git remote add linux-review https://github.com/0day-ci/linux git fetch

> --no-tags linux-review

> Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-furthe

> r-drivers/20201122-163319 git checkout

> 78cc4b0e1739511ed9712c9466a48ddc6885d153

> # save the attached .config to linux build tree

> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross

> ARCH=sparc

>

> If you fix the issue, kindly add following tag as appropriate

> Reported-by: kernel test robot <lkp@intel.com>

>

> All warnings (new ones prefixed by >>):

>

> drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_cmp_init':

> drivers/net/phy/mscc/mscc_ptp.c:510:3: error: 'PTP_MSGTYPE_SYNC'

> undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'? 510

> | PTP_MSGTYPE_SYNC,

>

> | ^~~~~~~~~~~~~~~~

> | CSD_TYPE_SYNC

>

> drivers/net/phy/mscc/mscc_ptp.c:510:3: note: each undeclared identifier

> is reported only once for each function it appears in

> drivers/net/phy/mscc/mscc_ptp.c:511:3: error: 'PTP_MSGTYPE_DELAY_REQ'

> undeclared (first use in this function) 511 | PTP_MSGTYPE_DELAY_REQ

>

> | ^~~~~~~~~~~~~~~~~~~~~

>

> drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_conf':

> drivers/net/phy/mscc/mscc_ptp.c:851:3: error: 'PTP_MSGTYPE_SYNC'

> undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'? 851

> | PTP_MSGTYPE_SYNC,

>

> | ^~~~~~~~~~~~~~~~

> | CSD_TYPE_SYNC

> >>

> >> drivers/net/phy/mscc/mscc_ptp.c:851:3: warning: initialization of

> >> 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from

> >> pointer without a cast [-Wint-conversion]

> drivers/net/phy/mscc/mscc_ptp.c:851:3: note: (near initialization for

> 'msgs[0]') drivers/net/phy/mscc/mscc_ptp.c:852:3: error:

> 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function) 852 |

> PTP_MSGTYPE_DELAY_REQ

>

> | ^~~~~~~~~~~~~~~~~~~~~

>

> drivers/net/phy/mscc/mscc_ptp.c:852:3: warning: initialization of

> 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from

> pointer without a cast [-Wint-conversion]

> drivers/net/phy/mscc/mscc_ptp.c:852:3: note: (near initialization for

> 'msgs[1]')

> >> drivers/net/phy/mscc/mscc_ptp.c:861:20: warning: comparison between

> >> pointer and integer

> 861 | else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)

>

> | ^~

>

> vim +851 drivers/net/phy/mscc/mscc_ptp.c

>

> 846

> 847 static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk

> blk, 848 bool one_step, bool enable)

> 849 {

> 850 u8 msgs[] = {

>

> > 851 PTP_MSGTYPE_SYNC,

>

> 852 PTP_MSGTYPE_DELAY_REQ

> 853 };

> 854 u32 val;

> 855 u8 i;

> 856

> 857 for (i = 0; i < ARRAY_SIZE(msgs); i++) {

> 858 if (blk == INGRESS)

> 859 vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],

> 860 PTP_WRITE_NS);

>

> > 861 else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)

>

> 862 /* no need to know Sync t when sending in one_step */

> 863 vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],

> 864 PTP_WRITE_1588);

> 865 else

> 866 vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],

> 867 PTP_SAVE_IN_TS_FIFO);

> 868

> 869 val = vsc85xx_ts_read_csr(phydev, blk,

> 870 MSCC_ANA_PTP_FLOW_ENA(i));

> 871 val &= ~PTP_FLOW_ENA;

> 872 if (enable)

> 873 val |= PTP_FLOW_ENA;

> 874 vsc85xx_ts_write_csr(phydev, blk, MSCC_ANA_PTP_FLOW_ENA(i),

> 875 val);

> 876 }

> 877

> 878 return 0;

> 879 }

> 880

>

> ---

> 0-DAY CI Kernel Test Service, Intel Corporation

> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org





________________________________
[http://assets.arri.com/media/sign/2020-04-03-E-mail-signature-Stellar2_V1.jpg]<https://microsites.arri.com/stellar/>

Get all the latest information from www.arri.com<https://www.arri.com/>, Facebook<https://www.facebook.com/TeamARRI>, Twitter<https://twitter.com/ARRIChannel>, Instagram<https://instagram.com/arri> and YouTube<https://www.youtube.com/user/ARRIChannel>.

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: München - Registergericht: Amtsgericht München - Handelsregisternummer: HRA 57918
Persönlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: München - Registergericht: Amtsgericht München - Handelsregisternummer: HRB 54477
Geschäftsführer: Dr. Michael Neuhäuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 25974 bytes --]

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Ido Schimmel]

On Sun, Nov 22, 2020 at 09:26:35AM +0100, Christian Eggers wrote:
> Use recently introduced PTP wide defines instead of a driver internal
> enumeration.
> 
> Signed-off-by: Christian Eggers <ceggers@gmx.de>
> Cc: Petr Machata <petrm@mellanox.com>
> Cc: Jiri Pirko <jiri@nvidia.com>
> Cc: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

But:

1. Checkpatch complains about:
WARNING: From:/Signed-off-by: email address mismatch: 'From: Christian Eggers <ceggers@arri.de>' != 'Signed-off-by: Christian Eggers <ceggers@gmx.de>'

2. This series does not build, which fails the CI [1][2] and also
required me to fetch the dependencies that are currently under review
[3]. I believe it is generally discouraged to create dependencies
between patch sets that are under review for exactly these reasons. I
don't know what are Jakub's preferences, but had this happened on our
internal patchwork instance, I would just ask the author to submit
another version with all the patches.

Anyway, I added all six patches to our regression as we have some PTP
tests. Will let you know tomorrow.

Thanks

[1] https://lore.kernel.org/netdev/20201122082636.12451-1-ceggers@arri.de/T/#mcef35858585d23b72b8f75450a51618d5c5d3260
[2] https://patchwork.hopto.org/static/nipa/389053/11923809/build_allmodconfig_warn/summary
[3] https://patchwork.kernel.org/project/netdevbpf/cover/20201120084106.10046-1-ceggers@arri.de/

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Christian Eggers]

On Sunday, 22 November 2020, 15:35:55 CET, Ido Schimmel wrote:
> On Sun, Nov 22, 2020 at 09:26:35AM +0100, Christian Eggers wrote:
> > Use recently introduced PTP wide defines instead of a driver internal
> > enumeration.
> > 
> > Signed-off-by: Christian Eggers <ceggers@gmx.de>
> > Cc: Petr Machata <petrm@mellanox.com>
> > Cc: Jiri Pirko <jiri@nvidia.com>
> > Cc: Ido Schimmel <idosch@nvidia.com>
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> 
> But:
> 
> 1. Checkpatch complains about:
> WARNING: From:/Signed-off-by: email address mismatch: 'From: Christian
> Eggers <ceggers@arri.de>' != 'Signed-off-by: Christian Eggers
> <ceggers@gmx.de>'
unfortunately I changed this after running checkpatch. My intention was to 
separate my (private) weekend work from the patches I do while I'm on the job.

> 2. This series does not build, which fails the CI [1][2] and also
> required me to fetch the dependencies that are currently under review
> [3]. I believe it is generally discouraged to create dependencies
> between patch sets that are under review for exactly these reasons. 
this was also not by intention. Vladimir found some files I missed in the
first series. As the whole first series had already been reviewed at that time,
I wasn't sure whether I am allowed to add further patches to it. Additionally
I didn't concern that although my local build is successful, I should wait
until the first series is applied...

> I don't know what are Jakub's preferences, but had this happened on our
> internal patchwork instance, I would just ask the author to submit
> another version with all the patches.
Please let me know how I shall proceed...

> Anyway, I added all six patches to our regression as we have some PTP
> tests. Will let you know tomorrow.
> 
> Thanks
> 
> [1]
> https://lore.kernel.org/netdev/20201122082636.12451-1-ceggers@arri.de/T/#mc
> ef35858585d23b72b8f75450a51618d5c5d3260 [2]
> https://patchwork.hopto.org/static/nipa/389053/11923809/build_allmodconfig_
> warn/summary [3]
> https://patchwork.kernel.org/project/netdevbpf/cover/20201120084106.10046-1
> -ceggers@arri.de/

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Ido Schimmel]

On Sun, Nov 22, 2020 at 08:29:22PM +0100, Christian Eggers wrote:
> On Sunday, 22 November 2020, 15:35:55 CET, Ido Schimmel wrote:
> > On Sun, Nov 22, 2020 at 09:26:35AM +0100, Christian Eggers wrote:
> > > Use recently introduced PTP wide defines instead of a driver internal
> > > enumeration.
> > >
> > > Signed-off-by: Christian Eggers <ceggers@gmx.de>
> > > Cc: Petr Machata <petrm@mellanox.com>
> > > Cc: Jiri Pirko <jiri@nvidia.com>
> > > Cc: Ido Schimmel <idosch@nvidia.com>
> >
> > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> >
> > But:
> >
> > 1. Checkpatch complains about:
> > WARNING: From:/Signed-off-by: email address mismatch: 'From: Christian
> > Eggers <ceggers@arri.de>' != 'Signed-off-by: Christian Eggers
> > <ceggers@gmx.de>'
> unfortunately I changed this after running checkpatch. My intention was to
> separate my (private) weekend work from the patches I do while I'm on the job.

No problem. Just make sure that authorship and Signed-off-by agree. You
can use:

# git commit --amend --author="Christian Eggers <ceggers@gmx.de>"

> 
> > 2. This series does not build, which fails the CI [1][2] and also
> > required me to fetch the dependencies that are currently under review
> > [3]. I believe it is generally discouraged to create dependencies
> > between patch sets that are under review for exactly these reasons.
> this was also not by intention. Vladimir found some files I missed in the
> first series. As the whole first series had already been reviewed at that time,
> I wasn't sure whether I am allowed to add further patches to it. Additionally
> I didn't concern that although my local build is successful, I should wait
> until the first series is applied...

Yea, I saw that, no problem :)

> 
> > I don't know what are Jakub's preferences, but had this happened on our
> > internal patchwork instance, I would just ask the author to submit
> > another version with all the patches.
> Please let me know how I shall proceed...

Jakub has the final say, so I assume he will comment on that.

Regardless, thanks for the patches.

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Vladimir Oltean]

On Sun, Nov 22, 2020 at 08:29:22PM +0100, Christian Eggers wrote:
> this was also not by intention. Vladimir found some files I missed in the
> first series. As the whole first series had already been reviewed at that time,
> I wasn't sure whether I am allowed to add further patches to it. Additionally
> I didn't concern that although my local build is successful, I should wait
> until the first series is applied...

When I said that, what I was thinking of (although it might have not
been clear) was that if you send further patches, you send them _after_
the initial series is merged.

Alternatively, it would have been just as valid to resend the entire
series, as a 3+3 patchset with a new revision (v3 -> v4), with review
tags collected from the first three, and the last 3 being completely
new. Jakub could have superseded v3 and applied v4.

The idea behind splicing N patches into a series is that they are
logically connected to one another. For example, a patch doesn't build
without another. You _could_ split logically-connected patches into
separate series and post them independently for review, as long as they
are build-time independent. If they aren't, well, what happens is
exactly what happened: various test robots will report build failures,
which from a maintainer's point of view inspires less confidence to
apply a patch as-is. I would not be surprised if Jakub asked you to
resend with no change at all, just to ensure that the patch series
passes build tests before merging it.

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Ido Schimmel]

On Sun, Nov 22, 2020 at 04:35:58PM +0200, Ido Schimmel wrote:
> Anyway, I added all six patches to our regression as we have some PTP
> tests. Will let you know tomorrow.

Looks good

Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines

[Antoine Tenart]

Hello Christian,

Quoting Christian Eggers (2020-11-22 09:26:36)
> Use recently introduced PTP_MSGTYPE_SYNC and PTP_MSGTYPE_DELAY_REQ
> defines instead of a driver internal enumeration.
> 
> Signed-off-by: Christian Eggers <ceggers@gmx.de>

Reviewed-by: Antoine Tenart <atenart@kernel.org>

Thanks!
Antoine

> Cc: Quentin Schulz <quentin.schulz@bootlin.com>
> Cc: Antoine Tenart <atenart@kernel.org>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
>  drivers/net/phy/mscc/mscc_ptp.c | 14 +++++++-------
>  drivers/net/phy/mscc/mscc_ptp.h |  5 -----
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
> index d8a61456d1ce..924ed5b034a4 100644
> --- a/drivers/net/phy/mscc/mscc_ptp.c
> +++ b/drivers/net/phy/mscc/mscc_ptp.c
> @@ -506,9 +506,9 @@ static int vsc85xx_ptp_cmp_init(struct phy_device *phydev, enum ts_blk blk)
>  {
>         struct vsc8531_private *vsc8531 = phydev->priv;
>         bool base = phydev->mdio.addr == vsc8531->ts_base_addr;
> -       enum vsc85xx_ptp_msg_type msgs[] = {
> -               PTP_MSG_TYPE_SYNC,
> -               PTP_MSG_TYPE_DELAY_REQ
> +       u8 msgs[] = {
> +               PTP_MSGTYPE_SYNC,
> +               PTP_MSGTYPE_DELAY_REQ
>         };
>         u32 val;
>         u8 i;
> @@ -847,9 +847,9 @@ static int vsc85xx_ts_ptp_action_flow(struct phy_device *phydev, enum ts_blk blk
>  static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
>                             bool one_step, bool enable)
>  {
> -       enum vsc85xx_ptp_msg_type msgs[] = {
> -               PTP_MSG_TYPE_SYNC,
> -               PTP_MSG_TYPE_DELAY_REQ
> +       u8 msgs[] = {
> +               PTP_MSGTYPE_SYNC,
> +               PTP_MSGTYPE_DELAY_REQ
>         };
>         u32 val;
>         u8 i;
> @@ -858,7 +858,7 @@ static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
>                 if (blk == INGRESS)
>                         vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
>                                                    PTP_WRITE_NS);
> -               else if (msgs[i] == PTP_MSG_TYPE_SYNC && one_step)
> +               else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
>                         /* no need to know Sync t when sending in one_step */
>                         vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
>                                                    PTP_WRITE_1588);
> diff --git a/drivers/net/phy/mscc/mscc_ptp.h b/drivers/net/phy/mscc/mscc_ptp.h
> index 3ea163af0f4f..da3465360e90 100644
> --- a/drivers/net/phy/mscc/mscc_ptp.h
> +++ b/drivers/net/phy/mscc/mscc_ptp.h
> @@ -436,11 +436,6 @@ enum ptp_cmd {
>         PTP_SAVE_IN_TS_FIFO = 11, /* invalid when writing in reg */
>  };
>  
> -enum vsc85xx_ptp_msg_type {
> -       PTP_MSG_TYPE_SYNC,
> -       PTP_MSG_TYPE_DELAY_REQ,
> -};
> -
>  struct vsc85xx_ptphdr {
>         u8 tsmt; /* transportSpecific | messageType */
>         u8 ver;  /* reserved0 | versionPTP */
> -- 
> Christian Eggers
> Embedded software developer
> 

Re: [PATCH net-next 1/3] net: phy: dp83640: use new PTP_MSGTYPE_SYNC define

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2258 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-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ca8d2b8ed7f512e08334c0da147ff3cb53b89d51
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
        git checkout ca8d2b8ed7f512e08334c0da147ff3cb53b89d51
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/phy/dp83640.c: In function 'is_sync':
>> drivers/net/phy/dp83640.c:972:39: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     972 |  return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
         |                                       ^~~~~~~~~~~~~~~~
         |                                       CSD_TYPE_SYNC
   drivers/net/phy/dp83640.c:972:39: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/phy/dp83640.c:973:1: error: control reaches end of non-void function [-Werror=return-type]
     973 | }
         | ^
   cc1: some warnings being treated as errors

vim +972 drivers/net/phy/dp83640.c

   963	
   964	static int is_sync(struct sk_buff *skb, int type)
   965	{
   966		struct ptp_header *hdr;
   967	
   968		hdr = ptp_parse_header(skb, type);
   969		if (!hdr)
   970			return 0;
   971	
 > 972		return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
   973	}
   974	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45410 bytes --]

Re: [PATCH net-next 1/3] net: phy: dp83640: use new PTP_MSGTYPE_SYNC define

[Christian Eggers]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

Sorry for this, I sent my 2nd series too early. These errors will disappear as
soon as the first series has been merged.

regards
Christian

On Monday, 23 November 2020, 18:01:51 CET, kernel test robot wrote:
> 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-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
> config: x86_64-rhel (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/0day-ci/linux/commit/ca8d2b8ed7f512e08334c0da147ff3cb53b89d51
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
>         git checkout ca8d2b8ed7f512e08334c0da147ff3cb53b89d51
>         # save the attached .config to linux build tree
>         make W=1 ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    drivers/net/phy/dp83640.c: In function 'is_sync':
> >> drivers/net/phy/dp83640.c:972:39: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
>      972 |  return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
>          |                                       ^~~~~~~~~~~~~~~~
>          |                                       CSD_TYPE_SYNC
>    drivers/net/phy/dp83640.c:972:39: note: each undeclared identifier is reported only once for each function it appears in
>    drivers/net/phy/dp83640.c:973:1: error: control reaches end of non-void function [-Werror=return-type]
>      973 | }
>          | ^
>    cc1: some warnings being treated as errors
>
> vim +972 drivers/net/phy/dp83640.c
>
>    963
>    964        static int is_sync(struct sk_buff *skb, int type)
>    965        {
>    966                struct ptp_header *hdr;
>    967
>    968                hdr = ptp_parse_header(skb, type);
>    969                if (!hdr)
>    970                        return 0;
>    971
>  > 972                return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
>    973        }
>    974
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>




________________________________
 [http://assets.arri.com/media/sign/2020-04-03-E-mail-signature-Stellar2_V1.jpg] <https://microsites.arri.com/stellar/>

Get all the latest information from www.arri.com<https://www.arri.com/>, Facebook<https://www.facebook.com/TeamARRI>, Twitter<https://twitter.com/ARRIChannel>, Instagram<https://instagram.com/arri> and YouTube<https://www.youtube.com/user/ARRIChannel>.

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: München - Registergericht: Amtsgericht München - Handelsregisternummer: HRA 57918
Persönlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: München - Registergericht: Amtsgericht München - Handelsregisternummer: HRB 54477
Geschäftsführer: Dr. Michael Neuhäuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

Re: [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions

[Jakub Kicinski]

On Sun, 22 Nov 2020 22:01:54 +0200 Ido Schimmel wrote:
> > > I don't know what are Jakub's preferences, but had this happened on our
> > > internal patchwork instance, I would just ask the author to submit
> > > another version with all the patches.  
> > Please let me know how I shall proceed...  
> 
> Jakub has the final say, so I assume he will comment on that.

The pre-requisite was just merged, so please collect all the review tags
you got so far and repost.