Thread (4 messages) 4 messages, 3 authors, 2013-03-29

Fw: Bug in ks8851.c

From: Max.Nekludov at us.elster.com <hidden>
Date: 2013-03-29 15:29:17
Also in: lkml, netdev

Denis,
here is patch with commit message:

Subject: [PATCH] Fix bug in ks8851 driver

According to the Datasheet (page 52):
15-12 Reserved
11-0 RXBC Receive Byte Count
This field indicates the present received frame byte size.

The code has a bug:
                 rxh = ks8851_rdreg32(ks, KS_RXFHSR);
                 rxstat = rxh & 0xffff;
                 rxlen = rxh >> 16; // BUG!!! 0xFFF mask should be applied

Signed-off-by: Max Nekludov <redacted>
---
drivers/net/ethernet/micrel/ks8851.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/micrel/ks8851.c
b/drivers/net/ethernet/micrel/ks8851.c
index 33bcb63d56a2..8fb481252e2c 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -528,7 +528,7 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 	for (; rxfc != 0; rxfc--) {
 		rxh = ks8851_rdreg32(ks, KS_RXFHSR);
 		rxstat = rxh & 0xffff;
-		rxlen = rxh >> 16;
+		rxlen = (rxh >> 16) & 0xfff;

 		netif_dbg(ks, rx_status, ks->netdev,
 			  "rx: stat 0x%04x, len 0x%04x\n", rxstat, rxlen);

|------------>
| From:      |
|------------>
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
  |Denis Kirjanov [off-list ref]                                                                                                            |
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
|------------>
| To:        |
|------------>
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
  |Max.Nekludov at us.elster.com                                                                                                                        |
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
|------------>
| Cc:        |
|------------>
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
  |Linus Torvalds [off-list ref], "David S. Miller" [off-list ref], Jiri Pirko [off-list ref], linus971 at gmail.com, Linux |
  |Kernel Mailing List [off-list ref], Matt Renzelmann [off-list ref], Network Development [off-list ref], Stephen Boyd |
  |[off-list ref], Greg Ungerer [off-list ref], linux-arm-kernel at lists.infradead.org                                                     |
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
|------------>
| Date:      |
|------------>
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
  |29.03.2013 18:07                                                                                                                                  |
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
|------------>
| Subject:   |
|------------>
  >--------------------------------------------------------------------------------------------------------------------------------------------------|
  |Re: Fw: Bug in ks8851.c                                                                                                                           |
  >--------------------------------------------------------------------------------------------------------------------------------------------------|





please respin this patch with a commit description.

On 3/29/13, Max.Nekludov at us.elster.com [off-list ref] wrote:
Linus,

I tried to send the mail to 'Ben Dooks [off-list ref]' but the
address
quoted hunk ↗ jump to hunk
is dead now.
quoted
I assume you've tested it in practice?
Yes, I'm running the modified code both in bootloader and Linux kernel on
my board.

Thanks,
Max

 Signed-off-by: Max Nekludov [off-list ref]
 ---
 drivers/net/ethernet/micrel/ks8851.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/micrel/ks8851.c
b/drivers/net/ethernet/micrel/ks8851.c
index 33bcb63d56a2..8fb481252e2c 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -528,7 +528,7 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 		 for (; rxfc != 0; rxfc--) {
 		 		 rxh = ks8851_rdreg32(ks, KS_RXFHSR);
 		 		 rxstat = rxh & 0xffff;
-		 		 rxlen = rxh >> 16;
+		 		 rxlen = (rxh >> 16) & 0xfff;

 		 		 netif_dbg(ks, rx_status, ks->netdev,
 		 		 		   "rx: stat 0x%04x, len 0x%04x\n",
rxstat, rxlen);



Max,
 please cc the actual maintainers of the driver. The patch looks sane,
though. I assume you've tested it in practice?

You also seem to have based this on an ancient version, the code has
long since moved from drivers/net/ks8851.c to
drivers/net/ethernet/micrel/ks8851.c (back in June of 2011), and it's
missing a sign-off from you.

I'm attaching an updated patch for the rename/capitalization issue.

     Linus

On Thu, Mar 28, 2013 at 11:25 AM,  [off-list ref] wrote:
quoted
According to the Datasheet (page 52):
15-12 Reserved
11-0 RXBC Receive Byte Count
This field indicates the present received frame byte size.

I suppose the code has a bug:
                rxh = ks8851_rdreg32(ks, KS_RXFHSR);
                rxstat = rxh & 0xffff;
                rxlen = rxh >> 16; // BUG!!! 0xFFF mask should be
applied
quoted
P.S.
without bit mask applied I saw rxlen equal to 15360 which is bigger then
entire RX queue size (12KB).

Thanks,
Max Nekludov

From cb3199cee4490f98d6062e32a75ca377a32b55bc Mon Sep 17 00:00:00 2001
From: Max Neklyudov <redacted>
Date: Tue, 26 Mar 2013 11:46:57 +0400
Subject: [PATCH] Fix bug in ks8851 driver

---
 drivers/net/ks8851.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 91a93cb..0dc03da 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -553,7 +553,7 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
        for (; rxfc != 0; rxfc--) {
                rxh = ks8851_rdreg32(ks, KS_RXFHSR);
                rxstat = rxh & 0xffff;
-               rxlen = rxh >> 16;
+               rxlen = (rxh >> 16) & 0xFFF;

                netif_dbg(ks, rx_status, ks->netdev,
                          "rx: stat 0x%04x, len 0x%04x\n", rxstat,
rxlen);
quoted
--
1.7.10.4

______________________________________________________________________
This email has been spam and virus checked by Elster IT Services.(See
attached file: patch.diff)
______________________________________________________________________
This email has been spam and virus checked by Elster IT Services.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help