* net_device_ops support in bridging and fec_mpc52xx.c
@ 2009-02-18 10:41 Henk Stegeman
2009-02-18 21:48 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Henk Stegeman @ 2009-02-18 10:41 UTC (permalink / raw)
To: linuxppc-dev, bridge
Hello,
I discovered the hard way that because linux bridging uses
net_device_ops, bridging only works with network drivers that publish
their device operations trough net_device_ops.
In my case running:
brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
net_device_ops) gave me a:
Unable to handle kernel paging request...
After changing fec_mpc52xx.c to support net_device_ops the problem was fixed.
If possible some kind of detection in the bridging software is i think
mostly appreciated for early detection of this problem, as it is
pretty hard to relate the error message to a not updated driver.
cheers,
Henk
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index cd8e98b..a2841eb 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -888,6 +888,22 @@ static int mpc52xx_fec_ioctl(struct net_device
*dev, struct ifreq *rq, int cmd)
/* ======================================================================== */
/* OF Driver */
/* ======================================================================== */
+static const struct net_device_ops mpc52xx_fec_netdev_ops = {
+ .ndo_open = mpc52xx_fec_open,
+ .ndo_stop = mpc52xx_fec_close,
+ .ndo_start_xmit = mpc52xx_fec_hard_start_xmit,
+ .ndo_tx_timeout = mpc52xx_fec_tx_timeout,
+ .ndo_get_stats = mpc52xx_fec_get_stats,
+ .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = mpc52xx_fec_set_mac_address,
+ .ndo_do_ioctl = mpc52xx_fec_ioctl,
+
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = mpc52xx_fec_poll_controller,
+#endif
+};
+
static int __devinit
mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
@@ -929,20 +945,7 @@ mpc52xx_fec_probe(struct of_device *op, const
struct of_device_id *match)
return -EBUSY;
/* Init ether ndev with what we have */
- ndev->open = mpc52xx_fec_open;
- ndev->stop = mpc52xx_fec_close;
- ndev->hard_start_xmit = mpc52xx_fec_hard_start_xmit;
- ndev->do_ioctl = mpc52xx_fec_ioctl;
- ndev->ethtool_ops = &mpc52xx_fec_ethtool_ops;
- ndev->get_stats = mpc52xx_fec_get_stats;
- ndev->set_mac_address = mpc52xx_fec_set_mac_address;
- ndev->set_multicast_list = mpc52xx_fec_set_multicast_list;
- ndev->tx_timeout = mpc52xx_fec_tx_timeout;
- ndev->watchdog_timeo = FEC_WATCHDOG_TIMEOUT;
- ndev->base_addr = mem.start;
-#ifdef CONFIG_NET_POLL_CONTROLLER
- ndev->poll_controller = mpc52xx_fec_poll_controller;
-#endif
+ ndev->netdev_ops = &mpc52xx_fec_netdev_ops;
priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-02-18 10:41 net_device_ops support in bridging and fec_mpc52xx.c Henk Stegeman
@ 2009-02-18 21:48 ` David Miller
2009-02-18 22:31 ` Stephen Hemminger
2009-02-19 9:45 ` Henk Stegeman
0 siblings, 2 replies; 8+ messages in thread
From: David Miller @ 2009-02-18 21:48 UTC (permalink / raw)
To: henk.stegeman; +Cc: linuxppc-dev, bridge, netdev
From: Henk Stegeman <henk.stegeman@gmail.com>
Date: Wed, 18 Feb 2009 11:41:14 +0100
Please CC: netdev, now added, on all networking reports and patches.
Thank you.
> I discovered the hard way that because linux bridging uses
> net_device_ops, bridging only works with network drivers that publish
> their device operations trough net_device_ops.
>
> In my case running:
>
> brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
> net_device_ops) gave me a:
>
> Unable to handle kernel paging request...
>
> After changing fec_mpc52xx.c to support net_device_ops the problem was fixed.
>
> If possible some kind of detection in the bridging software is i think
> mostly appreciated for early detection of this problem, as it is
> pretty hard to relate the error message to a not updated driver.
>
> cheers,
>
> Henk
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cd8e98b..a2841eb 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -888,6 +888,22 @@ static int mpc52xx_fec_ioctl(struct net_device
> *dev, struct ifreq *rq, int cmd)
> /* ======================================================================== */
> /* OF Driver */
> /* ======================================================================== */
> +static const struct net_device_ops mpc52xx_fec_netdev_ops = {
> + .ndo_open = mpc52xx_fec_open,
> + .ndo_stop = mpc52xx_fec_close,
> + .ndo_start_xmit = mpc52xx_fec_hard_start_xmit,
> + .ndo_tx_timeout = mpc52xx_fec_tx_timeout,
> + .ndo_get_stats = mpc52xx_fec_get_stats,
> + .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list,
> + .ndo_validate_addr = eth_validate_addr,
> + .ndo_set_mac_address = mpc52xx_fec_set_mac_address,
> + .ndo_do_ioctl = mpc52xx_fec_ioctl,
> +
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + .ndo_poll_controller = mpc52xx_fec_poll_controller,
> +#endif
> +};
> +
>
> static int __devinit
> mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
> @@ -929,20 +945,7 @@ mpc52xx_fec_probe(struct of_device *op, const
> struct of_device_id *match)
> return -EBUSY;
>
> /* Init ether ndev with what we have */
> - ndev->open = mpc52xx_fec_open;
> - ndev->stop = mpc52xx_fec_close;
> - ndev->hard_start_xmit = mpc52xx_fec_hard_start_xmit;
> - ndev->do_ioctl = mpc52xx_fec_ioctl;
> - ndev->ethtool_ops = &mpc52xx_fec_ethtool_ops;
> - ndev->get_stats = mpc52xx_fec_get_stats;
> - ndev->set_mac_address = mpc52xx_fec_set_mac_address;
> - ndev->set_multicast_list = mpc52xx_fec_set_multicast_list;
> - ndev->tx_timeout = mpc52xx_fec_tx_timeout;
> - ndev->watchdog_timeo = FEC_WATCHDOG_TIMEOUT;
> - ndev->base_addr = mem.start;
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> - ndev->poll_controller = mpc52xx_fec_poll_controller;
> -#endif
> + ndev->netdev_ops = &mpc52xx_fec_netdev_ops;
>
> priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-02-18 21:48 ` David Miller
@ 2009-02-18 22:31 ` Stephen Hemminger
2009-02-19 9:45 ` Henk Stegeman
1 sibling, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2009-02-18 22:31 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, henk.stegeman, bridge, netdev
On Wed, 18 Feb 2009 13:48:52 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Henk Stegeman <henk.stegeman@gmail.com>
> Date: Wed, 18 Feb 2009 11:41:14 +0100
>
> Please CC: netdev, now added, on all networking reports and patches.
>
> Thank you.
>
> > I discovered the hard way that because linux bridging uses
> > net_device_ops, bridging only works with network drivers that publish
> > their device operations trough net_device_ops.
> >
> > In my case running:
> >
> > brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
> > net_device_ops) gave me a:
> >
> > Unable to handle kernel paging request...
> >
> > After changing fec_mpc52xx.c to support net_device_ops the problem was fixed.
> >
> > If possible some kind of detection in the bridging software is i think
> > mostly appreciated for early detection of this problem, as it is
> > pretty hard to relate the error message to a not updated driver.
> >
> > cheers,
> >
> > Henk
The normal register_netdevice stuff take care of setting up net_device_ops
for old style drivers. Was there something different about how this
device was being setup?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-02-18 21:48 ` David Miller
2009-02-18 22:31 ` Stephen Hemminger
@ 2009-02-19 9:45 ` Henk Stegeman
2009-03-10 17:13 ` Grant Likely
1 sibling, 1 reply; 8+ messages in thread
From: Henk Stegeman @ 2009-02-19 9:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: netdev, bridge
I must have made a mistake when I tested the previous patch, I
discovered later it still had errors:
- I had accidentally removed the base address in the fec_mpc52xx driver.
- The priv->phydev pointer was sometimes not initialized (NULL) but
still passed by the fec_mpc52xx driver, this pointer is then used
unchecked by the eth_tool_* functions (used by bridging to determine
port priority). As far as I see this depends on whether
mpc52xx_fec_open (or mpc52xx_fec_close) is called which in turn call
mpc52xx_init_phy to initialize priv->phydev. My work around checks the
priv->phydev pointer in the fec_mpc52xx driver and returns -ENODEV to
indicate there's no physical device. Big chance this is not the right
way to handle the problem, but it works, hopefully someone with some
more fundamental Linux network driver experience can pick this up or
give me some hints on this.
At least bridging now works on my board in combination with the
fec_mpc52xx driver.
ifconfig eth0 0.0.0.0 down
ifconfig eth1 0.0.0.0 down
brctl addbr br0
brctl setfd br0 0
brctl stp br0 off
ifconfig br0 192.168.1.30 down
ifconfig br0 up
brctl addif br0 eth0
ifconfig eth0 up
brctl addif br0 eth1
ifconfig eth1 up
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index cd8e98b..e228973 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -847,24 +847,40 @@ static void mpc52xx_fec_get_drvinfo(struct
net_device *dev,
static int mpc52xx_fec_get_settings(struct net_device *dev, struct
ethtool_cmd *cmd)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return -ENODEV;
+
return phy_ethtool_gset(priv->phydev, cmd);
}
static int mpc52xx_fec_set_settings(struct net_device *dev, struct
ethtool_cmd *cmd)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return -ENODEV;
+
return phy_ethtool_sset(priv->phydev, cmd);
}
static u32 mpc52xx_fec_get_msglevel(struct net_device *dev)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return 0;
+
return priv->msg_enable;
}
static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 level)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return;
+
priv->msg_enable = level;
}
@@ -882,12 +898,31 @@ static int mpc52xx_fec_ioctl(struct net_device
*dev, struct ifreq *rq, int cmd)
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ if (!priv->phydev)
+ return -ENODEV;
+
return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd);
}
/* ======================================================================== */
/* OF Driver */
/* ======================================================================== */
+static const struct net_device_ops mpc52xx_fec_netdev_ops = {
+ .ndo_open = mpc52xx_fec_open,
+ .ndo_stop = mpc52xx_fec_close,
+ .ndo_start_xmit = mpc52xx_fec_hard_start_xmit,
+ .ndo_tx_timeout = mpc52xx_fec_tx_timeout,
+ .ndo_get_stats = mpc52xx_fec_get_stats,
+ .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = mpc52xx_fec_set_mac_address,
+ .ndo_do_ioctl = mpc52xx_fec_ioctl,
+
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = mpc52xx_fec_poll_controller,
+#endif
+};
+
static int __devinit
mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
@@ -929,20 +964,10 @@ mpc52xx_fec_probe(struct of_device *op, const
struct of_device_id *match)
return -EBUSY;
/* Init ether ndev with what we have */
- ndev->open = mpc52xx_fec_open;
- ndev->stop = mpc52xx_fec_close;
- ndev->hard_start_xmit = mpc52xx_fec_hard_start_xmit;
- ndev->do_ioctl = mpc52xx_fec_ioctl;
ndev->ethtool_ops = &mpc52xx_fec_ethtool_ops;
- ndev->get_stats = mpc52xx_fec_get_stats;
- ndev->set_mac_address = mpc52xx_fec_set_mac_address;
- ndev->set_multicast_list = mpc52xx_fec_set_multicast_list;
- ndev->tx_timeout = mpc52xx_fec_tx_timeout;
ndev->watchdog_timeo = FEC_WATCHDOG_TIMEOUT;
ndev->base_addr = mem.start;
-#ifdef CONFIG_NET_POLL_CONTROLLER
- ndev->poll_controller = mpc52xx_fec_poll_controller;
-#endif
+ ndev->netdev_ops = &mpc52xx_fec_netdev_ops;
priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */
On Wed, Feb 18, 2009 at 10:48 PM, David Miller <davem@davemloft.net> wrote:
> From: Henk Stegeman <henk.stegeman@gmail.com>
> Date: Wed, 18 Feb 2009 11:41:14 +0100
>
> Please CC: netdev, now added, on all networking reports and patches.
>
> Thank you.
>
>> I discovered the hard way that because linux bridging uses
>> net_device_ops, bridging only works with network drivers that publish
>> their device operations trough net_device_ops.
>>
>> In my case running:
>>
>> brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
>> net_device_ops) gave me a:
>>
>> Unable to handle kernel paging request...
>>
>> After changing fec_mpc52xx.c to support net_device_ops the problem was fixed.
>>
>> If possible some kind of detection in the bridging software is i think
>> mostly appreciated for early detection of this problem, as it is
>> pretty hard to relate the error message to a not updated driver.
>>
>> cheers,
>>
>> Henk
>>
>> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
>> index cd8e98b..a2841eb 100644
>> --- a/drivers/net/fec_mpc52xx.c
>> +++ b/drivers/net/fec_mpc52xx.c
>> @@ -888,6 +888,22 @@ static int mpc52xx_fec_ioctl(struct net_device
>> *dev, struct ifreq *rq, int cmd)
>> /* ======================================================================== */
>> /* OF Driver */
>> /* ======================================================================== */
>> +static const struct net_device_ops mpc52xx_fec_netdev_ops = {
>> + .ndo_open = mpc52xx_fec_open,
>> + .ndo_stop = mpc52xx_fec_close,
>> + .ndo_start_xmit = mpc52xx_fec_hard_start_xmit,
>> + .ndo_tx_timeout = mpc52xx_fec_tx_timeout,
>> + .ndo_get_stats = mpc52xx_fec_get_stats,
>> + .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list,
>> + .ndo_validate_addr = eth_validate_addr,
>> + .ndo_set_mac_address = mpc52xx_fec_set_mac_address,
>> + .ndo_do_ioctl = mpc52xx_fec_ioctl,
>> +
>> +#ifdef CONFIG_NET_POLL_CONTROLLER
>> + .ndo_poll_controller = mpc52xx_fec_poll_controller,
>> +#endif
>> +};
>> +
>>
>> static int __devinit
>> mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
>> @@ -929,20 +945,7 @@ mpc52xx_fec_probe(struct of_device *op, const
>> struct of_device_id *match)
>> return -EBUSY;
>>
>> /* Init ether ndev with what we have */
>> - ndev->open = mpc52xx_fec_open;
>> - ndev->stop = mpc52xx_fec_close;
>> - ndev->hard_start_xmit = mpc52xx_fec_hard_start_xmit;
>> - ndev->do_ioctl = mpc52xx_fec_ioctl;
>> - ndev->ethtool_ops = &mpc52xx_fec_ethtool_ops;
>> - ndev->get_stats = mpc52xx_fec_get_stats;
>> - ndev->set_mac_address = mpc52xx_fec_set_mac_address;
>> - ndev->set_multicast_list = mpc52xx_fec_set_multicast_list;
>> - ndev->tx_timeout = mpc52xx_fec_tx_timeout;
>> - ndev->watchdog_timeo = FEC_WATCHDOG_TIMEOUT;
>> - ndev->base_addr = mem.start;
>> -#ifdef CONFIG_NET_POLL_CONTROLLER
>> - ndev->poll_controller = mpc52xx_fec_poll_controller;
>> -#endif
>> + ndev->netdev_ops = &mpc52xx_fec_netdev_ops;
>>
>> priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-02-19 9:45 ` Henk Stegeman
@ 2009-03-10 17:13 ` Grant Likely
2009-03-10 17:19 ` David Miller
2009-03-21 22:00 ` Grant Likely
0 siblings, 2 replies; 8+ messages in thread
From: Grant Likely @ 2009-03-10 17:13 UTC (permalink / raw)
To: Henk Stegeman; +Cc: linuxppc-dev, bridge, Jeff Garzik, netdev
Hi Henk,
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Can you please repost with a blurb for the commit description and your
signed-off-by line? The blub below makes sense in the context of this
mailing list thread, but it won't be very useful for someone looking
at the commit message in git. Also, your patch is line-wrap damaged
(cut and paste into your mail client doesn't usually work) and has
inconsistent whitespace (run it through scripts/checkpatch.pl).
Jeff, after Henk provides his s-o-b line, do you want to pick it up,
or should I merge it through my mpc52xx powerpc tree (via benh).
Thanks,
g.
On Thu, Feb 19, 2009 at 3:45 AM, Henk Stegeman <henk.stegeman@gmail.com> wr=
ote:
> I must have made a mistake when I tested the previous patch, I
> discovered later it still had errors:
> - I had accidentally removed the base address in the fec_mpc52xx driver.
> - The priv->phydev pointer was sometimes not initialized (NULL) but
> still passed by the fec_mpc52xx driver, this pointer is then used
> unchecked by the eth_tool_* functions (used by bridging to determine
> port priority). As far as I see this depends on whether
> mpc52xx_fec_open (or mpc52xx_fec_close) is called which in turn call
> mpc52xx_init_phy to initialize priv->phydev. My work around checks the
> priv->phydev pointer in the fec_mpc52xx driver and returns -ENODEV to
> indicate there's no physical device. Big chance this is not the right
> way to handle the problem, but it works, hopefully someone with some
> more fundamental Linux network driver experience can pick this up or
> give me some hints on this.
>
> At least bridging now works on my board in combination with the
> fec_mpc52xx driver.
>
> ifconfig eth0 0.0.0.0 down
> ifconfig eth1 0.0.0.0 down
> brctl addbr br0
> brctl setfd br0 0
> brctl stp br0 off
> ifconfig br0 192.168.1.30 down
> ifconfig br0 up
> brctl addif br0 eth0
> ifconfig eth0 up
> brctl addif br0 eth1
> ifconfig eth1 up
>
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cd8e98b..e228973 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -847,24 +847,40 @@ static void mpc52xx_fec_get_drvinfo(struct
> net_device *dev,
> =A0static int mpc52xx_fec_get_settings(struct net_device *dev, struct
> ethtool_cmd *cmd)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> +
> + =A0 =A0 =A0 if (!priv->phydev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> +
> =A0 =A0 =A0 =A0return phy_ethtool_gset(priv->phydev, cmd);
> =A0}
>
> =A0static int mpc52xx_fec_set_settings(struct net_device *dev, struct
> ethtool_cmd *cmd)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> +
> + =A0 =A0 =A0 if (!priv->phydev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> +
> =A0 =A0 =A0 =A0return phy_ethtool_sset(priv->phydev, cmd);
> =A0}
>
> =A0static u32 mpc52xx_fec_get_msglevel(struct net_device *dev)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> +
> + =A0 =A0 =A0 if (!priv->phydev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> +
> =A0 =A0 =A0 =A0return priv->msg_enable;
> =A0}
>
> =A0static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 level=
)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
> +
> + =A0 =A0 =A0 if (!priv->phydev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> +
> =A0 =A0 =A0 =A0priv->msg_enable =3D level;
> =A0}
>
> @@ -882,12 +898,31 @@ static int mpc52xx_fec_ioctl(struct net_device
> *dev, struct ifreq *rq, int cmd)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>
> + =A0 =A0 =A0 if (!priv->phydev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> +
> =A0 =A0 =A0 =A0return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd);
> =A0}
>
> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
*/
> =A0/* OF Driver =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
*/
> +static const struct net_device_ops mpc52xx_fec_netdev_ops =3D {
> + =A0 =A0 =A0 .ndo_open =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_open,
> + =A0 =A0 =A0 .ndo_stop =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_close=
,
> + =A0 =A0 =A0 .ndo_start_xmit =A0 =A0 =A0 =A0 =3D mpc52xx_fec_hard_start_=
xmit,
> + =A0 =A0 =A0 .ndo_tx_timeout =A0 =A0 =A0 =A0 =3D mpc52xx_fec_tx_timeout,
> + =A0 =A0 =A0 .ndo_get_stats =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_get_stats=
,
> + =A0 =A0 =A0 .ndo_set_multicast_list =3D mpc52xx_fec_set_multicast_list,
> + =A0 =A0 =A0 .ndo_validate_addr =A0 =A0 =A0=3D eth_validate_addr,
> + =A0 =A0 =A0 .ndo_set_mac_address =A0 =A0=3D mpc52xx_fec_set_mac_address=
,
> + =A0 =A0 =A0 .ndo_do_ioctl =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_ioctl,
> +
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + =A0 =A0 =A0 .ndo_poll_controller =A0 =A0 =3D mpc52xx_fec_poll_controlle=
r,
> +#endif
> +};
> +
>
> =A0static int __devinit
> =A0mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *mat=
ch)
> @@ -929,20 +964,10 @@ mpc52xx_fec_probe(struct of_device *op, const
> struct of_device_id *match)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EBUSY;
>
> =A0 =A0 =A0 =A0/* Init ether ndev with what we have */
> - =A0 =A0 =A0 ndev->open =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_open;
> - =A0 =A0 =A0 ndev->stop =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_close=
;
> - =A0 =A0 =A0 ndev->hard_start_xmit =A0 =3D mpc52xx_fec_hard_start_xmit;
> - =A0 =A0 =A0 ndev->do_ioctl =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_ioctl;
> =A0 =A0 =A0 =A0ndev->ethtool_ops =A0 =A0 =A0 =3D &mpc52xx_fec_ethtool_ops=
;
> - =A0 =A0 =A0 ndev->get_stats =A0 =A0 =A0 =A0 =3D mpc52xx_fec_get_stats;
> - =A0 =A0 =A0 ndev->set_mac_address =A0 =3D mpc52xx_fec_set_mac_address;
> - =A0 =A0 =A0 ndev->set_multicast_list =3D mpc52xx_fec_set_multicast_list=
;
> - =A0 =A0 =A0 ndev->tx_timeout =A0 =A0 =A0 =A0=3D mpc52xx_fec_tx_timeout;
> =A0 =A0 =A0 =A0ndev->watchdog_timeo =A0 =A0=3D FEC_WATCHDOG_TIMEOUT;
> =A0 =A0 =A0 =A0ndev->base_addr =A0 =A0 =A0 =A0 =3D mem.start;
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> - =A0 =A0 =A0 ndev->poll_controller =3D mpc52xx_fec_poll_controller;
> -#endif
> + =A0 =A0 =A0 ndev->netdev_ops =3D &mpc52xx_fec_netdev_ops;
>
> =A0 =A0 =A0 =A0priv->t_irq =3D priv->r_irq =3D ndev->irq =3D NO_IRQ; /* I=
RQ are free for now */
>
>
>
> On Wed, Feb 18, 2009 at 10:48 PM, David Miller <davem@davemloft.net> wrot=
e:
>> From: Henk Stegeman <henk.stegeman@gmail.com>
>> Date: Wed, 18 Feb 2009 11:41:14 +0100
>>
>> Please CC: netdev, now added, on all networking reports and patches.
>>
>> Thank you.
>>
>>> I discovered the hard way that because linux bridging uses
>>> net_device_ops, bridging only works with network drivers that publish
>>> their device operations trough net_device_ops.
>>>
>>> In my case running:
>>>
>>> brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
>>> net_device_ops) gave me a:
>>>
>>> Unable to handle kernel paging request...
>>>
>>> After changing fec_mpc52xx.c to support net_device_ops the problem was =
fixed.
>>>
>>> If possible some kind of detection in the bridging software is i think
>>> mostly appreciated for early detection of this problem, as it is
>>> pretty hard to relate the error message to a not updated driver.
>>>
>>> cheers,
>>>
>>> Henk
>>>
>>> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
>>> index cd8e98b..a2841eb 100644
>>> --- a/drivers/net/fec_mpc52xx.c
>>> +++ b/drivers/net/fec_mpc52xx.c
>>> @@ -888,6 +888,22 @@ static int mpc52xx_fec_ioctl(struct net_device
>>> *dev, struct ifreq *rq, int cmd)
>>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D */
>>> =A0/* OF Driver =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D */
>>> +static const struct net_device_ops mpc52xx_fec_netdev_ops =3D {
>>> + =A0 =A0 =A0 .ndo_open =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_ope=
n,
>>> + =A0 =A0 =A0 .ndo_stop =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_clo=
se,
>>> + =A0 =A0 =A0 .ndo_start_xmit =A0 =A0 =A0 =A0 =3D mpc52xx_fec_hard_star=
t_xmit,
>>> + =A0 =A0 =A0 .ndo_tx_timeout =A0 =A0 =A0 =A0 =3D mpc52xx_fec_tx_timeou=
t,
>>> + =A0 =A0 =A0 .ndo_get_stats =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_get_sta=
ts,
>>> + =A0 =A0 =A0 .ndo_set_multicast_list =3D mpc52xx_fec_set_multicast_lis=
t,
>>> + =A0 =A0 =A0 .ndo_validate_addr =A0 =A0 =A0=3D eth_validate_addr,
>>> + =A0 =A0 =A0 .ndo_set_mac_address =A0 =A0=3D mpc52xx_fec_set_mac_addre=
ss,
>>> + =A0 =A0 =A0 .ndo_do_ioctl =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_ioctl,
>>> +
>>> +#ifdef CONFIG_NET_POLL_CONTROLLER
>>> + =A0 =A0 =A0 .ndo_poll_controller =A0 =A0 =3D mpc52xx_fec_poll_control=
ler,
>>> +#endif
>>> +};
>>> +
>>>
>>> =A0static int __devinit
>>> =A0mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *m=
atch)
>>> @@ -929,20 +945,7 @@ mpc52xx_fec_probe(struct of_device *op, const
>>> struct of_device_id *match)
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
>>>
>>> =A0 =A0 =A0 /* Init ether ndev with what we have */
>>> - =A0 =A0 ndev->open =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_open;
>>> - =A0 =A0 ndev->stop =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_close;
>>> - =A0 =A0 ndev->hard_start_xmit =A0 =3D mpc52xx_fec_hard_start_xmit;
>>> - =A0 =A0 ndev->do_ioctl =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_ioctl;
>>> - =A0 =A0 ndev->ethtool_ops =A0 =A0 =A0 =3D &mpc52xx_fec_ethtool_ops;
>>> - =A0 =A0 ndev->get_stats =A0 =A0 =A0 =A0 =3D mpc52xx_fec_get_stats;
>>> - =A0 =A0 ndev->set_mac_address =A0 =3D mpc52xx_fec_set_mac_address;
>>> - =A0 =A0 ndev->set_multicast_list =3D mpc52xx_fec_set_multicast_list;
>>> - =A0 =A0 ndev->tx_timeout =A0 =A0 =A0 =A0=3D mpc52xx_fec_tx_timeout;
>>> - =A0 =A0 ndev->watchdog_timeo =A0 =A0=3D FEC_WATCHDOG_TIMEOUT;
>>> - =A0 =A0 ndev->base_addr =A0 =A0 =A0 =A0 =3D mem.start;
>>> -#ifdef CONFIG_NET_POLL_CONTROLLER
>>> - =A0 =A0 ndev->poll_controller =3D mpc52xx_fec_poll_controller;
>>> -#endif
>>> + =A0 =A0 ndev->netdev_ops =3D &mpc52xx_fec_netdev_ops;
>>>
>>> =A0 =A0 =A0 priv->t_irq =3D priv->r_irq =3D ndev->irq =3D NO_IRQ; /* IR=
Q are free for now */
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@ozlabs.org
>>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-03-10 17:13 ` Grant Likely
@ 2009-03-10 17:19 ` David Miller
2009-03-10 17:36 ` Grant Likely
2009-03-21 22:00 ` Grant Likely
1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2009-03-10 17:19 UTC (permalink / raw)
To: grant.likely; +Cc: linuxppc-dev, netdev, henk.stegeman, bridge, jgarzik
From: Grant Likely <grant.likely@secretlab.ca>
Date: Tue, 10 Mar 2009 11:13:02 -0600
> Hi Henk,
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
...
> Jeff, after Henk provides his s-o-b line, do you want to pick it up,
> or should I merge it through my mpc52xx powerpc tree (via benh).
Send it to netdev, CC:'d to me, Jeff hasn't been handling networking
driver changes for a while now.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-03-10 17:19 ` David Miller
@ 2009-03-10 17:36 ` Grant Likely
0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2009-03-10 17:36 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, netdev, henk.stegeman, bridge, jgarzik
On Tue, Mar 10, 2009 at 11:19 AM, David Miller <davem@davemloft.net> wrote:
> Send it to netdev, CC:'d to me, Jeff hasn't been handling networking
> driver changes for a while now.
Ah, okay. I didn't know. I looked in MAINTAINERS today, and Jeff is
listed there for NETWORK DEVICE DRIVERS.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net_device_ops support in bridging and fec_mpc52xx.c
2009-03-10 17:13 ` Grant Likely
2009-03-10 17:19 ` David Miller
@ 2009-03-21 22:00 ` Grant Likely
1 sibling, 0 replies; 8+ messages in thread
From: Grant Likely @ 2009-03-21 22:00 UTC (permalink / raw)
To: Henk Stegeman; +Cc: linuxppc-dev, bridge, Jeff Garzik, netdev
Henk,
At the very least, I still need a signed-off-by: line from you on this one.
g.
On Tue, Mar 10, 2009 at 11:13 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> Hi Henk,
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> Can you please repost with a blurb for the commit description and your
> signed-off-by line? =A0The blub below makes sense in the context of this
> mailing list thread, but it won't be very useful for someone looking
> at the commit message in git. =A0Also, your patch is line-wrap damaged
> (cut and paste into your mail client doesn't usually work) and has
> inconsistent whitespace (run it through scripts/checkpatch.pl).
>
> Jeff, after Henk provides his s-o-b line, do you want to pick it up,
> or should I merge it through my mpc52xx powerpc tree (via benh).
>
> Thanks,
> g.
>
> On Thu, Feb 19, 2009 at 3:45 AM, Henk Stegeman <henk.stegeman@gmail.com> =
wrote:
>> I must have made a mistake when I tested the previous patch, I
>> discovered later it still had errors:
>> - I had accidentally removed the base address in the fec_mpc52xx driver.
>> - The priv->phydev pointer was sometimes not initialized (NULL) but
>> still passed by the fec_mpc52xx driver, this pointer is then used
>> unchecked by the eth_tool_* functions (used by bridging to determine
>> port priority). As far as I see this depends on whether
>> mpc52xx_fec_open (or mpc52xx_fec_close) is called which in turn call
>> mpc52xx_init_phy to initialize priv->phydev. My work around checks the
>> priv->phydev pointer in the fec_mpc52xx driver and returns -ENODEV to
>> indicate there's no physical device. Big chance this is not the right
>> way to handle the problem, but it works, hopefully someone with some
>> more fundamental Linux network driver experience can pick this up or
>> give me some hints on this.
>>
>> At least bridging now works on my board in combination with the
>> fec_mpc52xx driver.
>>
>> ifconfig eth0 0.0.0.0 down
>> ifconfig eth1 0.0.0.0 down
>> brctl addbr br0
>> brctl setfd br0 0
>> brctl stp br0 off
>> ifconfig br0 192.168.1.30 down
>> ifconfig br0 up
>> brctl addif br0 eth0
>> ifconfig eth0 up
>> brctl addif br0 eth1
>> ifconfig eth1 up
>>
>>
>> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
>> index cd8e98b..e228973 100644
>> --- a/drivers/net/fec_mpc52xx.c
>> +++ b/drivers/net/fec_mpc52xx.c
>> @@ -847,24 +847,40 @@ static void mpc52xx_fec_get_drvinfo(struct
>> net_device *dev,
>> =A0static int mpc52xx_fec_get_settings(struct net_device *dev, struct
>> ethtool_cmd *cmd)
>> =A0{
>> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>> +
>> + =A0 =A0 =A0 if (!priv->phydev)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> +
>> =A0 =A0 =A0 =A0return phy_ethtool_gset(priv->phydev, cmd);
>> =A0}
>>
>> =A0static int mpc52xx_fec_set_settings(struct net_device *dev, struct
>> ethtool_cmd *cmd)
>> =A0{
>> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>> +
>> + =A0 =A0 =A0 if (!priv->phydev)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> +
>> =A0 =A0 =A0 =A0return phy_ethtool_sset(priv->phydev, cmd);
>> =A0}
>>
>> =A0static u32 mpc52xx_fec_get_msglevel(struct net_device *dev)
>> =A0{
>> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>> +
>> + =A0 =A0 =A0 if (!priv->phydev)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> +
>> =A0 =A0 =A0 =A0return priv->msg_enable;
>> =A0}
>>
>> =A0static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 leve=
l)
>> =A0{
>> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>> +
>> + =A0 =A0 =A0 if (!priv->phydev)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>> +
>> =A0 =A0 =A0 =A0priv->msg_enable =3D level;
>> =A0}
>>
>> @@ -882,12 +898,31 @@ static int mpc52xx_fec_ioctl(struct net_device
>> *dev, struct ifreq *rq, int cmd)
>> =A0{
>> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D netdev_priv(dev);
>>
>> + =A0 =A0 =A0 if (!priv->phydev)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> +
>> =A0 =A0 =A0 =A0return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd);
>> =A0}
>>
>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
*/
>> =A0/* OF Driver =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
*/
>> +static const struct net_device_ops mpc52xx_fec_netdev_ops =3D {
>> + =A0 =A0 =A0 .ndo_open =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_open=
,
>> + =A0 =A0 =A0 .ndo_stop =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_clos=
e,
>> + =A0 =A0 =A0 .ndo_start_xmit =A0 =A0 =A0 =A0 =3D mpc52xx_fec_hard_start=
_xmit,
>> + =A0 =A0 =A0 .ndo_tx_timeout =A0 =A0 =A0 =A0 =3D mpc52xx_fec_tx_timeout=
,
>> + =A0 =A0 =A0 .ndo_get_stats =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_get_stat=
s,
>> + =A0 =A0 =A0 .ndo_set_multicast_list =3D mpc52xx_fec_set_multicast_list=
,
>> + =A0 =A0 =A0 .ndo_validate_addr =A0 =A0 =A0=3D eth_validate_addr,
>> + =A0 =A0 =A0 .ndo_set_mac_address =A0 =A0=3D mpc52xx_fec_set_mac_addres=
s,
>> + =A0 =A0 =A0 .ndo_do_ioctl =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_ioctl,
>> +
>> +#ifdef CONFIG_NET_POLL_CONTROLLER
>> + =A0 =A0 =A0 .ndo_poll_controller =A0 =A0 =3D mpc52xx_fec_poll_controll=
er,
>> +#endif
>> +};
>> +
>>
>> =A0static int __devinit
>> =A0mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *ma=
tch)
>> @@ -929,20 +964,10 @@ mpc52xx_fec_probe(struct of_device *op, const
>> struct of_device_id *match)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EBUSY;
>>
>> =A0 =A0 =A0 =A0/* Init ether ndev with what we have */
>> - =A0 =A0 =A0 ndev->open =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_open=
;
>> - =A0 =A0 =A0 ndev->stop =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_clos=
e;
>> - =A0 =A0 =A0 ndev->hard_start_xmit =A0 =3D mpc52xx_fec_hard_start_xmit;
>> - =A0 =A0 =A0 ndev->do_ioctl =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_ioctl;
>> =A0 =A0 =A0 =A0ndev->ethtool_ops =A0 =A0 =A0 =3D &mpc52xx_fec_ethtool_op=
s;
>> - =A0 =A0 =A0 ndev->get_stats =A0 =A0 =A0 =A0 =3D mpc52xx_fec_get_stats;
>> - =A0 =A0 =A0 ndev->set_mac_address =A0 =3D mpc52xx_fec_set_mac_address;
>> - =A0 =A0 =A0 ndev->set_multicast_list =3D mpc52xx_fec_set_multicast_lis=
t;
>> - =A0 =A0 =A0 ndev->tx_timeout =A0 =A0 =A0 =A0=3D mpc52xx_fec_tx_timeout=
;
>> =A0 =A0 =A0 =A0ndev->watchdog_timeo =A0 =A0=3D FEC_WATCHDOG_TIMEOUT;
>> =A0 =A0 =A0 =A0ndev->base_addr =A0 =A0 =A0 =A0 =3D mem.start;
>> -#ifdef CONFIG_NET_POLL_CONTROLLER
>> - =A0 =A0 =A0 ndev->poll_controller =3D mpc52xx_fec_poll_controller;
>> -#endif
>> + =A0 =A0 =A0 ndev->netdev_ops =3D &mpc52xx_fec_netdev_ops;
>>
>> =A0 =A0 =A0 =A0priv->t_irq =3D priv->r_irq =3D ndev->irq =3D NO_IRQ; /* =
IRQ are free for now */
>>
>>
>>
>> On Wed, Feb 18, 2009 at 10:48 PM, David Miller <davem@davemloft.net> wro=
te:
>>> From: Henk Stegeman <henk.stegeman@gmail.com>
>>> Date: Wed, 18 Feb 2009 11:41:14 +0100
>>>
>>> Please CC: netdev, now added, on all networking reports and patches.
>>>
>>> Thank you.
>>>
>>>> I discovered the hard way that because linux bridging uses
>>>> net_device_ops, bridging only works with network drivers that publish
>>>> their device operations trough net_device_ops.
>>>>
>>>> In my case running:
>>>>
>>>> brctl addif br0 eth0 (where eth0 fec_mpc52xx.c did not yet support
>>>> net_device_ops) gave me a:
>>>>
>>>> Unable to handle kernel paging request...
>>>>
>>>> After changing fec_mpc52xx.c to support net_device_ops the problem was=
fixed.
>>>>
>>>> If possible some kind of detection in the bridging software is i think
>>>> mostly appreciated for early detection of this problem, as it is
>>>> pretty hard to relate the error message to a not updated driver.
>>>>
>>>> cheers,
>>>>
>>>> Henk
>>>>
>>>> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
>>>> index cd8e98b..a2841eb 100644
>>>> --- a/drivers/net/fec_mpc52xx.c
>>>> +++ b/drivers/net/fec_mpc52xx.c
>>>> @@ -888,6 +888,22 @@ static int mpc52xx_fec_ioctl(struct net_device
>>>> *dev, struct ifreq *rq, int cmd)
>>>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D */
>>>> =A0/* OF Driver =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
*/
>>>> =A0/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D */
>>>> +static const struct net_device_ops mpc52xx_fec_netdev_ops =3D {
>>>> + =A0 =A0 =A0 .ndo_open =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_op=
en,
>>>> + =A0 =A0 =A0 .ndo_stop =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_cl=
ose,
>>>> + =A0 =A0 =A0 .ndo_start_xmit =A0 =A0 =A0 =A0 =3D mpc52xx_fec_hard_sta=
rt_xmit,
>>>> + =A0 =A0 =A0 .ndo_tx_timeout =A0 =A0 =A0 =A0 =3D mpc52xx_fec_tx_timeo=
ut,
>>>> + =A0 =A0 =A0 .ndo_get_stats =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_get_st=
ats,
>>>> + =A0 =A0 =A0 .ndo_set_multicast_list =3D mpc52xx_fec_set_multicast_li=
st,
>>>> + =A0 =A0 =A0 .ndo_validate_addr =A0 =A0 =A0=3D eth_validate_addr,
>>>> + =A0 =A0 =A0 .ndo_set_mac_address =A0 =A0=3D mpc52xx_fec_set_mac_addr=
ess,
>>>> + =A0 =A0 =A0 .ndo_do_ioctl =A0 =A0 =A0 =A0 =A0 =3D mpc52xx_fec_ioctl,
>>>> +
>>>> +#ifdef CONFIG_NET_POLL_CONTROLLER
>>>> + =A0 =A0 =A0 .ndo_poll_controller =A0 =A0 =3D mpc52xx_fec_poll_contro=
ller,
>>>> +#endif
>>>> +};
>>>> +
>>>>
>>>> =A0static int __devinit
>>>> =A0mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *=
match)
>>>> @@ -929,20 +945,7 @@ mpc52xx_fec_probe(struct of_device *op, const
>>>> struct of_device_id *match)
>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
>>>>
>>>> =A0 =A0 =A0 /* Init ether ndev with what we have */
>>>> - =A0 =A0 ndev->open =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_open;
>>>> - =A0 =A0 ndev->stop =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_close;
>>>> - =A0 =A0 ndev->hard_start_xmit =A0 =3D mpc52xx_fec_hard_start_xmit;
>>>> - =A0 =A0 ndev->do_ioctl =A0 =A0 =A0 =A0 =A0=3D mpc52xx_fec_ioctl;
>>>> - =A0 =A0 ndev->ethtool_ops =A0 =A0 =A0 =3D &mpc52xx_fec_ethtool_ops;
>>>> - =A0 =A0 ndev->get_stats =A0 =A0 =A0 =A0 =3D mpc52xx_fec_get_stats;
>>>> - =A0 =A0 ndev->set_mac_address =A0 =3D mpc52xx_fec_set_mac_address;
>>>> - =A0 =A0 ndev->set_multicast_list =3D mpc52xx_fec_set_multicast_list;
>>>> - =A0 =A0 ndev->tx_timeout =A0 =A0 =A0 =A0=3D mpc52xx_fec_tx_timeout;
>>>> - =A0 =A0 ndev->watchdog_timeo =A0 =A0=3D FEC_WATCHDOG_TIMEOUT;
>>>> - =A0 =A0 ndev->base_addr =A0 =A0 =A0 =A0 =3D mem.start;
>>>> -#ifdef CONFIG_NET_POLL_CONTROLLER
>>>> - =A0 =A0 ndev->poll_controller =3D mpc52xx_fec_poll_controller;
>>>> -#endif
>>>> + =A0 =A0 ndev->netdev_ops =3D &mpc52xx_fec_netdev_ops;
>>>>
>>>> =A0 =A0 =A0 priv->t_irq =3D priv->r_irq =3D ndev->irq =3D NO_IRQ; /* I=
RQ are free for now */
>>>> _______________________________________________
>>>> Linuxppc-dev mailing list
>>>> Linuxppc-dev@ozlabs.org
>>>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-03-21 22:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-18 10:41 net_device_ops support in bridging and fec_mpc52xx.c Henk Stegeman
2009-02-18 21:48 ` David Miller
2009-02-18 22:31 ` Stephen Hemminger
2009-02-19 9:45 ` Henk Stegeman
2009-03-10 17:13 ` Grant Likely
2009-03-10 17:19 ` David Miller
2009-03-10 17:36 ` Grant Likely
2009-03-21 22:00 ` Grant Likely
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).