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.
Use recently introduced PTP_MSGTYPE_SYNC and PTP_MSGTYPE_DELAY_REQ
defines instead of a driver internal enumeration.
Signed-off-by: Christian Eggers <[email protected]>
Cc: Quentin Schulz <[email protected]>
Cc: Antoine Tenart <[email protected]>
Cc: Antoine Tenart <[email protected]>
---
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
Replace use of magic number with recently introduced define.
Signed-off-by: Christian Eggers <[email protected]>
Cc: Kurt Kanzenbach <[email protected]>
---
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
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 <[email protected]>
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/[email protected]
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 <[email protected]>
Reviewed-by: Antoine Tenart <[email protected]>
Thanks!
Antoine
> Cc: Quentin Schulz <[email protected]>
> Cc: Antoine Tenart <[email protected]>
> Cc: Antoine Tenart <[email protected]>
> ---
> 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
>