* 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: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 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
* [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 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:10 [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:20 Matvejchikov Ilya 2008-01-09 20:14 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).