* [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
@ 2008-01-08 19:05 Anton Vorontsov
2008-01-09 10:46 ` Sergej Stepanov
2008-01-12 22:45 ` Jeff Garzik
0 siblings, 2 replies; 13+ messages in thread
From: Anton Vorontsov @ 2008-01-08 19:05 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linuxppc-dev
Otherwise oops will happen if ethernet device has not been opened:
Unable to handle kernel paging request for data at address 0x0000014c
Faulting instruction address: 0xc016f7f0
Oops: Kernel access of bad area, sig: 11 [#1]
MPC85xx
NIP: c016f7f0 LR: c01722a0 CTR: 00000000
REGS: c79ddc70 TRAP: 0300 Not tainted (2.6.24-rc3-g820a386b)
MSR: 00029000 <EE,ME> CR: 20004428 XER: 20000000
DEAR: 0000014c, ESR: 00000000
TASK = c789f5e0[999] 'snmpd' THREAD: c79dc000
GPR00: c01aceb8 c79ddd20 c789f5e0 00000000 c79ddd3c 00000000 c79ddd64 00000000
GPR08: 00000000 c7845b60 c79dde3c c01ace80 20004422 200249fc 000002a0 100da728
GPR16: 100c0000 00000000 00000000 00000000 20022078 00000009 200220e0 bfc85558
GPR24: c79ddd3c 00000000 00000000 c02e0e70 c022fc64 ffffffff c7845800 bfc85498
NIP [c016f7f0] phy_ethtool_gset+0x0/0x4c
LR [c01722a0] fs_get_settings+0x18/0x28
Call Trace:
[c79ddd20] [c79dde38] 0xc79dde38 (unreliable)
[c79ddd30] [c01aceb8] dev_ethtool+0x294/0x11ec
[c79dde30] [c01aaa44] dev_ioctl+0x454/0x6a8
[c79ddeb0] [c019b9d4] sock_ioctl+0x84/0x230
[c79dded0] [c007ded8] do_ioctl+0x34/0x8c
[c79ddee0] [c007dfbc] vfs_ioctl+0x8c/0x41c
[c79ddf10] [c007e38c] sys_ioctl+0x40/0x74
[c79ddf40] [c000d4c0] ret_from_syscall+0x0/0x3c
Instruction dump:
81630000 800b0030 2f800000 419e0010 7c0803a6 4e800021 7c691b78 80010014
7d234b78 38210010 7c0803a6 4e800020 <8003014c> 7c6b1b78 38600000 90040004
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
---
Just resending it, it feels like it got lost during holidays.
drivers/net/fs_enet/fs_enet-main.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index f2a4d39..23fddc3 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -897,14 +897,21 @@ static void fs_get_regs(struct net_device *dev, struct ethtool_regs *regs,
static int fs_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
struct fs_enet_private *fep = netdev_priv(dev);
+
+ if (!fep->phydev)
+ return -ENODEV;
+
return phy_ethtool_gset(fep->phydev, cmd);
}
static int fs_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
struct fs_enet_private *fep = netdev_priv(dev);
- phy_ethtool_sset(fep->phydev, cmd);
- return 0;
+
+ if (!fep->phydev)
+ return -ENODEV;
+
+ return phy_ethtool_sset(fep->phydev, cmd);
}
static int fs_nway_reset(struct net_device *dev)
--
1.5.2.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-08 19:05 Anton Vorontsov
@ 2008-01-09 10:46 ` Sergej Stepanov
2008-01-09 12:58 ` Heiko Schocher
2008-01-12 22:45 ` Jeff Garzik
1 sibling, 1 reply; 13+ messages in thread
From: Sergej Stepanov @ 2008-01-09 10:46 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, Jeff Garzik, linuxppc-dev
Am Dienstag, den 08.01.2008, 22:05 +0300 schrieb Anton Vorontsov:
> Otherwise oops will happen if ethernet device has not been opened:
>
> Unable to handle kernel paging request for data at address 0x0000014c
> Faulting instruction address: 0xc016f7f0
> Oops: Kernel access of bad area, sig: 11 [#1]
> MPC85xx
> NIP: c016f7f0 LR: c01722a0 CTR: 00000000
> REGS: c79ddc70 TRAP: 0300 Not tainted (2.6.24-rc3-g820a386b)
> MSR: 00029000 <EE,ME> CR: 20004428 XER: 20000000
> DEAR: 0000014c, ESR: 00000000
> TASK = c789f5e0[999] 'snmpd' THREAD: c79dc000
> GPR00: c01aceb8 c79ddd20 c789f5e0 00000000 c79ddd3c 00000000 c79ddd64 00000000
> GPR08: 00000000 c7845b60 c79dde3c c01ace80 20004422 200249fc 000002a0 100da728
> GPR16: 100c0000 00000000 00000000 00000000 20022078 00000009 200220e0 bfc85558
> GPR24: c79ddd3c 00000000 00000000 c02e0e70 c022fc64 ffffffff c7845800 bfc85498
> NIP [c016f7f0] phy_ethtool_gset+0x0/0x4c
> LR [c01722a0] fs_get_settings+0x18/0x28
> Call Trace:
> [c79ddd20] [c79dde38] 0xc79dde38 (unreliable)
> [c79ddd30] [c01aceb8] dev_ethtool+0x294/0x11ec
> [c79dde30] [c01aaa44] dev_ioctl+0x454/0x6a8
> [c79ddeb0] [c019b9d4] sock_ioctl+0x84/0x230
> [c79dded0] [c007ded8] do_ioctl+0x34/0x8c
> [c79ddee0] [c007dfbc] vfs_ioctl+0x8c/0x41c
> [c79ddf10] [c007e38c] sys_ioctl+0x40/0x74
> [c79ddf40] [c000d4c0] ret_from_syscall+0x0/0x3c
> Instruction dump:
> 81630000 800b0030 2f800000 419e0010 7c0803a6 4e800021 7c691b78 80010014
> 7d234b78 38210010 7c0803a6 4e800020 <8003014c> 7c6b1b78 38600000 90040004
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
> ---
>
> Just resending it, it feels like it got lost during holidays.
>
> drivers/net/fs_enet/fs_enet-main.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> index f2a4d39..23fddc3 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -897,14 +897,21 @@ static void fs_get_regs(struct net_device *dev, struct ethtool_regs *regs,
> static int fs_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> {
> struct fs_enet_private *fep = netdev_priv(dev);
> +
> + if (!fep->phydev)
> + return -ENODEV;
> +
> return phy_ethtool_gset(fep->phydev, cmd);
> }
>
> static int fs_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> {
> struct fs_enet_private *fep = netdev_priv(dev);
> - phy_ethtool_sset(fep->phydev, cmd);
> - return 0;
> +
> + if (!fep->phydev)
> + return -ENODEV;
> +
> + return phy_ethtool_sset(fep->phydev, cmd);
> }
>
> static int fs_nway_reset(struct net_device *dev)
I got also oops problem with the driver.
What could be false?
After the following patch it functions.
Thanks for any advance.
diff --git a/drivers/net/fs_enet/fs_enet-main.c
b/drivers/net/fs_enet/fs_enet-main.c
index f2a4d39..d5081b1 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -99,6 +99,8 @@ static int fs_enet_rx_napi(struct napi_struct *napi,
int budget)
if (!netif_running(dev))
return 0;
+ if (fep->cur_rx == NULL)
+ return 0;
/*
* First, grab all of the stats for the incoming packet.
* These get messed up if we get called due to a busy condition.
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-09 10:46 ` Sergej Stepanov
@ 2008-01-09 12:58 ` Heiko Schocher
2008-01-09 18:20 ` Scott Wood
0 siblings, 1 reply; 13+ messages in thread
From: Heiko Schocher @ 2008-01-09 12:58 UTC (permalink / raw)
To: Sergej Stepanov; +Cc: linuxppc-dev, Jeff Garzik
Hello Sergej,
Wed Jan 9 21:46:06 EST 2008 Sergej Stepanov Sergej.Stepanov@ids.de wrote:
> I got also oops problem with the driver.
> What could be false?
> After the following patch it functions.
> Thanks for any advance.
>
> diff --git a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c
> index f2a4d39..d5081b1 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -99,6 +99,8 @@ static int fs_enet_rx_napi(struct napi_struct *napi,
> int budget)
seems your mailer wraps long lines
> if (!netif_running(dev))
> return 0;
>
> + if (fep->cur_rx == NULL)
> + return 0;
> /*
> * First, grab all of the stats for the incoming packet.
> * These get messed up if we get called due to a busy condition.
Hmm... I had also a oops in fs_enet_rx_napi () because of an
uninitialized fep->cur_rx. The following Patch solves this, also
adds support for using Ethernet over SCC on a CPM2.
>From 62cd02d481eb772f4417e9ba17fb010d1954c330 Mon Sep 17 00:00:00 2001
From: Heiko Schocher <hs@denx.de>
Date: Mon, 7 Jan 2008 09:42:09 +0100
Subject: [PATCH] [POWERPC] Fix Ethernet over SCC on a CPM2
Signed-off-by: Heiko Schocher <hs@denx.de>
---
drivers/net/fs_enet/fs_enet-main.c | 11 +++++++++--
drivers/net/fs_enet/mac-scc.c | 18 ++++++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index f2a4d39..b4a1480 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -982,6 +982,7 @@ static struct net_device *fs_init_instance(struct device *dev,
fep = netdev_priv(ndev);
fep->dev = dev;
+ fep->ndev = ndev;
dev_set_drvdata(dev, ndev);
fep->fpi = fpi;
if (fpi->init_ioports)
@@ -1085,7 +1086,6 @@ static struct net_device *fs_init_instance(struct device *dev,
}
registered = 1;
-
return ndev;
err:
@@ -1312,6 +1312,9 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
ndev->dev_addr[0], ndev->dev_addr[1], ndev->dev_addr[2],
ndev->dev_addr[3], ndev->dev_addr[4], ndev->dev_addr[5]);
+ /* to initialize the fep->cur_rx,... */
+ /* not doing this, will cause a crash in fs_enet_rx_napi */
+ fs_init_bds(ndev);
return 0;
out_free_bd:
@@ -1342,9 +1345,13 @@ static int fs_enet_remove(struct of_device *ofdev)
}
static struct of_device_id fs_enet_match[] = {
-#ifdef CONFIG_FS_ENET_HAS_SCC
+#if defined(CONFIG_FS_ENET_HAS_SCC)
{
+#if defined(CONFIG_CPM1)
.compatible = "fsl,cpm1-scc-enet",
+#else
+ .compatible = "fsl,cpm2-scc-enet",
+#endif
.data = (void *)&fs_scc_ops,
},
#endif
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index 48f2f30..3b5ca76 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -50,6 +50,7 @@
#include "fs_enet.h"
/*************************************************/
+#define SCC_EB ((u_char)0x10) /* Set big endian byte order */
#if defined(CONFIG_CPM1)
/* for a 8xx __raw_xxx's are sufficient */
@@ -65,6 +66,8 @@
#define __fs_out16(addr, x) out_be16(addr, x)
#define __fs_in32(addr) in_be32(addr)
#define __fs_in16(addr) in_be16(addr)
+#define __fs_out8(addr, x) out_8(addr, x)
+#define __fs_in8(addr) in_8(addr)
#endif
/* write, read, set bits, clear bits */
@@ -96,10 +99,18 @@ static inline int scc_cr_cmd(struct fs_enet_private *fep, u32 op)
const struct fs_platform_info *fpi = fep->fpi;
int i;
+#if defined(CONFIG_CPM1)
W16(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | (op << 8));
for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
if ((R16(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
return 0;
+#else
+ W32(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | op);
+ for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
+ if ((R32(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
+ return 0;
+
+#endif
printk(KERN_ERR "%s(): Not able to issue CPM command\n",
__FUNCTION__);
@@ -306,8 +317,15 @@ static void restart(struct net_device *dev)
/* Initialize function code registers for big-endian.
*/
+#ifdef CONFIG_CPM2
+ /* from oldstyle driver in arch/ppc */
+ /* seems necessary */
+ W8(ep, sen_genscc.scc_rfcr, SCC_EB | 0x20);
+ W8(ep, sen_genscc.scc_tfcr, SCC_EB | 0x20);
+#else
W8(ep, sen_genscc.scc_rfcr, SCC_EB);
W8(ep, sen_genscc.scc_tfcr, SCC_EB);
+#endif
/* Set maximum bytes per receive buffer.
* This appears to be an Ethernet frame size, not the buffer
--
1.5.2.2
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-09 12:58 ` Heiko Schocher
@ 2008-01-09 18:20 ` Scott Wood
2008-01-10 8:14 ` Heiko Schocher
0 siblings, 1 reply; 13+ messages in thread
From: Scott Wood @ 2008-01-09 18:20 UTC (permalink / raw)
To: Heiko Schocher; +Cc: linuxppc-dev, Jeff Garzik
On Wed, Jan 09, 2008 at 01:58:49PM +0100, Heiko Schocher wrote:
> @@ -1312,6 +1312,9 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
> ndev->dev_addr[0], ndev->dev_addr[1], ndev->dev_addr[2],
> ndev->dev_addr[3], ndev->dev_addr[4], ndev->dev_addr[5]);
>
> + /* to initialize the fep->cur_rx,... */
> + /* not doing this, will cause a crash in fs_enet_rx_napi */
> + fs_init_bds(ndev);
> return 0;
We don't want to allocate ring buffers for network interfaces that are never
opened, especially given the small amount of memory on some boards that use
this driver.
Instead, we should probably not be calling napi_enable() until the link is
up and init_bds() has been called.
> @@ -1342,9 +1345,13 @@ static int fs_enet_remove(struct of_device *ofdev)
> }
>
> static struct of_device_id fs_enet_match[] = {
> -#ifdef CONFIG_FS_ENET_HAS_SCC
> +#if defined(CONFIG_FS_ENET_HAS_SCC)
> {
> +#if defined(CONFIG_CPM1)
> .compatible = "fsl,cpm1-scc-enet",
> +#else
> + .compatible = "fsl,cpm2-scc-enet",
> +#endif
I know there are already ifdefs of this sort, and that multiplatform
cpm1/cpm2 is very unlikely to ever happen, but can we try to avoid
introducing more such ifdefs?
We can have both match entries present at the same time.
> .data = (void *)&fs_scc_ops,
> },
> #endif
> diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
> index 48f2f30..3b5ca76 100644
> --- a/drivers/net/fs_enet/mac-scc.c
> +++ b/drivers/net/fs_enet/mac-scc.c
> @@ -50,6 +50,7 @@
> #include "fs_enet.h"
>
> /*************************************************/
> +#define SCC_EB ((u_char)0x10) /* Set big endian byte order */
This is already defined in asm-powerpc/commproc.h, and thus will cause a
duplicate definition when building for 8xx. Please add this definition to
asm-powerpc/cpm2.h.
> +#if defined(CONFIG_CPM1)
> W16(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | (op << 8));
> for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
> if ((R16(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
> return 0;
> +#else
> + W32(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | op);
> + for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
> + if ((R32(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
> + return 0;
> +
> +#endif
Commit 362f9b6fa8c9670cc5496390845021c2865d049b in Paul's tree makes this
unnecessary.
> @@ -306,8 +317,15 @@ static void restart(struct net_device *dev)
>
> /* Initialize function code registers for big-endian.
> */
> +#ifdef CONFIG_CPM2
> + /* from oldstyle driver in arch/ppc */
> + /* seems necessary */
> + W8(ep, sen_genscc.scc_rfcr, SCC_EB | 0x20);
> + W8(ep, sen_genscc.scc_tfcr, SCC_EB | 0x20);
> +#else
> W8(ep, sen_genscc.scc_rfcr, SCC_EB);
> W8(ep, sen_genscc.scc_tfcr, SCC_EB);
> +#endif
Please define 0x20 as SCC_GBL (Snooping Enabled) in cpm2.h, and
conditionalize this on #ifndef CONFIG_NOT_COHERENT_CACHE.
You can remove the comment; it's really necessary, not just "seems" so. :-)
-Scott
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
@ 2008-01-09 20:10 Matvejchikov Ilya
0 siblings, 0 replies; 13+ messages in thread
From: Matvejchikov Ilya @ 2008-01-09 20:10 UTC (permalink / raw)
To: linuxppc-embedded, netdev
[-- Attachment #1: Type: text/plain, Size: 306 bytes --]
Hi folks!
I had the same problem too. The solution was the following:
http://www.mail-archive.com/netdev@vger.kernel.org/msg37951.html
Also have a look at the potential multicasting recovery problem in fs_enet
driver:
http://patchwork.ozlabs.org/linuxppc/patch?id=10700
Best regards,
Matvejchikov Ilya.
[-- Attachment #2: Type: text/html, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
@ 2008-01-09 20:14 Matvejchikov Ilya
0 siblings, 0 replies; 13+ messages in thread
From: Matvejchikov Ilya @ 2008-01-09 20:14 UTC (permalink / raw)
To: linuxppc-embedded, netdev
Hi folks!
I had the same problem too. The solution was the following:
http://www.mail-archive.com/netdev@vger.kernel.org/msg37951.html
Also have a look at the potential multicasting recovery problem in
fs_enet driver:
http://patchwork.ozlabs.org/linuxppc/patch?id=10700
Best regards,
Matvejchikov Ilya.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
@ 2008-01-09 20:20 Matvejchikov Ilya
0 siblings, 0 replies; 13+ messages in thread
From: Matvejchikov Ilya @ 2008-01-09 20:20 UTC (permalink / raw)
To: linuxppc-embedded, netdev
Hi folks!
I had the same problem too. The solution was the following:
http://www.mail-archive.com/netdev@vger.kernel.org/msg37951.html
Also have a look at the potential multicasting recovery problem in
fs_enet driver:
http://patchwork.ozlabs.org/linuxppc/patch?id=10700
Best regards,
Matvejchikov Ilya.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-09 18:20 ` Scott Wood
@ 2008-01-10 8:14 ` Heiko Schocher
2008-01-10 9:06 ` Heiko Schocher
0 siblings, 1 reply; 13+ messages in thread
From: Heiko Schocher @ 2008-01-10 8:14 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Jeff Garzik
Hello Scott,
Scott Wood wrote:
> On Wed, Jan 09, 2008 at 01:58:49PM +0100, Heiko Schocher wrote:
>> @@ -1312,6 +1312,9 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
>> ndev->dev_addr[0], ndev->dev_addr[1], ndev->dev_addr[2],
>> ndev->dev_addr[3], ndev->dev_addr[4], ndev->dev_addr[5]);
>>
>> + /* to initialize the fep->cur_rx,... */
>> + /* not doing this, will cause a crash in fs_enet_rx_napi */
>> + fs_init_bds(ndev);
>> return 0;
>
> We don't want to allocate ring buffers for network interfaces that are never
> opened, especially given the small amount of memory on some boards that use
> this driver.
>
> Instead, we should probably not be calling napi_enable() until the link is
> up and init_bds() has been called.
Ah, okay. I actually tried calling fs_init_bds(ndev); in fs_enet_open() after
napi_enable, and this also works fine. I think there is the better place for
it. Thanks.
>
>> @@ -1342,9 +1345,13 @@ static int fs_enet_remove(struct of_device *ofdev)
>> }
>>
>> static struct of_device_id fs_enet_match[] = {
>> -#ifdef CONFIG_FS_ENET_HAS_SCC
>> +#if defined(CONFIG_FS_ENET_HAS_SCC)
>> {
>> +#if defined(CONFIG_CPM1)
>> .compatible = "fsl,cpm1-scc-enet",
>> +#else
>> + .compatible = "fsl,cpm2-scc-enet",
>> +#endif
>
> I know there are already ifdefs of this sort, and that multiplatform
> cpm1/cpm2 is very unlikely to ever happen, but can we try to avoid
> introducing more such ifdefs?
>
> We can have both match entries present at the same time.
OK, fix this.
>
>> .data = (void *)&fs_scc_ops,
>> },
>> #endif
>> diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
>> index 48f2f30..3b5ca76 100644
>> --- a/drivers/net/fs_enet/mac-scc.c
>> +++ b/drivers/net/fs_enet/mac-scc.c
>> @@ -50,6 +50,7 @@
>> #include "fs_enet.h"
>>
>> /*************************************************/
>> +#define SCC_EB ((u_char)0x10) /* Set big endian byte order */
>
> This is already defined in asm-powerpc/commproc.h, and thus will cause a
> duplicate definition when building for 8xx. Please add this definition to
> asm-powerpc/cpm2.h.
OK, will fix it.
>
>> +#if defined(CONFIG_CPM1)
>> W16(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | (op << 8));
>> for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
>> if ((R16(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
>> return 0;
>> +#else
>> + W32(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | op);
>> + for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
>> + if ((R32(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
>> + return 0;
>> +
>> +#endif
>
> Commit 362f9b6fa8c9670cc5496390845021c2865d049b in Paul's tree makes this
> unnecessary.
Tried this patch, works fine for me :-)
>
>> @@ -306,8 +317,15 @@ static void restart(struct net_device *dev)
>>
>> /* Initialize function code registers for big-endian.
>> */
>> +#ifdef CONFIG_CPM2
>> + /* from oldstyle driver in arch/ppc */
>> + /* seems necessary */
>> + W8(ep, sen_genscc.scc_rfcr, SCC_EB | 0x20);
>> + W8(ep, sen_genscc.scc_tfcr, SCC_EB | 0x20);
>> +#else
>> W8(ep, sen_genscc.scc_rfcr, SCC_EB);
>> W8(ep, sen_genscc.scc_tfcr, SCC_EB);
>> +#endif
>
> Please define 0x20 as SCC_GBL (Snooping Enabled) in cpm2.h, and
> conditionalize this on #ifndef CONFIG_NOT_COHERENT_CACHE.
>
> You can remove the comment; it's really necessary, not just "seems" so. :-)
OK, fix it.
Will resend this fixed patch.
thanks
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-10 8:14 ` Heiko Schocher
@ 2008-01-10 9:06 ` Heiko Schocher
2008-01-10 17:27 ` Scott Wood
0 siblings, 1 reply; 13+ messages in thread
From: Heiko Schocher @ 2008-01-10 9:06 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Jeff Garzik
Hello Scott,
Heiko Schocher wrote:
> Hello Scott,
[...]
> Will resend this fixed patch.
Here it is:
[POWERPC] Fix Ethernet over SCC on a CPM2,
also Fix crash in fs_enet_rx_napi()
Signed-off-by: Heiko Schocher <hs@denx.de>
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index f2a4d39..f432a18 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -810,6 +810,10 @@ static int fs_enet_open(struct net_device *dev)
if (fep->fpi->use_napi)
napi_enable(&fep->napi);
+ /* to initialize the fep->cur_rx,... */
+ /* not doing this, will cause a crash in fs_enet_rx_napi */
+ fs_init_bds(fep->ndev);
+
/* Install our interrupt handler. */
r = fs_request_irq(dev, fep->interrupt, "fs_enet-mac", fs_enet_interrupt);
if (r != 0) {
@@ -982,6 +986,7 @@ static struct net_device *fs_init_instance(struct device *dev,
fep = netdev_priv(ndev);
fep->dev = dev;
+ fep->ndev = ndev;
dev_set_drvdata(dev, ndev);
fep->fpi = fpi;
if (fpi->init_ioports)
@@ -1085,7 +1090,6 @@ static struct net_device *fs_init_instance(struct device *dev,
}
registered = 1;
-
return ndev;
err:
@@ -1347,6 +1351,10 @@ static struct of_device_id fs_enet_match[] = {
.compatible = "fsl,cpm1-scc-enet",
.data = (void *)&fs_scc_ops,
},
+ {
+ .compatible = "fsl,cpm2-scc-enet",
+ .data = (void *)&fs_scc_ops,
+ },
#endif
#ifdef CONFIG_FS_ENET_HAS_FCC
{
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index fe3d8a6..3889271 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -50,7 +50,6 @@
#include "fs_enet.h"
/*************************************************/
-
#if defined(CONFIG_CPM1)
/* for a 8xx __raw_xxx's are sufficient */
#define __fs_out32(addr, x) __raw_writel(x, addr)
@@ -65,6 +64,8 @@
#define __fs_out16(addr, x) out_be16(addr, x)
#define __fs_in32(addr) in_be32(addr)
#define __fs_in16(addr) in_be16(addr)
+#define __fs_out8(addr, x) out_8(addr, x)
+#define __fs_in8(addr) in_8(addr)
#endif
/* write, read, set bits, clear bits */
@@ -297,8 +298,13 @@ static void restart(struct net_device *dev)
/* Initialize function code registers for big-endian.
*/
+#ifndef CONFIG_NOT_COHERENT_CACHE
+ W8(ep, sen_genscc.scc_rfcr, SCC_EB | SCC_GBL);
+ W8(ep, sen_genscc.scc_tfcr, SCC_EB | SCC_GBL);
+#else
W8(ep, sen_genscc.scc_rfcr, SCC_EB);
W8(ep, sen_genscc.scc_tfcr, SCC_EB);
+#endif
/* Set maximum bytes per receive buffer.
* This appears to be an Ethernet frame size, not the buffer
diff --git a/include/asm-powerpc/cpm2.h b/include/asm-powerpc/cpm2.h
index f1112c1..14c6496 100644
--- a/include/asm-powerpc/cpm2.h
+++ b/include/asm-powerpc/cpm2.h
@@ -375,6 +375,11 @@ typedef struct scc_param {
uint scc_tcrc; /* Internal */
} sccp_t;
+/* Function code bits.
+*/
+#define SCC_EB ((u_char) 0x10) /* Set big endian byte order */
+#define SCC_GBL ((u_char) 0x20) /* Snooping enabled */
+
/* CPM Ethernet through SCC1.
*/
typedef struct scc_enet {
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-10 9:06 ` Heiko Schocher
@ 2008-01-10 17:27 ` Scott Wood
2008-01-10 18:41 ` Heiko Schocher
0 siblings, 1 reply; 13+ messages in thread
From: Scott Wood @ 2008-01-10 17:27 UTC (permalink / raw)
To: hs; +Cc: linuxppc-dev, Jeff Garzik
Heiko Schocher wrote:
> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> index f2a4d39..f432a18 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -810,6 +810,10 @@ static int fs_enet_open(struct net_device *dev)
> if (fep->fpi->use_napi)
> napi_enable(&fep->napi);
>
> + /* to initialize the fep->cur_rx,... */
> + /* not doing this, will cause a crash in fs_enet_rx_napi */
> + fs_init_bds(fep->ndev);
We should do this just before napi_enable() rather than just after, to
eliminate any chance of a race window.
Looks good otherwise.
-Scott
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-10 17:27 ` Scott Wood
@ 2008-01-10 18:41 ` Heiko Schocher
2008-01-11 16:01 ` Sergej Stepanov
0 siblings, 1 reply; 13+ messages in thread
From: Heiko Schocher @ 2008-01-10 18:41 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Jeff Garzik
Hello Scott,
Scott Wood wrote:
> Heiko Schocher wrote:
>> diff --git a/drivers/net/fs_enet/fs_enet-main.c
>> b/drivers/net/fs_enet/fs_enet-main.c
>> index f2a4d39..f432a18 100644
>> --- a/drivers/net/fs_enet/fs_enet-main.c
>> +++ b/drivers/net/fs_enet/fs_enet-main.c
>> @@ -810,6 +810,10 @@ static int fs_enet_open(struct net_device *dev)
>> if (fep->fpi->use_napi)
>> napi_enable(&fep->napi);
>>
>> + /* to initialize the fep->cur_rx,... */
>> + /* not doing this, will cause a crash in fs_enet_rx_napi */
>> + fs_init_bds(fep->ndev);
>
> We should do this just before napi_enable() rather than just after, to
> eliminate any chance of a race window.
Yes, you are right.
Here is the new patch:
>From 3b8db4261199f0115d8e0188b6bffbc9f5e7673d Mon Sep 17 00:00:00 2001
From: Heiko Schocher <hs@denx.de>
Date: Thu, 10 Jan 2008 19:28:18 +0100
Subject: [PATCH] [POWERPC] Fix Ethernet over SCC on a CPM2,
also Fix crash in fs_enet_rx_napi()
Signed-off-by: Heiko Schocher <hs@denx.de>
---
drivers/net/fs_enet/fs_enet-main.c | 10 +++++++++-
drivers/net/fs_enet/mac-scc.c | 8 +++++++-
include/asm-powerpc/cpm2.h | 5 +++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index f2a4d39..70a0319 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -807,6 +807,10 @@ static int fs_enet_open(struct net_device *dev)
int r;
int err;
+ /* to initialize the fep->cur_rx,... */
+ /* not doing this, will cause a crash in fs_enet_rx_napi */
+ fs_init_bds(fep->ndev);
+
if (fep->fpi->use_napi)
napi_enable(&fep->napi);
@@ -982,6 +986,7 @@ static struct net_device *fs_init_instance(struct device *dev,
fep = netdev_priv(ndev);
fep->dev = dev;
+ fep->ndev = ndev;
dev_set_drvdata(dev, ndev);
fep->fpi = fpi;
if (fpi->init_ioports)
@@ -1085,7 +1090,6 @@ static struct net_device *fs_init_instance(struct device *dev,
}
registered = 1;
-
return ndev;
err:
@@ -1347,6 +1351,10 @@ static struct of_device_id fs_enet_match[] = {
.compatible = "fsl,cpm1-scc-enet",
.data = (void *)&fs_scc_ops,
},
+ {
+ .compatible = "fsl,cpm2-scc-enet",
+ .data = (void *)&fs_scc_ops,
+ },
#endif
#ifdef CONFIG_FS_ENET_HAS_FCC
{
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index fe3d8a6..3889271 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -50,7 +50,6 @@
#include "fs_enet.h"
/*************************************************/
-
#if defined(CONFIG_CPM1)
/* for a 8xx __raw_xxx's are sufficient */
#define __fs_out32(addr, x) __raw_writel(x, addr)
@@ -65,6 +64,8 @@
#define __fs_out16(addr, x) out_be16(addr, x)
#define __fs_in32(addr) in_be32(addr)
#define __fs_in16(addr) in_be16(addr)
+#define __fs_out8(addr, x) out_8(addr, x)
+#define __fs_in8(addr) in_8(addr)
#endif
/* write, read, set bits, clear bits */
@@ -297,8 +298,13 @@ static void restart(struct net_device *dev)
/* Initialize function code registers for big-endian.
*/
+#ifndef CONFIG_NOT_COHERENT_CACHE
+ W8(ep, sen_genscc.scc_rfcr, SCC_EB | SCC_GBL);
+ W8(ep, sen_genscc.scc_tfcr, SCC_EB | SCC_GBL);
+#else
W8(ep, sen_genscc.scc_rfcr, SCC_EB);
W8(ep, sen_genscc.scc_tfcr, SCC_EB);
+#endif
/* Set maximum bytes per receive buffer.
* This appears to be an Ethernet frame size, not the buffer
diff --git a/include/asm-powerpc/cpm2.h b/include/asm-powerpc/cpm2.h
index f1112c1..14c6496 100644
--- a/include/asm-powerpc/cpm2.h
+++ b/include/asm-powerpc/cpm2.h
@@ -375,6 +375,11 @@ typedef struct scc_param {
uint scc_tcrc; /* Internal */
} sccp_t;
+/* Function code bits.
+*/
+#define SCC_EB ((u_char) 0x10) /* Set big endian byte order */
+#define SCC_GBL ((u_char) 0x20) /* Snooping enabled */
+
/* CPM Ethernet through SCC1.
*/
typedef struct scc_enet {
--
1.5.2.2
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-10 18:41 ` Heiko Schocher
@ 2008-01-11 16:01 ` Sergej Stepanov
0 siblings, 0 replies; 13+ messages in thread
From: Sergej Stepanov @ 2008-01-11 16:01 UTC (permalink / raw)
To: hs; +Cc: Scott Wood, linuxppc-dev, Jeff Garzik
Am Donnerstag, den 10.01.2008, 19:41 +0100 schrieb Heiko Schocher:
> Hello Scott,
>
> Scott Wood wrote:
> > Heiko Schocher wrote:
> >> diff --git a/drivers/net/fs_enet/fs_enet-main.c
> >> b/drivers/net/fs_enet/fs_enet-main.c
> >> index f2a4d39..f432a18 100644
> >> --- a/drivers/net/fs_enet/fs_enet-main.c
> >> +++ b/drivers/net/fs_enet/fs_enet-main.c
> >> @@ -810,6 +810,10 @@ static int fs_enet_open(struct net_device *dev)
> >> if (fep->fpi->use_napi)
> >> napi_enable(&fep->napi);
> >>
> >> + /* to initialize the fep->cur_rx,... */
> >> + /* not doing this, will cause a crash in fs_enet_rx_napi */
> >> + fs_init_bds(fep->ndev);
> >
> > We should do this just before napi_enable() rather than just after, to
> > eliminate any chance of a race window.
>
> Yes, you are right.
> Here is the new patch:
>
It works fine.
Sergej.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
2008-01-08 19:05 Anton Vorontsov
2008-01-09 10:46 ` Sergej Stepanov
@ 2008-01-12 22:45 ` Jeff Garzik
1 sibling, 0 replies; 13+ messages in thread
From: Jeff Garzik @ 2008-01-12 22:45 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, linuxppc-dev
Anton Vorontsov wrote:
> Otherwise oops will happen if ethernet device has not been opened:
>
> Unable to handle kernel paging request for data at address 0x0000014c
> Faulting instruction address: 0xc016f7f0
> Oops: Kernel access of bad area, sig: 11 [#1]
> MPC85xx
> NIP: c016f7f0 LR: c01722a0 CTR: 00000000
> REGS: c79ddc70 TRAP: 0300 Not tainted (2.6.24-rc3-g820a386b)
> MSR: 00029000 <EE,ME> CR: 20004428 XER: 20000000
> DEAR: 0000014c, ESR: 00000000
> TASK = c789f5e0[999] 'snmpd' THREAD: c79dc000
> GPR00: c01aceb8 c79ddd20 c789f5e0 00000000 c79ddd3c 00000000 c79ddd64 00000000
> GPR08: 00000000 c7845b60 c79dde3c c01ace80 20004422 200249fc 000002a0 100da728
> GPR16: 100c0000 00000000 00000000 00000000 20022078 00000009 200220e0 bfc85558
> GPR24: c79ddd3c 00000000 00000000 c02e0e70 c022fc64 ffffffff c7845800 bfc85498
> NIP [c016f7f0] phy_ethtool_gset+0x0/0x4c
> LR [c01722a0] fs_get_settings+0x18/0x28
> Call Trace:
> [c79ddd20] [c79dde38] 0xc79dde38 (unreliable)
> [c79ddd30] [c01aceb8] dev_ethtool+0x294/0x11ec
> [c79dde30] [c01aaa44] dev_ioctl+0x454/0x6a8
> [c79ddeb0] [c019b9d4] sock_ioctl+0x84/0x230
> [c79dded0] [c007ded8] do_ioctl+0x34/0x8c
> [c79ddee0] [c007dfbc] vfs_ioctl+0x8c/0x41c
> [c79ddf10] [c007e38c] sys_ioctl+0x40/0x74
> [c79ddf40] [c000d4c0] ret_from_syscall+0x0/0x3c
> Instruction dump:
> 81630000 800b0030 2f800000 419e0010 7c0803a6 4e800021 7c691b78 80010014
> 7d234b78 38210010 7c0803a6 4e800020 <8003014c> 7c6b1b78 38600000 90040004
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
> ---
>
> Just resending it, it feels like it got lost during holidays.
>
> drivers/net/fs_enet/fs_enet-main.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
applied
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-01-12 22:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-09 20:20 [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers Matvejchikov Ilya
-- strict thread matches above, loose matches on Subject: below --
2008-01-09 20:14 Matvejchikov Ilya
2008-01-09 20:10 Matvejchikov Ilya
2008-01-08 19:05 Anton Vorontsov
2008-01-09 10:46 ` Sergej Stepanov
2008-01-09 12:58 ` Heiko Schocher
2008-01-09 18:20 ` Scott Wood
2008-01-10 8:14 ` Heiko Schocher
2008-01-10 9:06 ` Heiko Schocher
2008-01-10 17:27 ` Scott Wood
2008-01-10 18:41 ` Heiko Schocher
2008-01-11 16:01 ` Sergej Stepanov
2008-01-12 22:45 ` Jeff Garzik
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).