* Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation"
@ 2016-11-12 14:34 ` Marion & Christophe JAILLET
0 siblings, 0 replies; 4+ messages in thread
From: Marion & Christophe JAILLET @ 2016-11-12 14:34 UTC (permalink / raw)
To: thomas.lendacky, davem; +Cc: netdev, linux-kernel, Kernel Janitors
Hi,
in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37
auto-negotiation"), we can find:
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
index 695e982..8bcf4ef 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
[...]
#define XGBE_AN_CL37_PCS_MODE_MASK 0x06
#define XGBE_AN_CL37_PCS_MODE_BASEX 0x00
#define XGBE_AN_CL37_PCS_MODE_SGMII 0x04
#define XGBE_AN_CL37_TX_CONFIG_MASK 0x08
[...]
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
index d5bfbe4..723eb90 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
[...]
/* Set up the Control register */
reg = XMDIO_READ(pdata, MDIO_MMD_VEND2, MDIO_VEND2_AN_CTRL);
reg &= XGBE_AN_CL37_TX_CONFIG_MASK;
reg &= XGBE_AN_CL37_PCS_MODE_MASK;
[...]
the "reg &=" statements look spurious. The 2 constants being 0x06 and
0x08, the current code is equivalent to "reg = 0"
It is likely that "reg |=" (or "reg &= ~") was expected here.
Best regards,
CJ
^ permalink raw reply [flat|nested] 4+ messages in thread
* Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation"
@ 2016-11-12 14:34 ` Marion & Christophe JAILLET
0 siblings, 0 replies; 4+ messages in thread
From: Marion & Christophe JAILLET @ 2016-11-12 14:34 UTC (permalink / raw)
To: thomas.lendacky, davem; +Cc: netdev, linux-kernel, Kernel Janitors
Hi,
in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37
auto-negotiation"), we can find:
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
index 695e982..8bcf4ef 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
[...]
#define XGBE_AN_CL37_PCS_MODE_MASK 0x06
#define XGBE_AN_CL37_PCS_MODE_BASEX 0x00
#define XGBE_AN_CL37_PCS_MODE_SGMII 0x04
#define XGBE_AN_CL37_TX_CONFIG_MASK 0x08
[...]
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
index d5bfbe4..723eb90 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
[...]
/* Set up the Control register */
reg = XMDIO_READ(pdata, MDIO_MMD_VEND2, MDIO_VEND2_AN_CTRL);
reg &= XGBE_AN_CL37_TX_CONFIG_MASK;
reg &= XGBE_AN_CL37_PCS_MODE_MASK;
[...]
the "reg &=" statements look spurious. The 2 constants being 0x06 and
0x08, the current code is equivalent to "reg = 0"
It is likely that "reg |=" (or "reg &= ~") was expected here.
Best regards,
CJ
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation"
2016-11-12 14:34 ` Marion & Christophe JAILLET
@ 2016-11-12 15:47 ` Tom Lendacky
-1 siblings, 0 replies; 4+ messages in thread
From: Tom Lendacky @ 2016-11-12 15:47 UTC (permalink / raw)
To: Marion & Christophe JAILLET, davem
Cc: netdev, linux-kernel, Kernel Janitors
On 11/12/2016 8:34 AM, Marion & Christophe JAILLET wrote:
> Hi,
>
> in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37
> auto-negotiation"), we can find:
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> index 695e982..8bcf4ef 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> [...]
> #define XGBE_AN_CL37_PCS_MODE_MASK 0x06
> #define XGBE_AN_CL37_PCS_MODE_BASEX 0x00
> #define XGBE_AN_CL37_PCS_MODE_SGMII 0x04
> #define XGBE_AN_CL37_TX_CONFIG_MASK 0x08
> [...]
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> index d5bfbe4..723eb90 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> [...]
> /* Set up the Control register */
> reg = XMDIO_READ(pdata, MDIO_MMD_VEND2, MDIO_VEND2_AN_CTRL);
> reg &= XGBE_AN_CL37_TX_CONFIG_MASK;
> reg &= XGBE_AN_CL37_PCS_MODE_MASK;
> [...]
>
> the "reg &=" statements look spurious. The 2 constants being 0x06 and
> 0x08, the current code is equivalent to "reg = 0"
>
> It is likely that "reg |=" (or "reg &= ~") was expected here.
Yes, those should have been "reg &= ~". I didn't find this in my testing
because the register is all zeroes after reset. I'll submit a patch to
fix that.
Thanks,
Tom
>
> Best regards,
> CJ
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation"
@ 2016-11-12 15:47 ` Tom Lendacky
0 siblings, 0 replies; 4+ messages in thread
From: Tom Lendacky @ 2016-11-12 15:47 UTC (permalink / raw)
To: Marion & Christophe JAILLET, davem
Cc: netdev, linux-kernel, Kernel Janitors
On 11/12/2016 8:34 AM, Marion & Christophe JAILLET wrote:
> Hi,
>
> in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37
> auto-negotiation"), we can find:
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> index 695e982..8bcf4ef 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
> [...]
> #define XGBE_AN_CL37_PCS_MODE_MASK 0x06
> #define XGBE_AN_CL37_PCS_MODE_BASEX 0x00
> #define XGBE_AN_CL37_PCS_MODE_SGMII 0x04
> #define XGBE_AN_CL37_TX_CONFIG_MASK 0x08
> [...]
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> index d5bfbe4..723eb90 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> [...]
> /* Set up the Control register */
> reg = XMDIO_READ(pdata, MDIO_MMD_VEND2, MDIO_VEND2_AN_CTRL);
> reg &= XGBE_AN_CL37_TX_CONFIG_MASK;
> reg &= XGBE_AN_CL37_PCS_MODE_MASK;
> [...]
>
> the "reg &=" statements look spurious. The 2 constants being 0x06 and
> 0x08, the current code is equivalent to "reg = 0"
>
> It is likely that "reg |=" (or "reg &= ~") was expected here.
Yes, those should have been "reg &= ~". I didn't find this in my testing
because the register is all zeroes after reset. I'll submit a patch to
fix that.
Thanks,
Tom
>
> Best regards,
> CJ
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-12 15:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-12 14:34 Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation" Marion & Christophe JAILLET
2016-11-12 14:34 ` Marion & Christophe JAILLET
2016-11-12 15:47 ` Tom Lendacky
2016-11-12 15:47 ` Tom Lendacky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.