* Small glitch in neptune ethernet driver
@ 2011-10-26 17:39 Thomas Jarosch
2011-12-02 3:00 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Jarosch @ 2011-10-26 17:39 UTC (permalink / raw)
To: davem; +Cc: netdev
Hi Dave,
consider this piece of code in net/ethernet/sun/niu.c:
static int __devinit phy_record(struct niu_parent *parent,
struct phy_probe_info *p,
int dev_id_1, int dev_id_2, u8 phy_port,
int type)
{
...
if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
if (((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8704) &&
((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_MRVL88X2011) &&
((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8706))
return 0;
}
...
}
Here are the defines from sun/niu.h:
#define NIU_PHY_ID_MASK 0xfffff0f0
#define NIU_PHY_ID_BCM8704 0x00206030
#define NIU_PHY_ID_BCM8706 0x00206035
There's a zero at the end of the ID_MASK,
so the NIU_PHY_ID_BCM8706 will never match (ends on 5).
Report from cppcheck:
[sun/niu.c:8594]: (style) Mismatching comparison, the result is always true
The code will probably still work as the id for BCM8706
should match on NIU_PHY_ID_BCM8704, too.
Keep it or leave it?
Cheers,
Thomas
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Small glitch in neptune ethernet driver
2011-10-26 17:39 Small glitch in neptune ethernet driver Thomas Jarosch
@ 2011-12-02 3:00 ` David Miller
2011-12-02 8:49 ` Thomas Jarosch
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2011-12-02 3:00 UTC (permalink / raw)
To: thomas.jarosch; +Cc: netdev
From: Thomas Jarosch <thomas.jarosch@intra2net.com>
Date: Wed, 26 Oct 2011 19:39:27 +0200
> if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
> if (((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8704) &&
> ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_MRVL88X2011) &&
> ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8706))
> return 0;
> }
> ...
> }
>
> Here are the defines from sun/niu.h:
>
> #define NIU_PHY_ID_MASK 0xfffff0f0
> #define NIU_PHY_ID_BCM8704 0x00206030
> #define NIU_PHY_ID_BCM8706 0x00206035
>
> There's a zero at the end of the ID_MASK,
> so the NIU_PHY_ID_BCM8706 will never match (ends on 5).
>
> Report from cppcheck:
> [sun/niu.c:8594]: (style) Mismatching comparison, the result is always true
>
> The code will probably still work as the id for BCM8706
> should match on NIU_PHY_ID_BCM8704, too.
Thanks for pointing this out, I'll remove the extra test but add a
comment explaining the situation nearby, as follows:
--------------------
niu: Remove redundant PHY ID test.
Reported-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/sun/niu.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 680b107..56d106e 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -8579,9 +8579,11 @@ static int __devinit phy_record(struct niu_parent *parent,
if (dev_id_1 < 0 || dev_id_2 < 0)
return 0;
if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
+ /* Becuase of the NIU_PHY_ID_MASK being applied, the 8704
+ * test covers the 8706 as well.
+ */
if (((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8704) &&
- ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_MRVL88X2011) &&
- ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM8706))
+ ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_MRVL88X2011))
return 0;
} else {
if ((id & NIU_PHY_ID_MASK) != NIU_PHY_ID_BCM5464R)
--
1.7.7.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: Small glitch in neptune ethernet driver
2011-12-02 3:00 ` David Miller
@ 2011-12-02 8:49 ` Thomas Jarosch
2011-12-02 17:37 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Jarosch @ 2011-12-02 8:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Friday, 2. December 2011 04:00:03 David Miller wrote:
> diff --git a/drivers/net/ethernet/sun/niu.c
> b/drivers/net/ethernet/sun/niu.c index 680b107..56d106e 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -8579,9 +8579,11 @@ static int __devinit phy_record(struct niu_parent
> *parent, if (dev_id_1 < 0 || dev_id_2 < 0)
> return 0;
> if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
> + /* Becuase of the NIU_PHY_ID_MASK being applied, the 8704
The change looks good to me, just fix the typo in "Becuase".
Thanks,
Thomas
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Small glitch in neptune ethernet driver
2011-12-02 8:49 ` Thomas Jarosch
@ 2011-12-02 17:37 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2011-12-02 17:37 UTC (permalink / raw)
To: thomas.jarosch; +Cc: netdev
From: Thomas Jarosch <thomas.jarosch@intra2net.com>
Date: Fri, 2 Dec 2011 09:49:32 +0100
> On Friday, 2. December 2011 04:00:03 David Miller wrote:
>> diff --git a/drivers/net/ethernet/sun/niu.c
>> b/drivers/net/ethernet/sun/niu.c index 680b107..56d106e 100644
>> --- a/drivers/net/ethernet/sun/niu.c
>> +++ b/drivers/net/ethernet/sun/niu.c
>> @@ -8579,9 +8579,11 @@ static int __devinit phy_record(struct niu_parent
>> *parent, if (dev_id_1 < 0 || dev_id_2 < 0)
>> return 0;
>> if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
>> + /* Becuase of the NIU_PHY_ID_MASK being applied, the 8704
>
> The change looks good to me, just fix the typo in "Becuase".
Fixed, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-02 17:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-26 17:39 Small glitch in neptune ethernet driver Thomas Jarosch
2011-12-02 3:00 ` David Miller
2011-12-02 8:49 ` Thomas Jarosch
2011-12-02 17:37 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).