Re: [patch 2.6.25-rc2-git] rndis_host: fix transfer size negotiation
> > This patch should resolve a problem that's troubled support for > > some RNDIS peripherals. It seems to have boiled down to using a > > variable to establish transfer size limits before it was assigned, > > which caused those devices to fallback to a default "jumbogram" > > mode we don't support. Fix by assigning it earlier for RNDIS. > > Any chance that something like this is appropriate for rndis_wlan? I'd expect so; but, isn't it already getting that benefit? - Dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.25-rc3] smc91x section fix
On Sunday 24 February 2008, Sam Ravnborg wrote: > From a quick look this is wrong. > smc_drv_probe is assined the .probe member so it is used during > hotplug and thus should be __devinit. > Likewise smc_probe is used by smc_drv_probe and thus smc_probe > should be __devinit too. Thing is, with only rare exceptions, devices on the platform bus are *NOT* hotpluggable. So using __devinit/__devexit and friends adds up to no more than a waste of I-space. Nico may know if this driver is one of the rare exceptions. Example, is it used in PCMCIA cards? See also platform_driver_probe(), which is a much safer way to handle such issues. I've seen it save a full page in some drivers, but usually it's less. - Dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2.6.25-rc3] smc91x section fix
Section fixup:
WARNING: drivers/net/built-in.o(.text+0x1a2c): Section mismatch
in reference from the function smc_drv_probe()
to the function .init.text:smc_probe()
The function smc_drv_probe() references
the function __init smc_probe().
This is often because smc_drv_probe lacks a __init
annotation or the annotation of smc_probe is wrong.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/smc91x.c |2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/net/smc91x.c 2008-02-24 18:50:32.0 -0800
+++ b/drivers/net/smc91x.c 2008-02-24 19:16:29.0 -0800
@@ -2121,7 +2121,7 @@ static void smc_release_datacs(struct pl
* 0 --> there is a device
* anything else, error
*/
-static int smc_drv_probe(struct platform_device *pdev)
+static int __init smc_drv_probe(struct platform_device *pdev)
{
struct net_device *ndev;
struct resource *res, *ires;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2.6.25-rc2-git] rndis_host: fix transfer size negotiation
From: Jean-Christophe Dubois <[EMAIL PROTECTED]>
This patch should resolve a problem that's troubled support for
some RNDIS peripherals. It seems to have boiled down to using a
variable to establish transfer size limits before it was assigned,
which caused those devices to fallback to a default "jumbogram"
mode we don't support. Fix by assigning it earlier for RNDIS.
Signed-off-by: Jean-Christophe Dubois <[EMAIL PROTECTED]>
[ cleanups ]
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
This bugfix should be merged before 2.6.25-final.
drivers/net/usb/rndis_host.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
--- g26.orig/drivers/net/usb/rndis_host.c 2008-02-01 22:24:38.0
-0800
+++ g26/drivers/net/usb/rndis_host.c2008-02-16 12:55:35.0 -0800
@@ -16,10 +16,6 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-
-// #define DEBUG // error path messages, extra info
-// #define VERBOSE // more; success messages
-
#include
#include
#include
@@ -318,6 +314,14 @@ generic_rndis_bind(struct usbnet *dev, s
net->hard_header_len += sizeof (struct rndis_data_hdr);
dev->hard_mtu = net->mtu + net->hard_header_len;
+ dev->maxpacket = usb_maxpacket(dev->udev, dev->out, 1);
+ if (dev->maxpacket == 0) {
+ if (netif_msg_probe(dev))
+ dev_dbg(&intf->dev, "dev->maxpacket can't be 0\n");
+ retval = -EINVAL;
+ goto fail_and_release;
+ }
+
dev->rx_urb_size = dev->hard_mtu + (dev->maxpacket + 1);
dev->rx_urb_size &= ~(dev->maxpacket - 1);
u.init->max_transfer_size = cpu_to_le32(dev->rx_urb_size);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND/patch 2.6.25-rc2-git] net/enc28j60: low power mode
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle; this
low power mode cuts that power usage by a factor of around 100.
This version provides a generic routine to poll a register until
its masked value equals some value ... e.g. bit set or cleared.
It's basically what the previous wait_phy_ready() did, but this
version is generalized to support the handshaking needed to
enter and exit low power mode.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Signed-off-by: Claudio Lanconelli <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c | 82 ++---
1 files changed, 58 insertions(+), 24 deletions(-)
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -400,26 +400,31 @@ enc28j60_packet_write(struct enc28j60_ne
mutex_unlock(&priv->lock);
}
-/*
- * Wait until the PHY operation is complete.
- */
-static int wait_phy_ready(struct enc28j60_net *priv)
+static unsigned long msec20_to_jiffies;
+
+static int poll_ready(struct enc28j60_net *priv, u8 reg, u8 mask, u8 val)
{
- unsigned long timeout = jiffies + 20 * HZ / 1000;
- int ret = 1;
+ unsigned long timeout = jiffies + msec20_to_jiffies;
/* 20 msec timeout read */
- while (nolock_regb_read(priv, MISTAT) & MISTAT_BUSY) {
+ while ((nolock_regb_read(priv, reg) & mask) != val) {
if (time_after(jiffies, timeout)) {
if (netif_msg_drv(priv))
- printk(KERN_DEBUG DRV_NAME
- ": PHY ready timeout!\n");
- ret = 0;
- break;
+ dev_dbg(&priv->spi->dev,
+ "reg %02x ready timeout!\n", reg);
+ return -ETIMEDOUT;
}
cpu_relax();
}
- return ret;
+ return 0;
+}
+
+/*
+ * Wait until the PHY operation is complete.
+ */
+static int wait_phy_ready(struct enc28j60_net *priv)
+{
+ return poll_ready(priv, MISTAT, MISTAT_BUSY, 0) ? 0 : 1;
}
/*
@@ -594,6 +599,32 @@ static void nolock_txfifo_init(struct en
nolock_regw_write(priv, ETXNDL, end);
}
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive. (However, we
+ * can't stay in lowpower mode during suspend with WOL active.)
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ if (netif_msg_drv(priv))
+ dev_dbg(&priv->spi->dev, "%s power...\n",
+ is_low ? "low" : "high");
+
+ mutex_lock(&priv->lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ poll_ready(priv, ESTAT, ESTAT_RXBUSY, 0);
+ poll_ready(priv, ECON1, ECON1_TXRTS, 0);
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ poll_ready(priv, ESTAT, ESTAT_CLKRDY, ESTAT_CLKRDY);
+ /* caller sets ECON1_RXEN */
+ }
+ mutex_unlock(&priv->lock);
+}
+
static int enc28j60_hw_init(struct enc28j60_net *priv)
{
u8 reg;
@@ -612,8 +643,8 @@ static int enc28j60_hw_init(struct enc28
priv->tx_retry_count = 0;
priv->max_pk_counter = 0;
priv->rxfilter = RXFILTER_NORMAL;
- /* enable address auto increment */
- nolock_regb_write(priv, ECON2, ECON2_AUTOINC);
+ /* enable address auto increment and voltage regulator powersave */
+ nolock_regb_write(priv, ECON2, ECON2_AUTOINC | ECON2_VRPS);
nolock_rxfifo_init(priv, RXSTART_INIT, RXEND_INIT);
nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
@@ -690,7 +721,7 @@ static int enc28j60_hw_init(struct enc28
static void enc28j60_hw_enable(struct enc28j60_net *priv)
{
- /* enable interrutps */
+ /* enable interrupts */
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME ": %s() enabling interrupts.\n",
__FUNCTION__);
@@ -726,15 +757,12 @@ enc28j60_setlink(struct net_device *ndev
int ret = 0;
if (!priv->hw_enable) {
- if (autoneg == AUTONEG_DISABLE && speed == SPEED_10) {
+ /* link is in low power mode now; duplex setting
+* will take effect on next enc28j60_hw_init().
+*/
+ if (autoneg == AUTONEG_DISABLE && speed == SPEED_10)
priv->full_duplex = (duplex == DUPLEX_FULL);
- if (!enc28j60_hw_init(priv)) {
-
[RESEND/patch 2.6.25-rc2-git] net/enc28j60: section fix
Minor bugfixes to the enc28j60 driver ... wrong section marking,
indentation, and bogus use of spi_bus_type. There's still major
overuse of printk; essentially every place it's used should switch
to dev_*() messaging primitives.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Acked-by: Claudio Lanconelli <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/net/enc28j60.c2008-01-29 11:07:14.0 -0800
+++ b/drivers/net/enc28j60.c2008-01-29 12:43:18.0 -0800
@@ -1555,7 +1555,7 @@ error_alloc:
return ret;
}
-static int enc28j60_remove(struct spi_device *spi)
+static int __devexit enc28j60_remove(struct spi_device *spi)
{
struct enc28j60_net *priv = dev_get_drvdata(&spi->dev);
@@ -1572,9 +1572,8 @@ static int enc28j60_remove(struct spi_de
static struct spi_driver enc28j60_driver = {
.driver = {
.name = DRV_NAME,
- .bus = &spi_bus_type,
.owner = THIS_MODULE,
- },
+},
.probe = enc28j60_probe,
.remove = __devexit_p(enc28j60_remove),
};
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND/patch 2.6.25-rc2-git] net/enc28j60: oops fix
Prevent oops on enc28j60 packet RX: make sure buffers are aligned.
Not all architectures support unaligned accesses in kernel space.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Acked-by: Claudio Lanconelli <[EMAIL PROTECTED]>
---
Seems the enc28j60 patches didn't get picked up, ergo resend...
they're not in net-2.6.git, even this oops fix.
drivers/net/enc28j60.c |3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/net/enc28j60.c2008-02-06 09:29:00.0 -0800
+++ b/drivers/net/enc28j60.c2008-02-06 09:30:03.0 -0800
@@ -900,7 +900,7 @@ static void enc28j60_hw_rx(struct net_de
if (RSV_GETBIT(rxstat, RSV_LENCHECKERR))
ndev->stats.rx_frame_errors++;
} else {
- skb = dev_alloc_skb(len);
+ skb = dev_alloc_skb(len + NET_IP_ALIGN);
if (!skb) {
if (netif_msg_rx_err(priv))
dev_err(&ndev->dev,
@@ -908,6 +908,7 @@ static void enc28j60_hw_rx(struct net_de
ndev->stats.rx_dropped++;
} else {
skb->dev = ndev;
+ skb_reserve(skb, NET_IP_ALIGN);
/* copy the packet from the receive buffer */
enc28j60_mem_read(priv, priv->next_pk_ptr + sizeof(rsv),
len, skb_put(skb, len));
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] plusb.c patched to support Belkin F5U258 USB host-to-host cable
Here's a patch with cleanups and without various encoding bugs.
Can you verify it still works?
Also, some of the Prolific chips use some bizarre control requests,
which by all rights should not be needed. They seem to exist only
to cope with things the device firmware should have handled ... like
resetting one end of a link after it's unplugged.
When you test this, please enable CONFIG_USB_DEBUG and let us know
if those the host software still needs to do that reset ... or at
least, whether the diagnostic appears.
- Dave
== CUT HERE
Some plusb driver updates: pl25a1 support (based on info from
Tony Gibbs), and various cleanups.
---
drivers/net/usb/Kconfig |2 -
drivers/net/usb/plusb.c | 52
2 files changed, 36 insertions(+), 18 deletions(-)
--- g26.orig/drivers/net/usb/plusb.c2008-02-16 11:49:04.0 -0800
+++ g26/drivers/net/usb/plusb.c 2008-02-16 12:28:54.0 -0800
@@ -17,9 +17,6 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-// #define DEBUG // error path messages, extra info
-// #define VERBOSE // more; success messages
-
#include
#include
#include
@@ -45,6 +42,14 @@
* seems to get wedged under load. Prolific docs are weak, and
* don't identify differences between PL2301 and PL2302, much less
* anything to explain the different PL2302 versions observed.
+ *
+ * NOTE: pl2501 has several modes, including pl2301 and pl2302
+ * compatibility. Some docs suggest the difference between 2301
+ * and 2302 is only to make MS-Windows use a different driver...
+ *
+ * pl25a1 glue based on patch from Tony Gibbs. Prolific "docs" on
+ * this chip are as usual incomplete about what control messages
+ * are supported.
*/
/*
@@ -86,16 +91,20 @@ pl_set_QuickLink_features(struct usbnet
static int pl_reset(struct usbnet *dev)
{
+ int status;
+
/* some units seem to need this reset, others reject it utterly.
* FIXME be more like "naplink" or windows drivers.
*/
- (void) pl_set_QuickLink_features(dev,
+ status = pl_set_QuickLink_features(dev,
PL_S_EN|PL_RESET_OUT|PL_RESET_IN|PL_PEER_E);
+ if (status != 0 && netif_msg_probe(dev))
+ devdbg(dev, "pl_reset --> %d\n", status);
return 0;
}
static const struct driver_infoprolific_info = {
- .description = "Prolific PL-2301/PL-2302",
+ .description = "Prolific PL-2301/PL-2302/PL-25A1",
.flags =FLAG_NO_SETINT,
/* some PL-2302 versions seem to fail usb_set_interface() */
.reset =pl_reset,
@@ -110,16 +119,25 @@ static const struct driver_info prolific
*/
static const struct usb_device_id products [] = {
+ /* full speed cables */
+ {
+ USB_DEVICE(0x067b, 0x), /* PL-2301 */
+ .driver_info = (unsigned long) &prolific_info,
+ }, {
+ USB_DEVICE(0x067b, 0x0001), /* PL-2302 */
+ .driver_info = (unsigned long) &prolific_info,
+ },
+
+ /* high speed cables */
+ {
+ USB_DEVICE(0x067b, 0x25a1), /* PL-25A1, no eeprom */
+ .driver_info = (unsigned long) &prolific_info,
+ }, {
+ USB_DEVICE(0x050d, 0x258a), /* Belkin F5U258 (PL-25A1) */
+ .driver_info = (unsigned long) &prolific_info,
+ },
-{
- USB_DEVICE(0x067b, 0x), // PL-2301
- .driver_info = (unsigned long) &prolific_info,
-}, {
- USB_DEVICE(0x067b, 0x0001), // PL-2302
- .driver_info = (unsigned long) &prolific_info,
-},
-
- { },// END
+ { },/* END */
};
MODULE_DEVICE_TABLE(usb, products);
@@ -134,16 +152,16 @@ static struct usb_driver plusb_driver =
static int __init plusb_init(void)
{
- return usb_register(&plusb_driver);
+ return usb_register(&plusb_driver);
}
module_init(plusb_init);
static void __exit plusb_exit(void)
{
- usb_deregister(&plusb_driver);
+ usb_deregister(&plusb_driver);
}
module_exit(plusb_exit);
MODULE_AUTHOR("David Brownell");
-MODULE_DESCRIPTION("Prolific PL-2301/2302 USB Host to Host Link Driver");
+MODULE_DESCRIPTION("Prolific PL-2301/2302/25A1 USB Host to Host Link Driver");
MODULE_LICENSE("GPL");
--- g26.orig/drivers/net/usb/Kconfig2008-02-16 12:23:13.0 -0800
+++ g26/drivers/net/usb/Kconfig 2008-02-16 12:23:39.0 -0800
@@ -208,7 +208,7 @@ config USB_NET_NET1080
optionally with LEDs that indicate traffic
config USB_NET_PLUSB
- tristate "Prolific PL-2301/2302 based cables"
+ tristate "Prolific PL-2301/2302/25A1 based cables"
Re: [PATCH] plusb.c patched to support Belkin F5U258 USB host-to-host cable
On Thursday 14 February 2008, tony_gibbs wrote: > I have tried to make the changes I have been working on and testing with > your help into a patch as attached. > > Please let me know what you think of it. It arrived line wrapped, and turned on debug options that should stay off by default ... it couldn't merge, on those bases alone. Also, "PL-25A1hack" isn't a product name. ;) Did you get this to pass data back and forth yet? The last report I had from you was that it wouldn't pass data. The Prolific hardware tends to enter that mode after a while. (Maybe there's documentation unseen by any Linux developer, saying how to make those chips actually work...) But you reported that they didn't seem to work even when freshly plugged in. - Dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
On Monday 11 February 2008, Claudio Lanconelli wrote:
> I have tried your latest patch. Only after the following change it
> works fine (no more rx errors during ifconfig up).
Hmm, what chip rev do you have? Different errata and all.
ISTR mine is rev4; so, not the most current, but not the
oldest version either.
> I added enc28j60_lowpower(false) just before enc28j60_hw_init()
Hmm, I'd have expected it would go best *before* that, but
what you include below shows it going *after* ...
If there's some problem where reset doesn't work correctly
in low power mode, who knows what else would need manual
resetting.
> @@ -1318,8 +1347,9 @@
> }
> return -EADDRNOTAVAIL;
> }
> - /* Reset the hardware here */
> + /* Reset the hardware here (and take it out of low power mode) */
> enc28j60_hw_disable(priv);
> + enc28j60_lowpower(priv, false);
> if (!enc28j60_hw_init(priv)) {
> if (netif_msg_ifup(priv))
> dev_err(&dev->dev, "hw_reset() failed\n");
>
> With this addition you can add Acked-by line.
Better yet, since I can't reproduce the problem, why don't
you just update my latest patch with the relevant version
of this tweak, and then forward it as "From: " me and with
both our signoffs. That's the usual way to cope with this
type of tweaking. (Not all updates to your driver should
need your signoff, but then most patches shouldn't need
very many iterations either.)
- Dave
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
On Thursday 07 February 2008, Claudio Lanconelli wrote: > David Brownell wrote: > > How long did that take? I did about four dozen > > > > ifconfig eth1 up > > sleep 3 > > ifconfig eth1 down > > > > cycles ... it worked fine. The "sleep" was to let the link > > negotiation complete. > > > > > After a couple of : > > ifconfig eth0 down > (wait just 1 second) > ifconfig eth0 up > > the network is frozen. > > If I do another > ifconfig eth0 down > (wait just 1 second) > ifconfig eth0 up > > restarts. > It's random, no rule. I write a shell loop to do that, and added a "ping -c2" too. If that was done before the "sleep 1" no packets flowed. Afterwards, no problem -- ever. (And outside the loop, "ethool -s eth1 duplex full".) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: low power mode
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle; this
low power mode cuts that power usage by a factor of around 100.
This version provides a generic routine to poll a register until
its masked value equals some value ... e.g. bit set or cleared.
It's basically what the previous wait_phy_ready() did, but this
version is generalized to support the handshaking needed to
enter and exit low power mode.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c | 81 ++---
1 files changed, 57 insertions(+), 24 deletions(-)
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -400,26 +400,31 @@ enc28j60_packet_write(struct enc28j60_ne
mutex_unlock(&priv->lock);
}
-/*
- * Wait until the PHY operation is complete.
- */
-static int wait_phy_ready(struct enc28j60_net *priv)
+static unsigned long msec20_to_jiffies;
+
+static int poll_ready(struct enc28j60_net *priv, u8 reg, u8 mask, u8 val)
{
- unsigned long timeout = jiffies + 20 * HZ / 1000;
- int ret = 1;
+ unsigned long timeout = jiffies + msec20_to_jiffies;
/* 20 msec timeout read */
- while (nolock_regb_read(priv, MISTAT) & MISTAT_BUSY) {
+ while ((nolock_regb_read(priv, reg) & mask) != val) {
if (time_after(jiffies, timeout)) {
if (netif_msg_drv(priv))
- printk(KERN_DEBUG DRV_NAME
- ": PHY ready timeout!\n");
- ret = 0;
- break;
+ dev_dbg(&priv->spi->dev,
+ "reg %02x ready timeout!\n", reg);
+ return -ETIMEDOUT;
}
cpu_relax();
}
- return ret;
+ return 0;
+}
+
+/*
+ * Wait until the PHY operation is complete.
+ */
+static int wait_phy_ready(struct enc28j60_net *priv)
+{
+ return poll_ready(priv, MISTAT, MISTAT_BUSY, 0) ? 0 : 1;
}
/*
@@ -594,6 +599,32 @@ static void nolock_txfifo_init(struct en
nolock_regw_write(priv, ETXNDL, end);
}
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive. (However, we
+ * can't stay in lowpower mode during suspend with WOL active.)
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ if (netif_msg_drv(priv))
+ dev_dbg(&priv->spi->dev, "%s power...\n",
+ is_low ? "low" : "high");
+
+ mutex_lock(&priv->lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ poll_ready(priv, ESTAT, ESTAT_RXBUSY, 0);
+ poll_ready(priv, ECON1, ECON1_TXRTS, 0);
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ poll_ready(priv, ESTAT, ESTAT_CLKRDY, ESTAT_CLKRDY);
+ /* caller sets ECON1_RXEN */
+ }
+ mutex_unlock(&priv->lock);
+}
+
static int enc28j60_hw_init(struct enc28j60_net *priv)
{
u8 reg;
@@ -612,8 +643,8 @@ static int enc28j60_hw_init(struct enc28
priv->tx_retry_count = 0;
priv->max_pk_counter = 0;
priv->rxfilter = RXFILTER_NORMAL;
- /* enable address auto increment */
- nolock_regb_write(priv, ECON2, ECON2_AUTOINC);
+ /* enable address auto increment and voltage regulator powersave */
+ nolock_regb_write(priv, ECON2, ECON2_AUTOINC | ECON2_VRPS);
nolock_rxfifo_init(priv, RXSTART_INIT, RXEND_INIT);
nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
@@ -690,7 +721,7 @@ static int enc28j60_hw_init(struct enc28
static void enc28j60_hw_enable(struct enc28j60_net *priv)
{
- /* enable interrutps */
+ /* enable interrupts */
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME ": %s() enabling interrupts.\n",
__FUNCTION__);
@@ -726,15 +757,12 @@ enc28j60_setlink(struct net_device *ndev
int ret = 0;
if (!priv->hw_enable) {
- if (autoneg == AUTONEG_DISABLE && speed == SPEED_10) {
+ /* link is in low power mode now; duplex setting
+* will take effect on next enc28j60_hw_init().
+*/
+ if (autoneg == AUTONEG_DISABLE && speed == SPEED_10)
priv->full_duplex = (duplex == DUPLEX_FULL);
- if (!enc28j60_hw_init(priv)) {
- if (netif_msg_drv(priv))
-
Re: [patch 2.6.24-git] net/enc28j60: low power mode
On Thursday 07 February 2008, Claudio Lanconelli wrote:
> Sorry,
> let me repeat what I said in previous mail.
> I propose you to add set_lowpower(true) in the enc28j60_probe()
As the current patch does...
> and in the enc28j60_net_close() after enc28j60_hw_disable().
> Probably we don't need to set_lowpower(false) in enc28j60_net_open() since
> it performs a soft reset with enc28j60_hw_init() (not sure).
The current patch sets the device in low power mode in hw_disable(),
and takes it out of that mode in hw_enable(). I can move them; and
the only "soft" thing about this chip's reset is when it starts from
a protocol command not the reset command.
> Furthermore, as you suggested, we also need to remove hw_init() from the
> setlink()
> because hw_init() is called when we bring link up.
>
> --- enc28j60.c 20 Dec 2007 10:47:01 - 1.22
> +++ enc28j60.c 7 Feb 2008 11:07:20 -
> @@ -740,12 +740,6 @@
> if (!priv->hw_enable) {
> if (autoneg == AUTONEG_DISABLE && speed == SPEED_10) {
> priv->full_duplex = (duplex == DUPLEX_FULL);
> - if (!enc28j60_hw_init(priv)) {
> - if (netif_msg_drv(priv))
> - dev_err(&ndev->dev,
> - "hw_reset() failed\n");
> - ret = -EINVAL;
> - }
Right. Without the patch mangling presumably done by your mailer. ;)
> } else {
> if (netif_msg_link(priv))
> dev_warn(&ndev->dev,
>
> Can you update your low power patch with these modifications?
>
Done -- see my next patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: low power mode
On Wednesday 06 February 2008, Claudio Lanconelli wrote:
> > + for (;;) {
> > + tmp = nolock_regb_read(priv, ESTAT);
> > + if (!(tmp & ESTAT_RXBUSY))
> > + break;
> > + }
> >
> Avoid infinite waiting loops, please.
> Look at enc28j60_phy_read() for example.
Updated patch is appended.
== CUT HERE
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle; this
low power mode cuts that power usage by a factor of around 100.
This version provides a generic routine to poll a register until
its masked value equals some value ... e.g. bit set or cleared.
It's basically what the previous wait_phy_ready() did, but this
version is generalized to support the handshaking needed to
enter and exit low power mode.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c | 70 ++---
1 files changed, 55 insertions(+), 15 deletions(-)
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -400,26 +400,31 @@ enc28j60_packet_write(struct enc28j60_ne
mutex_unlock(&priv->lock);
}
-/*
- * Wait until the PHY operation is complete.
- */
-static int wait_phy_ready(struct enc28j60_net *priv)
+static unsigned long msec20_to_jiffies;
+
+static int poll_ready(struct enc28j60_net *priv, u8 reg, u8 mask, u8 val)
{
- unsigned long timeout = jiffies + 20 * HZ / 1000;
- int ret = 1;
+ unsigned long timeout = jiffies + msec20_to_jiffies;
/* 20 msec timeout read */
- while (nolock_regb_read(priv, MISTAT) & MISTAT_BUSY) {
+ while ((nolock_regb_read(priv, reg) & mask) != val) {
if (time_after(jiffies, timeout)) {
if (netif_msg_drv(priv))
- printk(KERN_DEBUG DRV_NAME
- ": PHY ready timeout!\n");
- ret = 0;
- break;
+ dev_dbg(&priv->spi->dev,
+ "reg %02x ready timeout!\n", reg);
+ return -ETIMEDOUT;
}
cpu_relax();
}
- return ret;
+ return 0;
+}
+
+/*
+ * Wait until the PHY operation is complete.
+ */
+static int wait_phy_ready(struct enc28j60_net *priv)
+{
+ return poll_ready(priv, MISTAT, MISTAT_BUSY, 0) ? 0 : 1;
}
/*
@@ -594,6 +599,31 @@ static void nolock_txfifo_init(struct en
nolock_regw_write(priv, ETXNDL, end);
}
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive.
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ if (netif_msg_drv(priv))
+ dev_dbg(&priv->spi->dev, "%s power...\n",
+ is_low ? "low" : "high");
+
+ mutex_lock(&priv->lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ poll_ready(priv, ESTAT, ESTAT_RXBUSY, 0);
+ poll_ready(priv, ECON1, ECON1_TXRTS, 0);
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ poll_ready(priv, ESTAT, ESTAT_CLKRDY, ESTAT_CLKRDY);
+ /* caller sets ECON1_RXEN */
+ }
+ mutex_unlock(&priv->lock);
+}
+
static int enc28j60_hw_init(struct enc28j60_net *priv)
{
u8 reg;
@@ -612,8 +642,8 @@ static int enc28j60_hw_init(struct enc28
priv->tx_retry_count = 0;
priv->max_pk_counter = 0;
priv->rxfilter = RXFILTER_NORMAL;
- /* enable address auto increment */
- nolock_regb_write(priv, ECON2, ECON2_AUTOINC);
+ /* enable address auto increment and voltage regulator powersave */
+ nolock_regb_write(priv, ECON2, ECON2_AUTOINC | ECON2_VRPS);
nolock_rxfifo_init(priv, RXSTART_INIT, RXEND_INIT);
nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
@@ -690,7 +720,9 @@ static int enc28j60_hw_init(struct enc28
static void enc28j60_hw_enable(struct enc28j60_net *priv)
{
- /* enable interrutps */
+ enc28j60_lowpower(priv, false);
+
+ /* enable interrupts */
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME ": %s() enabling interrupts.\n",
__FUNCTION__);
@@ -717,6 +749,8 @@ static void enc28j60_hw_disable(struct e
nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
priv->hw_enable = false;
mutex_unlock(&priv->lock);
+
+ enc28j60_lowpower(priv, tr
Re: [patch 2.6.24-git] net/enc28j60: section fix
Minor bugfixes to the enc28j60 driver ... wrong section marking and
indentation, and bogus use of spi_bus_type. (Setting the bus type
of a driver is a SPI framework responsibility.)
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
Bonus patch.
drivers/net/enc28j60.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -1555,7 +1555,7 @@ error_alloc:
return ret;
}
-static int enc28j60_remove(struct spi_device *spi)
+static int __devexit enc28j60_remove(struct spi_device *spi)
{
struct enc28j60_net *priv = dev_get_drvdata(&spi->dev);
@@ -1572,9 +1572,8 @@ static int enc28j60_remove(struct spi_de
static struct spi_driver enc28j60_driver = {
.driver = {
.name = DRV_NAME,
- .bus = &spi_bus_type,
.owner = THIS_MODULE,
- },
+},
.probe = enc28j60_probe,
.remove = __devexit_p(enc28j60_remove),
};
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix, low power mode
On Wednesday 06 February 2008, Claudio Lanconelli wrote: > Good idea, but with your patch applied, after some ifconfig down - > ifconfig up cycle, the > enc28j60 is left in an unknown state and it doesn' Rx/Tx anything. How long did that take? I did about four dozen ifconfig eth1 up sleep 3 ifconfig eth1 down cycles ... it worked fine. The "sleep" was to let the link negotiation complete. - Dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: low power mode
> > Keep enc28j60 chips in low-power mode when they're not in use.
> > At typically 120 mA, these chips run hot even when idle. Low
> > power mode cuts that power usage by a factor of around 100.
>
> Good idea, but with your patch applied, after some ifconfig down -
> ifconfig up cycle, the
> enc28j60 is left in an unknown state and it doesn' Rx/Tx anything.
The driver seemed to already have some goofage there:
# ifconfig eth1 up
net eth1: link down
net eth1: link down
net eth1: normal mode
net eth1: multicast mode
net eth1: multicast mode
#
I'd normally expect it not to go down when told to go up, and
then only to do the multicast thing once ...
> In such cases If I dump the counters with ifconfig I got rx error
> counter > 0 and the RX and TX packets counters are freezed.
> Actually I don't know what causes the freeze, it needs investigation.
> The cause can be the rx error condition or the power down/up commands.
> May be receiving packets while it's going to wakeup causes problems.
The enc28j60_setlink() was odd too. It insists the link be down
before changing duplex, then brings the link up ... so I had to
put it down again to maintain the "lowpower if not up" invariant.
But the way it brings the link up is to enc28j60_hw_init(), which
it also does when bringing the link up. So there's no need to
bring the link up when changing duplex ...
> > +/*
> > + * Low power mode shrinks power consumption about 100x, so we'd like
> > + * the chip to be in that mode whenever it's inactive.
> > + */
> > +static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
> > +{
> > + int tmp;
> > +
> > + dev_dbg(&priv->spi->dev, "%s power...\n", is_low ? "low" : "high");
>
> use
> if(netif_msg_drv(priv)) ...
> Doing so we can switch on/off messages runtime using ethtool.
OK, although there's still a role for "-DDEBUG" compile-time
mesage removal.
> printk(... instead of dev_dbg(), please.
Actually, I had to resist sending a patch converting this driver
to stop abusing printk in all those locations it should have been
using dev_*() messaging instead.
And to use the dev_*() messaging correctly ... the above shouldn't
have said "net eth1", but either "eth1:" or "enc28j60 spi1.0".
The general policy in the kernel is to avoid using printk for driver
messaging. Couple messages to the hardware, so that when there are
two instances it's easy to tell which chip is affected. That happens
for free with the "driver and device" prefix of the dev_*() messages.
In absense of interface renaming, network devices sometimes use "eth1"
style message prefixes ... after a previous message coupling that to
the particular hardware.
This driver also abuses __FUNCTION__ (general policy: don't use it)
and underuses the "-DDEBUG" compile-time string removal.
> > +
> > + mutex_lock(&priv->lock);
> > + if (is_low) {
> > + nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
> > + for (;;) {
> > + tmp = nolock_regb_read(priv, ESTAT);
> > + if (!(tmp & ESTAT_RXBUSY))
> > + break;
> > + }
> >
> Avoid infinite waiting loops, please.
> Look at enc28j60_phy_read() for example.
Then there should be a single routine for all such busy-wait loops,
so each such usage doesn't need to be open-coded. Less space, and
more obviously correct. I'll add one and make phy_read() use it too.
> > + for (;;) {
> > + tmp = nolock_regb_read(priv, ECON1);
> > + if (!(tmp & ECON1_TXRTS))
> > + break;
> > + }
> >
> idem
> > + /* ECON2_VRPS was set during initialization */
> > + nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
> > + } else {
> > + nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
> > + for (;;) {
> > + tmp = nolock_regb_read(priv, ESTAT);
> > + if (tmp & ESTAT_CLKRDY)
> > + break;
> > + }
> >
> idem
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.24-git] net/enc28j60: oops fix
> > Prevent unaligned packet oops on enc28j60 packet RX.
>
> How can I reproduce the unaligned packet oops?
Use any architecture that doesn't support unaligned accesses
in kernel space. I used AVR32 for this (it's got a pretty
solid SPI master controller driver). ARM isn't immune either,
and there are others. That's why NET_IP_ALIGN exists.
> Did you use any utilities to force this condition?
"ping" ... the first RX packet oopsed.
> Can you split the patch in 2 parts, unaligned rx and power save?
Below is: unaligned access.
- Dave
=== CUT HERE
Prevent oops on enc28j60 packet RX: make sure buffers are aligned.
Not all architectures support unaligned accesses in kernel space.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c |3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/net/enc28j60.c2008-02-06 09:29:00.0 -0800
+++ b/drivers/net/enc28j60.c2008-02-06 09:30:03.0 -0800
@@ -900,7 +900,7 @@ static void enc28j60_hw_rx(struct net_de
if (RSV_GETBIT(rxstat, RSV_LENCHECKERR))
ndev->stats.rx_frame_errors++;
} else {
- skb = dev_alloc_skb(len);
+ skb = dev_alloc_skb(len + NET_IP_ALIGN);
if (!skb) {
if (netif_msg_rx_err(priv))
dev_err(&ndev->dev,
@@ -908,6 +908,7 @@ static void enc28j60_hw_rx(struct net_de
ndev->stats.rx_dropped++;
} else {
skb->dev = ndev;
+ skb_reserve(skb, NET_IP_ALIGN);
/* copy the packet from the receive buffer */
enc28j60_mem_read(priv, priv->next_pk_ptr + sizeof(rsv),
len, skb_put(skb, len));
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2.6.24-git] net/enc28j60: oops fix, low power mode
From: David Brownell <[EMAIL PROTECTED]>
Prevent unaligned packet oops on enc28j60 packet RX.
Keep enc28j60 chips in low-power mode when they're not in use.
At typically 120 mA, these chips run hot even when idle. Low
power mode cuts that power usage by a factor of around 100.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/enc28j60.c | 54 +
1 files changed, 50 insertions(+), 4 deletions(-)
--- avr.orig/drivers/net/enc28j60.c 2008-02-05 10:04:22.0 -0800
+++ avr/drivers/net/enc28j60.c 2008-02-05 10:50:50.0 -0800
@@ -594,6 +594,43 @@ static void nolock_txfifo_init(struct en
nolock_regw_write(priv, ETXNDL, end);
}
+/*
+ * Low power mode shrinks power consumption about 100x, so we'd like
+ * the chip to be in that mode whenever it's inactive.
+ */
+static void enc28j60_lowpower(struct enc28j60_net *priv, bool is_low)
+{
+ int tmp;
+
+ dev_dbg(&priv->spi->dev, "%s power...\n", is_low ? "low" : "high");
+
+ mutex_lock(&priv->lock);
+ if (is_low) {
+ nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
+ for (;;) {
+ tmp = nolock_regb_read(priv, ESTAT);
+ if (!(tmp & ESTAT_RXBUSY))
+ break;
+ }
+ for (;;) {
+ tmp = nolock_regb_read(priv, ECON1);
+ if (!(tmp & ECON1_TXRTS))
+ break;
+ }
+ /* ECON2_VRPS was set during initialization */
+ nolock_reg_bfset(priv, ECON2, ECON2_PWRSV);
+ } else {
+ nolock_reg_bfclr(priv, ECON2, ECON2_PWRSV);
+ for (;;) {
+ tmp = nolock_regb_read(priv, ESTAT);
+ if (tmp & ESTAT_CLKRDY)
+ break;
+ }
+ /* caller sets ECON1_RXEN */
+ }
+ mutex_unlock(&priv->lock);
+}
+
static int enc28j60_hw_init(struct enc28j60_net *priv)
{
u8 reg;
@@ -612,8 +649,8 @@ static int enc28j60_hw_init(struct enc28
priv->tx_retry_count = 0;
priv->max_pk_counter = 0;
priv->rxfilter = RXFILTER_NORMAL;
- /* enable address auto increment */
- nolock_regb_write(priv, ECON2, ECON2_AUTOINC);
+ /* enable address auto increment and voltage regulator powersave */
+ nolock_regb_write(priv, ECON2, ECON2_AUTOINC | ECON2_VRPS);
nolock_rxfifo_init(priv, RXSTART_INIT, RXEND_INIT);
nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
@@ -690,7 +727,9 @@ static int enc28j60_hw_init(struct enc28
static void enc28j60_hw_enable(struct enc28j60_net *priv)
{
- /* enable interrutps */
+ enc28j60_lowpower(priv, false);
+
+ /* enable interrupts */
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME ": %s() enabling interrupts.\n",
__FUNCTION__);
@@ -717,6 +756,8 @@ static void enc28j60_hw_disable(struct e
nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
priv->hw_enable = false;
mutex_unlock(&priv->lock);
+
+ enc28j60_lowpower(priv, true);
}
static int
@@ -734,6 +775,8 @@ enc28j60_setlink(struct net_device *ndev
"hw_reset() failed\n");
ret = -EINVAL;
}
+ enc28j60_lowpower(priv, true);
+
} else {
if (netif_msg_link(priv))
dev_warn(&ndev->dev,
@@ -900,7 +943,7 @@ static void enc28j60_hw_rx(struct net_de
if (RSV_GETBIT(rxstat, RSV_LENCHECKERR))
ndev->stats.rx_frame_errors++;
} else {
- skb = dev_alloc_skb(len);
+ skb = dev_alloc_skb(len + NET_IP_ALIGN);
if (!skb) {
if (netif_msg_rx_err(priv))
dev_err(&ndev->dev,
@@ -908,6 +951,7 @@ static void enc28j60_hw_rx(struct net_de
ndev->stats.rx_dropped++;
} else {
skb->dev = ndev;
+ skb_reserve(skb, NET_IP_ALIGN);
/* copy the packet from the receive buffer */
enc28j60_mem_read(priv, priv->next_pk_ptr + sizeof(rsv),
len, skb_put(skb, len));
@@ -1536,6 +1580,8 @@ static int __devinit enc28j60_probe(stru
dev->watchdog_timeo = TX_TIMEOUT;
SET_ETHTOOL_OPS(dev, &enc28j60_ethtool_ops);
+ enc28j60_lowpower(priv, true);
+
ret = register_netdev(dev);
if (ret) {
if (netif_msg_probe(priv))
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/14 v2] [rndis_host] Add RNDIS physical medium checking into generic_rndis_bind()
On Sunday 27 January 2008, Jussi Kivilinna wrote:
> Add RNDIS physical medium checking into generic_rndis_bind() and also make
> rndis_host to be only bind on every medium except wireless.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
I think that means the whole series is fair to merge into
the network drivers queue, assuming nobody has any issues
with the last patch (adding the actual rndis_wlan support).
Thanks for doing this ... it's nice to see folk successfully
building on top of this rndis_host code, since it started
without any "real" RNDIS devices to test against! (Just the
Linux-USB "g_ether" peripheral code.)
- Dave
> ---
>
> drivers/net/usb/rndis_host.c | 36 +---
> drivers/net/usb/rndis_host.h | 19 ++-
> 2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 800c9d0..0606e11 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -271,7 +271,8 @@ response_error:
> return -EDOM;
> }
>
> -int generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf)
> +int
> +generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
> {
> int retval;
> struct net_device *net = dev->net;
> @@ -287,7 +288,7 @@ int generic_rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> struct rndis_set_c *set_c;
> struct rndis_halt *halt;
> } u;
> - u32 tmp;
> + u32 tmp, *phym;
> int reply_len;
> unsigned char *bp;
>
> @@ -358,6 +359,30 @@ int generic_rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> dev->driver_info->early_init(dev) != 0)
> goto halt_fail_and_release;
>
> + /* Check physical medium */
> + reply_len = sizeof *phym;
> + retval = rndis_query(dev, intf, u.buf, OID_GEN_PHYSICAL_MEDIUM,
> + 0, (void **) &phym, &reply_len);
> + if (retval != 0)
> + /* OID is optional so don't fail here. */
> + *phym = RNDIS_PHYSICAL_MEDIUM_UNSPECIFIED;
> + if ((flags & FLAG_RNDIS_PHYM_WIRELESS) &&
> + *phym != RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN) {
> + if (netif_msg_probe(dev))
> + dev_dbg(&intf->dev, "driver requires wireless "
> + "physical medium, but device is not.\n");
> + retval = -ENODEV;
> + goto halt_fail_and_release;
> + }
> + if ((flags & FLAG_RNDIS_PHYM_NOT_WIRELESS) &&
> + *phym == RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN) {
> + if (netif_msg_probe(dev))
> + dev_dbg(&intf->dev, "driver requires non-wireless "
> + "physical medium, but device is wireless.\n");
> + retval = -ENODEV;
> + goto halt_fail_and_release;
> + }
> +
> /* Get designated host ethernet address */
> reply_len = ETH_ALEN;
> retval = rndis_query(dev, intf, u.buf, OID_802_3_PERMANENT_ADDRESS,
> @@ -403,6 +428,11 @@ fail:
> }
> EXPORT_SYMBOL_GPL(generic_rndis_bind);
>
> +static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + return generic_rndis_bind(dev, intf, FLAG_RNDIS_PHYM_NOT_WIRELESS);
> +}
> +
> void rndis_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
> struct rndis_halt *halt;
> @@ -518,7 +548,7 @@ EXPORT_SYMBOL_GPL(rndis_tx_fixup);
> static const struct driver_info rndis_info = {
> .description = "RNDIS device",
> .flags =FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
> - .bind = generic_rndis_bind,
> + .bind = rndis_bind,
> .unbind = rndis_unbind,
> .status = rndis_status,
> .rx_fixup = rndis_rx_fixup,
> diff --git a/drivers/net/usb/rndis_host.h b/drivers/net/usb/rndis_host.h
> index 61f1fd8..edc1d4a 100644
> --- a/drivers/net/usb/rndis_host.h
> +++ b/drivers/net/usb/rndis_host.h
> @@ -82,6 +82,17 @@ struct rndis_msg_hdr {
> #define RNDIS_STATUS_MEDIA_CONNECT ccpu2(0x4001000b)
> #define RNDIS_STATUS_MEDIA_DISCONNECT ccpu2(0x4001000c)
>
> +/* codes for OID_GEN_PHYSICAL_MEDIUM */
> +#define RNDIS_PHYSICAL_MEDIUM_UNSPECIFIED ccpu2(0x)
> +#define
Re: [PATCH 12/14] [rndis_host] Add RNDIS physical medium checking into generic_rndis_bind()
On Sunday 27 January 2008, Jussi Kivilinna wrote: > > I'm not very familiar with posting patches (as some might have noticed) > so I have some questions.. if you don't mind. Now that you have acked > most of the patches, is it ok for me to add your 'Acked-by' to those > patches? On those patches I've acked, yes. (Unless it's a different version from what I acked...) You may not need to re-send them ... that's kind of a policy choice of the subsystem maintainer (Jeff Garzik in this case). Most maintainers pick up acks from the mailing list, as part of their merge process. > Should I even repost all of these patches as patchset or just > ones that have been fixed? Again, that's a subsystem-specific policy. In most cases I'm familiar with, the answer is to avoid needless reposting ... so after it's acked, at most one repost-with-ack. (And typically not even that, when maintainers pick up the acks.) The main reason to repost an entire patch series is to avoid confusion that creeps in with too many tweaked versions. On the other hand, such reposting creates its own confusion... > Should I post new 'physical medium' patch as > reply to this post and then repost patchset with your ack just to > mailing list? My two cents: just post an updated version of $SUBJECT. If Jeff wants a version with the Acks, he'll tell you (or someone more up on netdev policies will). At this point, assuming you update $SUBJECT patch OK, I think this series is ready for Jeff's attention... - Dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] Move usbnet.h and rndis_host.h to include/linux/usb
On Friday 25 January 2008, Jussi Kivilinna wrote: > Move headers usbnet.h and rndis_host.h to include/linux/usb and fix includes > for drivers/net/usb modules. Headers are moved because rndis_wlan will be > outside drivers/net/usb in drivers/net/wireless and yet need these headers. > > Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]> Acked-by: David Brownell <[EMAIL PROTECTED]> > --- > > drivers/net/usb/asix.c |3 > drivers/net/usb/cdc_ether.c|3 > drivers/net/usb/cdc_subset.c |3 > drivers/net/usb/dm9601.c |3 > drivers/net/usb/gl620a.c |3 > drivers/net/usb/mcs7830.c |3 > drivers/net/usb/net1080.c |3 > drivers/net/usb/plusb.c|3 > drivers/net/usb/rndis_host.c |5 - > drivers/net/usb/rndis_host.h | 274 > > drivers/net/usb/usbnet.c |3 > drivers/net/usb/usbnet.h | 214 --- > drivers/net/usb/zaurus.c |3 > include/linux/usb/rndis_host.h | 274 > > include/linux/usb/usbnet.h | 214 +++ > 15 files changed, 500 insertions(+), 511 deletions(-) > > diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c > index 569028b..6f245cf 100644 > --- a/drivers/net/usb/asix.c > +++ b/drivers/net/usb/asix.c > @@ -33,8 +33,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > #define DRIVER_VERSION "14-Jun-2006" > static const char driver_name [] = "asix"; > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c > index 97c17bb..a934428 100644 > --- a/drivers/net/usb/cdc_ether.c > +++ b/drivers/net/usb/cdc_ether.c > @@ -31,8 +31,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > > #if defined(CONFIG_USB_NET_RNDIS_HOST) || > defined(CONFIG_USB_NET_RNDIS_HOST_MODULE) > diff --git a/drivers/net/usb/cdc_subset.c b/drivers/net/usb/cdc_subset.c > index 943988e..0ec7936 100644 > --- a/drivers/net/usb/cdc_subset.c > +++ b/drivers/net/usb/cdc_subset.c > @@ -26,8 +26,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > > /* > diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c > index 1ffdd10..5a4e40c 100644 > --- a/drivers/net/usb/dm9601.c > +++ b/drivers/net/usb/dm9601.c > @@ -20,8 +20,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > /* datasheet: > > http://www.davicom.com.tw/big5/download/Data%20Sheet/DM9601-DS-P01-930914.pdf > diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c > index 031cf5c..f7ccfad 100644 > --- a/drivers/net/usb/gl620a.c > +++ b/drivers/net/usb/gl620a.c > @@ -29,8 +29,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > > /* > diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c > index 5ea7411..c3d119f 100644 > --- a/drivers/net/usb/mcs7830.c > +++ b/drivers/net/usb/mcs7830.c > @@ -31,8 +31,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > /* requests */ > #define MCS7830_RD_BMREQ (USB_DIR_IN | USB_TYPE_VENDOR | \ > diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c > index 19bf8da..034e8a7 100644 > --- a/drivers/net/usb/net1080.c > +++ b/drivers/net/usb/net1080.c > @@ -28,11 +28,10 @@ > #include > #include > #include > +#include > > #include > > -#include "usbnet.h" > - > > /* > * Netchip 1080 driver ... http://www.netchip.com > diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c > index 4530093..08555f8 100644 > --- a/drivers/net/usb/plusb.c > +++ b/drivers/net/usb/plusb.c > @@ -28,8 +28,7 @@ > #include > #include > #include > - > -#include "usbnet.h" > +#include > > > /* > diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c > index 825ff51..411314e 100644 > --- a/drivers/net/usb/rndis_host.c > +++ b/drivers/net/usb/rndis_host.c > @@ -29,9 +29,8 @@ > #include > #include > #include > - > -#include "usbnet.h" > -#include "rndis_host.h" > +#include > +#include > > > /* > diff --git a/drivers/net/usb/rndis_host.h b/drivers/net/usb/rndis_host.h > deleted file mode 100644 > index edc1d4a..000 > --- a/drivers/net/usb/rndis_host
Re: [PATCH 11/14] [rndis_host] Add link_change function pointer to 'struct rndis_data'.
On Friday 25 January 2008, Jussi Kivilinna wrote:
> Callback to signal link state changes from minidriver to
> 'subminidrivers'.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 24
> drivers/net/usb/usbnet.h |4
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 0813903..800c9d0 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -148,10 +148,26 @@ int rndis_command(struct usbnet *dev, struct
> rndis_msg_hdr *buf)
> request_id, xid);
> /* then likely retry */
> } else switch (buf->msg_type) {
> - case RNDIS_MSG_INDICATE: { /* fault */
> - // struct rndis_indicate *msg = (void *)buf;
> - dev_info(&info->control->dev,
> - "rndis fault indication\n");
> + case RNDIS_MSG_INDICATE: { /* fault/event */
> + struct rndis_indicate *msg = (void *)buf;
> + int state = 0;
> +
> + switch (msg->status) {
> + case RNDIS_STATUS_MEDIA_CONNECT:
> + state = 1;
> + case RNDIS_STATUS_MEDIA_DISCONNECT:
> + dev_info(&info->control->dev,
> + "rndis media %sconnect\n",
> + !state?"dis":"");
> + if (dev->driver_info->link_change)
> + dev->driver_info->link_change(
> + dev, state);
> + break;
> + default:
> + dev_info(&info->control->dev,
> + "rndis indication: 0x%08x\n",
> + le32_to_cpu(msg->status));
> + }
> }
> break;
> case RNDIS_MSG_KEEPALIVE: { /* ping */
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 25b63d3..e0501da 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -121,6 +121,10 @@ struct driver_info {
>* right after minidriver have initialized hardware. */
> int (*early_init)(struct usbnet *dev);
>
> + /* called by minidriver when link state changes, state: 0=disconnect,
> + * 1=connect */
> + void(*link_change)(struct usbnet *dev, int state);
> +
> /* for new devices, use the descriptor-reading code instead */
> int in; /* rx endpoint */
> int out;/* tx endpoint */
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/14] [rndis_host] Add RNDIS physical medium checking into generic_rndis_bind()
On Friday 25 January 2008, Jussi Kivilinna wrote:
> + if(flags & FLAG_RNDIS_PHYM_WIRELESS &&
> + *phym != RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN) {
> + dev_err(&intf->dev, "driver requires wireless physical "
> + "medium, but device is not.\n");
> + retval = -ENODEV;
> + goto halt_fail_and_release;
> + }
> + if(flags & FLAG_RNDIS_PHYM_NOT_WIRELESS &&
> + *phym == RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN) {
> + dev_err(&intf->dev, "driver requires non-wireless physical "
> + "medium, but device is wireless.\n");
> + retval = -ENODEV;
Well, other than the obvious checkpatch.pl warnings waiting to trigger
("if" is not a function; put a space before the paren) and what I'd call
missing parens around the "flags & ...", those are *not* errors. No
wonder you thought this would cause too many messages!!
Just make those be dev_dbg() calls instead. The strongest message level
you can argue for there would be KERN_NOTICE, "normal but significant";
except it's not especially significant. Filtering by netif_msg_probe()
may be a good idea too; that's normally enabled in this framework.
- Dave
p.s. Before these get submitted, *all* of them need to pass "checkpatch.pl".
Ideally, "checkpatch.pl --strict" ...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/14] [rndis_host] Add early_init function pointer to 'struct rndis_data'.
On Friday 25 January 2008, Jussi Kivilinna wrote:
> Function pointer is for 'subminidrivers' that need to do work on device
> right after minidriver has initialized hardware.
>
> For example, rndis_wlan setting device specific configuration parameters
> with OID_GEN_RNDIS_CONFIG_PARAMETER right after rndis_host has
> initialized hardware with RNDIS_INIT.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
... though I'm not sure I'd coin a term like "subminidriver". ;)
> ---
>
> drivers/net/usb/rndis_host.c |6 ++
> drivers/net/usb/usbnet.h |5 +
> 2 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 1d6bf0a..0813903 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -336,6 +336,12 @@ int generic_rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> dev->hard_mtu, tmp, dev->rx_urb_size,
> 1 << le32_to_cpu(u.init_c->packet_alignment));
>
> + /* module has some device initialization code needs to be done right
> + * after RNDIS_INIT */
> + if (dev->driver_info->early_init &&
> + dev->driver_info->early_init(dev) != 0)
> + goto halt_fail_and_release;
> +
> /* Get designated host ethernet address */
> reply_len = ETH_ALEN;
> retval = rndis_query(dev, intf, u.buf, OID_802_3_PERMANENT_ADDRESS,
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 0b4bf09..25b63d3 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -116,6 +116,11 @@ struct driver_info {
> struct sk_buff *(*tx_fixup)(struct usbnet *dev,
> struct sk_buff *skb, gfp_t flags);
>
> + /* early initialization code, can sleep. This is for minidrivers
> + * having 'subminidrivers' that need to do extra initialization
> + * right after minidriver have initialized hardware. */
> + int (*early_init)(struct usbnet *dev);
> +
> /* for new devices, use the descriptor-reading code instead */
> int in; /* rx endpoint */
> int out;/* tx endpoint */
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14][v3]: Driver for Wireless RNDIS USB devices.
On Friday 25 January 2008, Jussi Kivilinna wrote: > Hello, > > Here is the third try on wireless RNDIS patchset. > > Patches 1-9 are from second patchset (with name change rndis_wext -> > rndis_wlan > in comments where needed): > 1. Fix sparse warning: returning void valued expression > 2. [cdc_ether] Hardwire CDC descriptors when missing > 3. [rndis_host] Use 1KB buffer in rndis_unbind > 4. [rndis_host] Halt device if rndis_bind fails > 5. [rndis_host] Fix rndis packet filter flags > 6. [usbnet] Use wlan device name for RNDIS wireless devices > 7. [rndis_host] Split up rndis_host.c > 8. [rndis_host] export functions > 9. [usbnet] add driver_priv pointer to 'struct usbnet' If they're the same, my ack (as maintainer for that infrastructure and, for now, rndis_host) still stands. I won't look at them again. > Changed patches 10-14: > 10. [rndis_host] Add early_init function pointer to 'struct rndis_data'. > 11. [rndis_host] Add link_change function pointer to 'struct rndis_data'. > 12. [rndis_host] Add RNDIS physical medium checking into generic_rndis_bind() > 13. Move usbnet.h and rndis_host.h to include/linux/usb > 14. Add new driver 'rndis_wlan' for wireless RNDIS devices. > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] RFC: Driver for Wireless RNDIS USB devices.
On Friday 25 January 2008, Jussi Kivilinna wrote: > On Thu, 2008-01-24 at 17:19 -0800, David Brownell wrote: > > > 13. [rndis_host] blacklist known wireless RNDIS devices > > > > That will be a headache over time though ... can't you just > > let the probe succeed enough to recogize it's wireless (using > > the media flag) and then bail, so the next driver can try? > > Sure, that works too (but causes a little bit more message flood). Excess messages can be dealt with, or at worst, ignored. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/14] [rndis_host] Halt device if rndis_bind fails.
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> When bind fails after device was initialized, shutdown device properly
> by sending RNDIS_MSG_HALT.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 12 +---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 42b161c..c686025 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -467,6 +467,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> struct rndis_query_c*get_c;
> struct rndis_set*set;
> struct rndis_set_c *set_c;
> + struct rndis_halt *halt;
> } u;
> u32 tmp;
> int reply_len;
> @@ -517,7 +518,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> "dev can't take %u byte packets (max %u)\n",
> dev->hard_mtu, tmp);
> retval = -EINVAL;
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
> dev->hard_mtu = tmp;
> net->mtu = dev->hard_mtu - net->hard_header_len;
> @@ -539,7 +540,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> 48, (void **) &bp, &reply_len);
> if (unlikely(retval< 0)) {
> dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval);
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
> memcpy(net->dev_addr, bp, ETH_ALEN);
>
> @@ -555,7 +556,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> retval = rndis_command(dev, u.header);
> if (unlikely(retval < 0)) {
> dev_err(&intf->dev, "rndis set packet filter, %d\n", retval);
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
>
> retval = 0;
> @@ -563,6 +564,11 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> kfree(u.buf);
> return retval;
>
> +halt_fail_and_release:
> + memset(u.halt, 0, sizeof *u.halt);
> + u.halt->msg_type = RNDIS_MSG_HALT;
> + u.halt->msg_len = ccpu2(sizeof *u.halt);
> + (void) rndis_command(dev, (void *)u.halt);
> fail_and_release:
> usb_set_intfdata(info->data, NULL);
> usb_driver_release_interface(driver_of(intf), info->data);
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] RFC: Driver for Wireless RNDIS USB devices.
On Saturday 19 January 2008, Jussi Kivilinna wrote: > Hello, > > This is second try on wireless RNDIS patchset started by Bjorge Dijkstra. > Since > Bjorge has disappeared, I claim maintainership of rndis_wext and this patchset > until he returns. > > This patchset adds support for various 802.11 USB devices based on Broadcom > 4320 chip. Chip uses RNDIS to communicate with the host, so module depend > heavily on rndis_host/cdc_ether/usbnet and needs some changes on these > modules in order to work. > > Patches 1-6 are from first patchset: > 1. Fix sparse warning: returning void valued expression > 2. [cdc_ether] Hardwire CDC descriptors when missing > 3. [rndis_host] Use 1KB buffer in rndis_unbind > 4. [rndis_host] Halt device if rndis_bind fails > 5. [rndis_host] Fix rndis packet filter flags > 6. [usbnet] Use wlan device name for RNDIS wireless devices > > Of these 1, 3 and 4 are not required for this version of rndis_wext to work. > > Actual wireless part is changed from extension on rndis_host to separate > driver. Different devices are detected by device specific USB vendor/product > IDs as the way done with Windows drivers instead of detecting RNDIS media > type > like in first patchset. > > New patches 7-14: > 7. [rndis_host] Split up rndis_host.c > 8. [rndis_host] export functions > 9. [usbnet] add driver_priv pointer to 'struct usbnet' So far as I'm concerned patches 1-9 can go in any time. The other patches I won't ack yet; see below. > 10. [rndis_host] Add rndis_early_init function pointer to 'struct rndis_data'. > 11. [rndis_host] Add rndis_link_change function pointer to 'struct > rndis_data'. Those aren't added to "struct rndis_data" ... they're added to the struct at the core of the usbnet framework. So they should not be RNDIS-specific ... even though the only current user will be the RNDIS host code. Rename those methods and I'll be happy. > 12. Move usbnet.h and rndis_host.h to include/linux/usb No problem with that, except that fixing #10 and #11 will break them. > 13. [rndis_host] blacklist known wireless RNDIS devices That will be a headache over time though ... can't you just let the probe succeed enough to recogize it's wireless (using the media flag) and then bail, so the next driver can try? > 14. Add new driver 'rndis_wext' for wireless RNDIS devices. The real goods! :) > Patches should be applied in order, series apply cleanly to 2.6.24-rc8. > > - Jussi Kivilinna > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] [rndis_host] blacklist known wireless RNDIS devices
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Blacklist known wireless RNDIS devices that will be handled by
> rndis_wext module.
This seems destined to become a headache. Wouldn't it be better
to let the probe progress far enough to detect that it's actually
a WLAN device, and then back out so the next driver can try?
- Dave
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 104
> ++
> 1 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 1ffbbb3..cd1f953 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -530,6 +530,110 @@ static const struct driver_info rndis_info = {
> /*-*/
>
> static const struct usb_device_idproducts [] = {
> +/*
> + * BLACKLIST !!
> + *
> + * Blacklist RNDIS devices that are handled in separate RNDIS modules.
> + */
> +
> +#define RNDIS_MASTER_INTERFACE \
> + .bInterfaceClass= USB_CLASS_COMM, \
> + .bInterfaceSubClass = 2 /* ACM */, \
> + .bInterfaceProtocol = 0x0ff
> +
> +/* Wireless RNDIS devices, rndis_wext module */
> +{
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0411,
> + .idProduct = 0x00bc, /* Buffalo WLI-U2-KG125S */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0baf,
> + .idProduct = 0x011b, /* U.S. Robotics USR5421 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x050d,
> + .idProduct = 0x011b, /* Belkin F5D7051 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x1799, /* Belkin has two vendor ids */
> + .idProduct = 0x011b, /* Belkin F5D7051 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x13b1,
> + .idProduct = 0x0014, /* Linksys WUSB54GSv2 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x13b1,
> + .idProduct = 0x0026, /* Linksys WUSB54GSC */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0b05,
> + .idProduct = 0x1717, /* Asus WL169gE */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0a5c,
> + .idProduct = 0xd11b, /* Eminent EM4045 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x1690,
> + .idProduct = 0x0715, /* BT Voyager 1055 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x13b1,
> + .idProduct = 0x000e, /* Linksys WUSB54GSv1 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0baf,
> + .idProduct = 0x0111, /* U.S. Robotics USR5420 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +}, {
> + .match_flags= USB_DEVICE_ID_MATCH_INT_INFO
> + | USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x0411,
> + .idProduct = 0x004b, /* BUFFALO WLI-USB-G54 */
> + RNDIS_MASTER_INTERFACE,
> + .driver_info= 0,
> +},
> +
> +/*
> + * WHI
Re: [PATCH 07/14] [rndis_host] Split up rndis_host.c
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Split up rndis_host.c into rndis_host.h and rndis_base.c. This is done so
> that rndis_wext can reuse common parts with rndis_host.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 223 --
> drivers/net/usb/rndis_host.h | 248
> ++
> 2 files changed, 249 insertions(+), 222 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 12daf9c..29d7e3b 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -31,6 +31,7 @@
> #include
>
> #include "usbnet.h"
> +#include "rndis_host.h"
>
>
> /*
> @@ -56,228 +57,6 @@
> */
>
> /*
> - * CONTROL uses CDC "encapsulated commands" with funky notifications.
> - * - control-out: SEND_ENCAPSULATED
> - * - interrupt-in: RESPONSE_AVAILABLE
> - * - control-in: GET_ENCAPSULATED
> - *
> - * We'll try to ignore the RESPONSE_AVAILABLE notifications.
> - *
> - * REVISIT some RNDIS implementations seem to have curious issues still
> - * to be resolved.
> - */
> -struct rndis_msg_hdr {
> - __le32 msg_type; /* RNDIS_MSG_* */
> - __le32 msg_len;
> - // followed by data that varies between messages
> - __le32 request_id;
> - __le32 status;
> - // ... and more
> -} __attribute__ ((packed));
> -
> -/* MS-Windows uses this strange size, but RNDIS spec says 1024 minimum */
> -#define CONTROL_BUFFER_SIZE 1025
> -
> -/* RNDIS defines an (absurdly huge) 10 second control timeout,
> - * but ActiveSync seems to use a more usual 5 second timeout
> - * (which matches the USB 2.0 spec).
> - */
> -#define RNDIS_CONTROL_TIMEOUT_MS(5 * 1000)
> -
> -
> -#define ccpu2 __constant_cpu_to_le32
> -
> -#define RNDIS_MSG_COMPLETION ccpu2(0x8000)
> -
> -/* codes for "msg_type" field of rndis messages;
> - * only the data channel uses packet messages (maybe batched);
> - * everything else goes on the control channel.
> - */
> -#define RNDIS_MSG_PACKET ccpu2(0x0001) /* 1-N packets */
> -#define RNDIS_MSG_INIT ccpu2(0x0002)
> -#define RNDIS_MSG_INIT_C (RNDIS_MSG_INIT|RNDIS_MSG_COMPLETION)
> -#define RNDIS_MSG_HALT ccpu2(0x0003)
> -#define RNDIS_MSG_QUERY ccpu2(0x0004)
> -#define RNDIS_MSG_QUERY_C(RNDIS_MSG_QUERY|RNDIS_MSG_COMPLETION)
> -#define RNDIS_MSG_SETccpu2(0x0005)
> -#define RNDIS_MSG_SET_C (RNDIS_MSG_SET|RNDIS_MSG_COMPLETION)
> -#define RNDIS_MSG_RESET ccpu2(0x0006)
> -#define RNDIS_MSG_RESET_C(RNDIS_MSG_RESET|RNDIS_MSG_COMPLETION)
> -#define RNDIS_MSG_INDICATE ccpu2(0x0007)
> -#define RNDIS_MSG_KEEPALIVE ccpu2(0x0008)
> -#define RNDIS_MSG_KEEPALIVE_C
> (RNDIS_MSG_KEEPALIVE|RNDIS_MSG_COMPLETION)
> -
> -/* codes for "status" field of completion messages */
> -#define RNDIS_STATUS_SUCCESSccpu2(0x)
> -#define RNDIS_STATUS_FAILUREccpu2(0xc001)
> -#define RNDIS_STATUS_INVALID_DATA ccpu2(0xc0010015)
> -#define RNDIS_STATUS_NOT_SUPPORTED ccpu2(0xc0bb)
> -#define RNDIS_STATUS_MEDIA_CONNECT ccpu2(0x4001000b)
> -#define RNDIS_STATUS_MEDIA_DISCONNECT ccpu2(0x4001000c)
> -
> -
> -struct rndis_data_hdr {
> - __le32 msg_type; /* RNDIS_MSG_PACKET */
> - __le32 msg_len;// rndis_data_hdr + data_len + pad
> - __le32 data_offset;// 36 -- right after header
> - __le32 data_len; // ... real packet size
> -
> - __le32 oob_data_offset;// zero
> - __le32 oob_data_len; // zero
> - __le32 num_oob;// zero
> - __le32 packet_data_offset; // zero
> -
> - __le32 packet_data_len;// zero
> - __le32 vc_handle; // zero
> - __le32 reserved; // zero
> -} __attribute__ ((packed));
> -
> -struct rndis_init { /* OUT */
> - // header and:
> - __le32 msg_type; /* RNDIS_MSG_INIT */
> - __le32 msg_len;// 24
> - __le32 request_id;
> - __le32 major_version; // of rndis (1.0)
> - __le32 minor_version;
> - __le32 max_transfer_size;
> -} __attribute__ ((packed));
> -
> -struct rndis_init_c {/* IN */
> -
Re: [PATCH 10/14] [rndis_host] Add rndis_early_init function pointer to 'struct rndis_data'.
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Function pointer is for rndis minidrivers that need to do work on device right
> after RNDIS_INIT. For example setting device specific configuration parameters
> with OID_GEN_RNDIS_CONFIG_PARAMETER.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Could this -- and #11/14 -- instead be generalized a bit,
so they're not RNDIS-specific? At least in name; the
only user for now would be the rndis_host code.
The generalization would presumably be "early_init" and
"link_change", paired with doc comments reflecting that
they're usable by any driver stack built over the usbnet
framework core.
There's no point IMO to having generalizable hooks be
restricted this way.
- Dave
> ---
>
> drivers/net/usb/rndis_host.c |6 ++
> drivers/net/usb/usbnet.h |3 +++
> 2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 1d6bf0a..22e5ca1 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -336,6 +336,12 @@ int generic_rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> dev->hard_mtu, tmp, dev->rx_urb_size,
> 1 << le32_to_cpu(u.init_c->packet_alignment));
>
> + /* module has some device initialization code needs to be done right
> + * after RNDIS_INIT */
> + if (dev->driver_info->rndis_early_init &&
> + dev->driver_info->rndis_early_init(dev) != 0)
> + goto halt_fail_and_release;
> +
> /* Get designated host ethernet address */
> reply_len = ETH_ALEN;
> retval = rndis_query(dev, intf, u.buf, OID_802_3_PERMANENT_ADDRESS,
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 0b4bf09..2bc5f76 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -116,6 +116,9 @@ struct driver_info {
> struct sk_buff *(*tx_fixup)(struct usbnet *dev,
> struct sk_buff *skb, gfp_t flags);
>
> + /* rndis minidriver early initialization code, can sleep */
> + int (*rndis_early_init)(struct usbnet *dev);
> +
> /* for new devices, use the descriptor-reading code instead */
> int in; /* rx endpoint */
> int out;/* tx endpoint */
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/14] [usbnet] add driver_priv pointer to 'struct usbnet'
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Add a private data pointer to usbnet for rndis_wext module to use.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/usbnet.h |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 29ab92e..0b4bf09 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -31,6 +31,7 @@ struct usbnet {
> struct usb_interface*intf;
> struct driver_info *driver_info;
> const char *driver_name;
> + void*driver_priv;
> wait_queue_head_t *wait;
> struct mutexphy_mutex;
> unsigned char suspend_count;
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/14] [rndis_host] export functions
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Export rndis_host functions and also rename rndis_bind() to
> generic_rndis_bind() for modules using rndis_host as base.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 20 +---
> drivers/net/usb/rndis_host.h |9 +
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 29d7e3b..1d6bf0a 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -60,13 +60,14 @@
> * RNDIS notifications from device: command completion; "reverse"
> * keepalives; etc
> */
> -static void rndis_status(struct usbnet *dev, struct urb *urb)
> +void rndis_status(struct usbnet *dev, struct urb *urb)
> {
> devdbg(dev, "rndis status urb, len %d stat %d",
> urb->actual_length, urb->status);
> // FIXME for keepalives, respond immediately (asynchronously)
> // if not an RNDIS status, do like cdc_status(dev,urb) does
> }
> +EXPORT_SYMBOL_GPL(rndis_status);
>
> /*
> * RPC done RNDIS-style. Caller guarantees:
> @@ -78,7 +79,7 @@ static void rndis_status(struct usbnet *dev, struct urb
> *urb)
> * Call context is likely probe(), before interface name is known,
> * which is why we won't try to use it in the diagnostics.
> */
> -static int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf)
> +int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf)
> {
> struct cdc_state*info = (void *) &dev->data;
> int master_ifnum;
> @@ -187,6 +188,7 @@ static int rndis_command(struct usbnet *dev, struct
> rndis_msg_hdr *buf)
> dev_dbg(&info->control->dev, "rndis response timeout\n");
> return -ETIMEDOUT;
> }
> +EXPORT_SYMBOL_GPL(rndis_command);
>
> /*
> * rndis_query:
> @@ -253,7 +255,7 @@ response_error:
> return -EDOM;
> }
>
> -static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
> +int generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf)
> {
> int retval;
> struct net_device *net = dev->net;
> @@ -377,8 +379,9 @@ fail:
> kfree(u.buf);
> return retval;
> }
> +EXPORT_SYMBOL_GPL(generic_rndis_bind);
>
> -static void rndis_unbind(struct usbnet *dev, struct usb_interface *intf)
> +void rndis_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
> struct rndis_halt *halt;
>
> @@ -393,11 +396,12 @@ static void rndis_unbind(struct usbnet *dev, struct
> usb_interface *intf)
>
> usbnet_cdc_unbind(dev, intf);
> }
> +EXPORT_SYMBOL_GPL(rndis_unbind);
>
> /*
> * DATA -- host must not write zlps
> */
> -static int rndis_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> +int rndis_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> {
> /* peripheral may have batched packets to us... */
> while (likely(skb->len)) {
> @@ -439,8 +443,9 @@ static int rndis_rx_fixup(struct usbnet *dev, struct
> sk_buff *skb)
> /* caller will usbnet_skb_return the remaining packet */
> return 1;
> }
> +EXPORT_SYMBOL_GPL(rndis_rx_fixup);
>
> -static struct sk_buff *
> +struct sk_buff *
> rndis_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
> {
> struct rndis_data_hdr *hdr;
> @@ -485,12 +490,13 @@ fill:
> /* FIXME make the last packet always be short ... */
> return skb;
> }
> +EXPORT_SYMBOL_GPL(rndis_tx_fixup);
>
>
> static const struct driver_info rndis_info = {
> .description = "RNDIS device",
> .flags =FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
> - .bind = rndis_bind,
> + .bind = generic_rndis_bind,
> .unbind = rndis_unbind,
> .status = rndis_status,
> .rx_fixup = rndis_rx_fixup,
> diff --git a/drivers/net/usb/rndis_host.h b/drivers/net/usb/rndis_host.h
> index 1386a17..61f1fd8 100644
> --- a/drivers/net/usb/rndis_host.h
> +++ b/drivers/net/usb/rndis_host.h
> @@ -244,5 +244,14 @@ struct rndis_keepalive_c { /* IN (optionally OUT)
> */
> RNDIS_PACKET_TYPE_ALL_MULTICAST | \
> RNDIS_PACKET_TYPE_PROMISCUOUS)
>
> +
> +extern void rndis_status(struct usbnet *dev, struct urb *urb);
> +extern int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf);
> +extern int generic_rndis_bind(struct usbnet *dev, struct usb_inte
Re: [PATCH 06/14] [usbnet] Use wlan device name for RNDIS wireless devices
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> Use wlan device name for RNDIS wireless devices.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/usbnet.c |3 +++
> drivers/net/usb/usbnet.h |2 ++
> 2 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 8ed1fc5..a2a2d5e 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1204,6 +1204,9 @@ usbnet_probe (struct usb_interface *udev, const struct
> usb_device_id *prod)
> if ((dev->driver_info->flags & FLAG_ETHER) != 0
> && (net->dev_addr [0] & 0x02) == 0)
> strcpy (net->name, "eth%d");
> + /* WLAN devices should always be named "wlan%d" */
> + if ((dev->driver_info->flags & FLAG_WLAN) != 0)
> + strcpy(net->name, "wlan%d");
>
> /* maybe the remote can't receive an Ethernet MTU */
> if (net->mtu > (dev->hard_mtu - net->hard_header_len))
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 1fae434..29ab92e 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -87,6 +87,8 @@ struct driver_info {
> #define FLAG_ETHER 0x0020 /* maybe use "eth%d" names */
>
> #define FLAG_FRAMING_AX 0x0040 /* AX88772/178 packets */
> +#define FLAG_WLAN0x0080 /* use "wlan%d" names */
> +
>
> /* init device ... can sleep, or cause probe() failure */
> int (*bind)(struct usbnet *, struct usb_interface *);
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/14] [rndis_host] Fix rndis packet filter flags.
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> RNDIS packet filter flags are not exactly the same as CDC flags
> so we cannot reuse them.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c | 23 ++-
> 1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index c686025..12daf9c 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -256,6 +256,27 @@ struct rndis_keepalive_c { /* IN (optionally OUT)
> */
> #define OID_GEN_MAXIMUM_FRAME_SIZE ccpu2(0x00010106)
> #define OID_GEN_CURRENT_PACKET_FILTERccpu2(0x0001010e)
>
> +/* packet filter bits used by OID_GEN_CURRENT_PACKET_FILTER */
> +#define RNDIS_PACKET_TYPE_DIRECTED ccpu2(0x0001)
> +#define RNDIS_PACKET_TYPE_MULTICAST ccpu2(0x0002)
> +#define RNDIS_PACKET_TYPE_ALL_MULTICAST ccpu2(0x0004)
> +#define RNDIS_PACKET_TYPE_BROADCAST ccpu2(0x0008)
> +#define RNDIS_PACKET_TYPE_SOURCE_ROUTING ccpu2(0x0010)
> +#define RNDIS_PACKET_TYPE_PROMISCUOUSccpu2(0x0020)
> +#define RNDIS_PACKET_TYPE_SMTccpu2(0x0040)
> +#define RNDIS_PACKET_TYPE_ALL_LOCAL ccpu2(0x0080)
> +#define RNDIS_PACKET_TYPE_GROUP ccpu2(0x1000)
> +#define RNDIS_PACKET_TYPE_ALL_FUNCTIONAL ccpu2(0x2000)
> +#define RNDIS_PACKET_TYPE_FUNCTIONAL ccpu2(0x4000)
> +#define RNDIS_PACKET_TYPE_MAC_FRAME ccpu2(0x8000)
> +
> +/* default filter used with RNDIS devices */
> +#define RNDIS_DEFAULT_FILTER ( \
> + RNDIS_PACKET_TYPE_DIRECTED | \
> + RNDIS_PACKET_TYPE_BROADCAST | \
> + RNDIS_PACKET_TYPE_ALL_MULTICAST | \
> + RNDIS_PACKET_TYPE_PROMISCUOUS)
> +
> /*
> * RNDIS notifications from device: command completion; "reverse"
> * keepalives; etc
> @@ -551,7 +572,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> u.set->oid = OID_GEN_CURRENT_PACKET_FILTER;
> u.set->len = ccpu2(4);
> u.set->offset = ccpu2((sizeof *u.set) - 8);
> - *(__le32 *)(u.buf + sizeof *u.set) = ccpu2(DEFAULT_FILTER);
> + *(__le32 *)(u.buf + sizeof *u.set) = RNDIS_DEFAULT_FILTER;
>
> retval = rndis_command(dev, u.header);
> if (unlikely(retval < 0)) {
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/14] [rndis_host] Use 1KB buffer in rndis_unbind
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> rndis_command requires the caller to pass in a buffer of at least 1KB.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/rndis_host.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 96ef6a9..42b161c 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -577,7 +577,7 @@ static void rndis_unbind(struct usbnet *dev, struct
> usb_interface *intf)
> struct rndis_halt *halt;
>
> /* try to clear any rndis state/activity (no i/o from stack!) */
> - halt = kzalloc(sizeof *halt, GFP_KERNEL);
> + halt = kzalloc(CONTROL_BUFFER_SIZE, GFP_KERNEL);
> if (halt) {
> halt->msg_type = RNDIS_MSG_HALT;
> halt->msg_len = ccpu2(sizeof *halt);
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/14] [cdc_ether] Hardwire CDC descriptors when missing
On Saturday 19 January 2008, Jussi Kivilinna wrote:
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
>
> Just as ActiveSync devices, some regular RNDIS devices also lack
> the CDC descriptors (e.g. devices based on BCM4320 WLAN chip).
> This patch hardwires the CDC descriptors for all RNDIS style devices
> when they are missing.
>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
>
> drivers/net/usb/cdc_ether.c | 10 +-
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index a42acc3..97c17bb 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -228,15 +228,16 @@ next_desc:
> buf += buf [0];
> }
>
> - /* Microsoft ActiveSync based RNDIS devices lack the CDC descriptors,
> - * so we'll hard-wire the interfaces and not check for descriptors.
> + /* Microsoft ActiveSync based and some regular RNDIS devices lack the
> + * CDC descriptors, so we'll hard-wire the interfaces and not check
> + * for descriptors.
>*/
> - if (is_activesync(&intf->cur_altsetting->desc) && !info->u) {
> + if (rndis && !info->u) {
> info->control = usb_ifnum_to_if(dev->udev, 0);
> info->data = usb_ifnum_to_if(dev->udev, 1);
> if (!info->control || !info->data) {
> dev_dbg(&intf->dev,
> - "activesync: master #0/%p slave #1/%p\n",
> + "rndis: master #0/%p slave #1/%p\n",
> info->control,
> info->data);
> goto bad_desc;
> @@ -316,7 +317,6 @@ void usbnet_cdc_unbind(struct usbnet *dev, struct
> usb_interface *intf)
> }
> EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
>
> -
> /*-
> *
> * Communications Device Class, Ethernet Control model
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] Fix sparse warning: returning void-valued expression
On Saturday 19 January 2008, Jussi Kivilinna wrote: > From: Bjorge Dijkstra <[EMAIL PROTECTED]> > > rndis_unbind and usbnet_cdc_unbind don't return anything. > > Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]> > Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]> Acked-by: David Brownell <[EMAIL PROTECTED]> > --- > > drivers/net/usb/rndis_host.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c > index 1ebe325..96ef6a9 100644 > --- a/drivers/net/usb/rndis_host.c > +++ b/drivers/net/usb/rndis_host.c > @@ -585,7 +585,7 @@ static void rndis_unbind(struct usbnet *dev, struct > usb_interface *intf) > kfree(halt); > } > > - return usbnet_cdc_unbind(dev, intf); > + usbnet_cdc_unbind(dev, intf); > } > > /* > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] [PATCH] Split up rndis_host.c
On Tuesday 08 January 2008, Johannes Berg wrote: > > > I see that the rndis_wext.c Kconfig won't kick in unless the > > 802.11 stuff is available ... what additional dependencies > > does that imply for a fatter rndis_host module? > > No extra modules are currently required for just plain wext [1]. In the > future, we hope to migrate stuff to cfg80211 though, which is a separate > module. Hmmm. Well, go the current route then ... but come up with a good plan to split it up later, so that connecting a typical non-wireless RNDIS device won't incur such needless extra modules. - Dave > johannes > > [1] mostly because all the code is simply compiled into the kernel... > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] [PATCH] Split up rndis_host.c
On Wednesday 02 January 2008, Jussi Kivilinna wrote: > Hello, > > Bjorge was not comfortable with double probing rndis_wext requires, > wireless RNDIS devices have same device id as the rest of RNDIS. I should have known Microsoft wouldn't start by assuming systems software module boundaries should be natural and obvious ones. :( I see that the rndis_wext.c Kconfig won't kick in unless the 802.11 stuff is available ... what additional dependencies does that imply for a fatter rndis_host module? If it doesn't cause extra modules to be loaded (even for non-WLAN devices), then I suppose I could just live with the RNDIS module being bigger than it needs to be. It was pretty thin to start with! What would bother me would be seeing that systems with this WLAN support option configured could no longer load the basic RNDIS code without also loading a bunch of extra modules that weren't needed for all RNDIS devices. > Our > module version checks OID_GEN_PHYSICAL_MEDIUM in generic_rndis_bind, > with rndis_host bind fails if OID is supported and wireless media type > is returned, with rndis_wext if OID isn't supported or type isn't > wireless. Should this be ok? Sounds like if you can go the different-modules route, then rndis_bind() should maybe accept a parameter giving it that option. Then rndis_wlan_bind(...) == rndis_bind(..., check_wlan_media) rndis_generic_bind(...) == rndis_bind(..., non_wlan_media) That would be getting pretty complicated, since as you say the same USB-level binding (hence hotplugging) would need to kick in ... but having different modules for different RNDIS flavors would need a new hotplug mechanism keying on media type. Ugh. > Should separate rndis_wext be located in drivers/net/wireless instead of > drivers/net/usb? If that doesn't complicate maintaining that software, I can't see why it shouldn't go there. It might imply moving the usbnet interface declarations to someplace linux/include though. (And that presumes there's actually a sane way to split that stuff into its own module, too...) - Dave > - Jussi Kivilinna > > On Sat, 2007-12-22 at 17:17 -0800, David Brownell wrote: > > > From: Bjorge Dijkstra <[EMAIL PROTECTED]> > > > Subject: [PATCH 6/8] [PATCH] Split up rndis_host.c > > > Date: Sat, 22 Dec 2007 22:51:32 +0100 > > > > > > Split up rndis_host.c into rndis_host.h and rndis_base.c and > > > change Makefile accordingly. This is done so we can add extra > > > source files to the rndis_host module later on. > > > > I'm fine with splitting out a header file and the EXPORT_SYMBOL_GPL. > > But why not just have a separate "rndis_wext" module? > > > > > > > --- > > > drivers/net/usb/Makefile |1 + > > > drivers/net/usb/rndis_base.c | 548 ++ > > > drivers/net/usb/rndis_host.c | 763 > > > -- > > > drivers/net/usb/rndis_host.h | 256 ++ > > > 4 files changed, 805 insertions(+), 763 deletions(-) > > > create mode 100644 drivers/net/usb/rndis_base.c > > > delete mode 100644 drivers/net/usb/rndis_host.c > > > create mode 100644 drivers/net/usb/rndis_host.h > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb regression] Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
On Wednesday 02 January 2008, Alan Stern wrote: > BTW, I don't recall ever seeing Tony's patch announced on > linux-usb or linux-usb-devel. Did I simply miss it? I think he didn't post it. I got some questions from him at one point, which I answered, but as I recall he decided for some reason to stop that work. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/8] RFC: Wireless Extensions for rndis_host
> From: Bjorge Dijkstra <[EMAIL PROTECTED]> > Subject: [PATCH 0/8] RFC: Wireless Extensions for rndis_host > Date: Sat, 22 Dec 2007 22:51:26 +0100 > > Hello all, > > I have here a patchset that needs some review. > This patchset adds wireless extensions for rndis_host to > enable support for RNDIS based USB wireless LAN adapters > (e.g. Linksys WUSB54GS, Belkin F05D7051). Cool! We always like to see support for more USB peripherals. Even (especially?) if they'e previously been Windows-only, penguin-deprived hardware. ;) Is all that stuff adequately documented? What I recall from working with RNDIS before is that docs were lacking in some critical details, and were sometimes wrong. > In this patchset: > 1. Fix sparse warning: returning void valued expression > 2. Hardwire CDC descriptors when missing > 3. Use 1KB buffer in rndis_unbind > 4. Halt device if rndis_bind fails > 5. Fix rndis packet filter flags > 6. Split up rndis_host.c > 7. Add Wireless Extensions for rndis_host > 8. Use wlan device name for RNDIS wireless devices > > The first five patches are more generic fixes that I think > can be applied as they are. Of these, patch 2 is required > to get these wireless devices working. I had no problem with those first five. I take it none of those should be prioritized for 2.6.24 integration? > Patches 6-8 introduce some larger changes that may need a > closer look. And you saw my feedback on those. Minimally, please split out the usbnet core extensions, instead of combining them with rndis_wext patches ... and on first princples I'd think rndis_wext should live in its own module. I just skimmed patch #7. There are people far more aware of the issues with Linux wireless drivers than me. :) - Dave > All these patches should be applied in order. > The entire series should apply cleanly to current linux > kernel and net-2.6.25 trees. > > regards, > Bjorge Dijkstra > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] [PATCH] Use wlan device name for RNDIS wireless devices
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
> Subject: [PATCH 8/8] [PATCH] Use wlan device name for RNDIS wireless devices
> Date: Sat, 22 Dec 2007 22:51:34 +0100
>
> From: Jussi Kivilinna <[EMAIL PROTECTED]>
>
> Use wlan device name for RNDIS wireless devices.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
... though it'd be a bit nicer IMO to see this before patch #7,
and just have one (or two) patches that only add infrastructure
to the usbnet core code, before the rndis_wext patch which uses
that new infrastructure.
> ---
> drivers/net/usb/rndis_wext.c |2 +-
> drivers/net/usb/usbnet.c |3 +++
> drivers/net/usb/usbnet.h |2 ++
> 3 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_wext.c b/drivers/net/usb/rndis_wext.c
> index a9ce944..1c28b2a 100644
> --- a/drivers/net/usb/rndis_wext.c
> +++ b/drivers/net/usb/rndis_wext.c
> @@ -2166,7 +2166,7 @@ static int rndis_wext_reset(struct usbnet *dev)
>
> struct driver_info rndis_wext_info = {
> .description = "Wireless RNDIS device",
> - .flags =FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
> + .flags =FLAG_WLAN | FLAG_FRAMING_RN | FLAG_NO_SETINT,
> .bind = rndis_wext_bind,
> .unbind = rndis_wext_unbind,
> .status = rndis_status,
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 8ed1fc5..a2a2d5e 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1204,6 +1204,9 @@ usbnet_probe (struct usb_interface *udev, const struct
> usb_device_id *prod)
> if ((dev->driver_info->flags & FLAG_ETHER) != 0
> && (net->dev_addr [0] & 0x02) == 0)
> strcpy (net->name, "eth%d");
> + /* WLAN devices should always be named "wlan%d" */
> + if ((dev->driver_info->flags & FLAG_WLAN) != 0)
> + strcpy(net->name, "wlan%d");
>
> /* maybe the remote can't receive an Ethernet MTU */
> if (net->mtu > (dev->hard_mtu - net->hard_header_len))
> diff --git a/drivers/net/usb/usbnet.h b/drivers/net/usb/usbnet.h
> index 83860a0..5c98ddc 100644
> --- a/drivers/net/usb/usbnet.h
> +++ b/drivers/net/usb/usbnet.h
> @@ -88,6 +88,8 @@ struct driver_info {
> #define FLAG_ETHER 0x0020 /* maybe use "eth%d" names */
>
> #define FLAG_FRAMING_AX 0x0040 /* AX88772/178 packets */
> +#define FLAG_WLAN0x0080 /* use "wlan%d" names */
> +
>
> /* init device ... can sleep, or cause probe() failure */
> int (*bind)(struct usbnet *, struct usb_interface *);
> --
> 1.5.2.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] Add Wireless Extensions to rndis_host
> From: Bjorge Dijkstra <[EMAIL PROTECTED]> > Subject: [PATCH 7/8] Add Wireless Extensions to rndis_host > Date: Sat, 22 Dec 2007 22:51:33 +0100 > > The bulk of this patch is the addition of a new file that > implements the wireless extensions for RNDIS devices. > The rest are some smaller changes to usbnet and rndis_host > to hook the wireless extensions into them: > * a private data pointer is added to usbnet. > * a callback is added to driver_info to signal link state changes. > * a physical medium type check is added to rndis_bind to check for > wireless lan devices and turn on the wireless extensions. > * and finally a Kconfig option to enable/disable this all. > > Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]> > Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]> I'm basically fine with this, but I'll hold off any ack until the issue with the preceding patch (6/8) is resolved. That would boil down to making this a standalone module, or knowing why that's not such a good idea. I'd prefer to see the usbnet core extension packaged separately instead of buried in this patch. :) And I think a MAINTAINERS update for rndis_wext would be appropriate. No way can I take care of that code! (In fact, would you feel comfortable taking over rndis_host too?) - Dave > --- > drivers/net/usb/Kconfig |7 + > drivers/net/usb/Makefile |4 + > drivers/net/usb/rndis_base.c | 50 +- > drivers/net/usb/rndis_host.h | 18 + > drivers/net/usb/rndis_wext.c | 2177 > ++ > drivers/net/usb/usbnet.h |4 + > 6 files changed, 2255 insertions(+), 5 deletions(-) > create mode 100644 drivers/net/usb/rndis_wext.c -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] [PATCH] Split up rndis_host.c
> From: Bjorge Dijkstra <[EMAIL PROTECTED]> > Subject: [PATCH 6/8] [PATCH] Split up rndis_host.c > Date: Sat, 22 Dec 2007 22:51:32 +0100 > > Split up rndis_host.c into rndis_host.h and rndis_base.c and > change Makefile accordingly. This is done so we can add extra > source files to the rndis_host module later on. I'm fine with splitting out a header file and the EXPORT_SYMBOL_GPL. But why not just have a separate "rndis_wext" module? > --- > drivers/net/usb/Makefile |1 + > drivers/net/usb/rndis_base.c | 548 ++ > drivers/net/usb/rndis_host.c | 763 > -- > drivers/net/usb/rndis_host.h | 256 ++ > 4 files changed, 805 insertions(+), 763 deletions(-) > create mode 100644 drivers/net/usb/rndis_base.c > delete mode 100644 drivers/net/usb/rndis_host.c > create mode 100644 drivers/net/usb/rndis_host.h -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] Fix rndis packet filter flags.
> From [EMAIL PROTECTED] Sat Dec 22 13:53:12 2007
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: netdev@vger.kernel.org, [EMAIL PROTECTED],
> Jussi Kivilinna <[EMAIL PROTECTED]>
> Subject: [PATCH 5/8] Fix rndis packet filter flags.
> Date: Sat, 22 Dec 2007 22:51:31 +0100
>
> From: Jussi Kivilinna <[EMAIL PROTECTED]>
>
> RNDIS packet filter flags are not exactly the same as CDC flags
> so we cannot reuse them.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
Hmm, the list seems to have grown since I last looked at it.
Or maybe it's just that the MSFT documentation was so hard
to make sense of ... I never could figure out what the RNDIS
power management messages were supposed to be doing, or what
the extra seemingly-undocumented messages were there for.
> ---
> drivers/net/usb/rndis_host.c | 23 ++-
> 1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index c686025..3c116f9 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -256,6 +256,27 @@ struct rndis_keepalive_c { /* IN (optionally OUT)
> */
> #define OID_GEN_MAXIMUM_FRAME_SIZE ccpu2(0x00010106)
> #define OID_GEN_CURRENT_PACKET_FILTERccpu2(0x0001010e)
>
> +/* packet filter bits used by OID_GEN_CURRENT_PACKET_FILTER */
> +#define RNDIS_PACKET_TYPE_DIRECTED ccpu2(0x0001)
> +#define RNDIS_PACKET_TYPE_MULTICAST ccpu2(0x0002)
> +#define RNDIS_PACKET_TYPE_ALL_MULTICAST ccpu2(0x0004)
> +#define RNDIS_PACKET_TYPE_BROADCAST ccpu2(0x0008)
> +#define RNDIS_PACKET_TYPE_SOURCE_ROUTING ccpu2(0x0010)
> +#define RNDIS_PACKET_TYPE_PROMISCUOUSccpu2(0x0020)
> +#define RNDIS_PACKET_TYPE_SMTccpu2(0x0040)
> +#define RNDIS_PACKET_TYPE_ALL_LOCAL ccpu2(0x0080)
> +#define RNDIS_PACKET_TYPE_GROUP ccpu2(0x1000)
> +#define RNDIS_PACKET_TYPE_ALL_FUNCTIONAL ccpu2(0x2000)
> +#define RNDIS_PACKET_TYPE_FUNCTIONAL ccpu2(0x4000)
> +#define RNDIS_PACKET_TYPE_MAC_FRAME ccpu2(0x8000)
> +
> +/* default filter used with RNDIS devices */
> +#define RNDIS_DEFAULT_FILTER ( \
> + RNDIS_PACKET_TYPE_DIRECTED | \
> + RNDIS_PACKET_TYPE_BROADCAST | \
> + RNDIS_PACKET_TYPE_ALL_MULTICAST | \
> + RNDIS_PACKET_TYPE_PROMISCUOUS )
> +
> /*
> * RNDIS notifications from device: command completion; "reverse"
> * keepalives; etc
> @@ -551,7 +572,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> u.set->oid = OID_GEN_CURRENT_PACKET_FILTER;
> u.set->len = ccpu2(4);
> u.set->offset = ccpu2((sizeof *u.set) - 8);
> - *(__le32 *)(u.buf + sizeof *u.set) = ccpu2(DEFAULT_FILTER);
> + *(__le32 *)(u.buf + sizeof *u.set) = RNDIS_DEFAULT_FILTER;
>
> retval = rndis_command(dev, u.header);
> if (unlikely(retval < 0)) {
> --
> 1.5.2.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] [PATCH] Halt device if rndis_bind fails.
> From [EMAIL PROTECTED] Sat Dec 22 13:53:07 2007
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: netdev@vger.kernel.org, [EMAIL PROTECTED],
> Jussi Kivilinna <[EMAIL PROTECTED]>
> Subject: [PATCH 4/8] [PATCH] Halt device if rndis_bind fails.
> Date: Sat, 22 Dec 2007 22:51:30 +0100
>
> From: Jussi Kivilinna <[EMAIL PROTECTED]>
>
> When bind fails after device was initialized, shutdown device properly
> by sending RNDIS_MSG_HALT.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
> drivers/net/usb/rndis_host.c | 12 +---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 42b161c..c686025 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -467,6 +467,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> struct rndis_query_c*get_c;
> struct rndis_set*set;
> struct rndis_set_c *set_c;
> + struct rndis_halt *halt;
> } u;
> u32 tmp;
> int reply_len;
> @@ -517,7 +518,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> "dev can't take %u byte packets (max %u)\n",
> dev->hard_mtu, tmp);
> retval = -EINVAL;
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
> dev->hard_mtu = tmp;
> net->mtu = dev->hard_mtu - net->hard_header_len;
> @@ -539,7 +540,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> 48, (void **) &bp, &reply_len);
> if (unlikely(retval< 0)) {
> dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval);
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
> memcpy(net->dev_addr, bp, ETH_ALEN);
>
> @@ -555,7 +556,7 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> retval = rndis_command(dev, u.header);
> if (unlikely(retval < 0)) {
> dev_err(&intf->dev, "rndis set packet filter, %d\n", retval);
> - goto fail_and_release;
> + goto halt_fail_and_release;
> }
>
> retval = 0;
> @@ -563,6 +564,11 @@ static int rndis_bind(struct usbnet *dev, struct
> usb_interface *intf)
> kfree(u.buf);
> return retval;
>
> +halt_fail_and_release:
> + memset(u.halt, 0, sizeof *u.halt);
> + u.halt->msg_type = RNDIS_MSG_HALT;
> + u.halt->msg_len = ccpu2(sizeof *u.halt);
> + (void) rndis_command(dev, (void *)u.halt);
> fail_and_release:
> usb_set_intfdata(info->data, NULL);
> usb_driver_release_interface(driver_of(intf), info->data);
> --
> 1.5.2.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] [PATCH] Use 1KB buffer in rndis_unbind
> From [EMAIL PROTECTED] Sat Dec 22 13:53:03 2007
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: netdev@vger.kernel.org, [EMAIL PROTECTED],
> Jussi Kivilinna <[EMAIL PROTECTED]>
> Subject: [PATCH 3/8] [PATCH] Use 1KB buffer in rndis_unbind
> Date: Sat, 22 Dec 2007 22:51:29 +0100
>
> From: Jussi Kivilinna <[EMAIL PROTECTED]>
>
> rndis_command requires the caller to pass in a buffer of at least 1KB.
>
> Signed-off-by: Jussi Kivilinna <[EMAIL PROTECTED]>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
> ---
> drivers/net/usb/rndis_host.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 96ef6a9..42b161c 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -577,7 +577,7 @@ static void rndis_unbind(struct usbnet *dev, struct
> usb_interface *intf)
> struct rndis_halt *halt;
>
> /* try to clear any rndis state/activity (no i/o from stack!) */
> - halt = kzalloc(sizeof *halt, GFP_KERNEL);
> + halt = kzalloc(CONTROL_BUFFER_SIZE, GFP_KERNEL);
> if (halt) {
> halt->msg_type = RNDIS_MSG_HALT;
> halt->msg_len = ccpu2(sizeof *halt);
> --
> 1.5.2.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] [PATCH] Hardwire CDC descriptors when missing
> From [EMAIL PROTECTED] Sat Dec 22 13:53:00 2007
> X-SourceIP: 213.93.131.145
> From: Bjorge Dijkstra <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: netdev@vger.kernel.org, [EMAIL PROTECTED]
> Subject: [PATCH 2/8] [PATCH] Hardwire CDC descriptors when missing
> Date: Sat, 22 Dec 2007 22:51:28 +0100
>
> Just as ActiveSync devices, some regular RNDIS devices also lack
> the CDC descriptors (e.g. devices based on BCM4320 WLAN chip).
> This patch hardwires the CDC descriptors for all RNDIS style devices
> when they are missing.
>
> Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
Note what this tells us about how little Microsoft cares about
the interoperability specs they claim to conform to. Having
those descriptors is *NOT* optional in their (incomplete and
in adequate) RNDIS specification. (I've not seen ActiveSync
specs, so that support is pure guesswork.)
> ---
> drivers/net/usb/cdc_ether.c | 10 +-
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index a42acc3..97c17bb 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -228,15 +228,16 @@ next_desc:
> buf += buf [0];
> }
>
> - /* Microsoft ActiveSync based RNDIS devices lack the CDC descriptors,
> - * so we'll hard-wire the interfaces and not check for descriptors.
> + /* Microsoft ActiveSync based and some regular RNDIS devices lack the
> + * CDC descriptors, so we'll hard-wire the interfaces and not check
> + * for descriptors.
>*/
> - if (is_activesync(&intf->cur_altsetting->desc) && !info->u) {
> + if (rndis && !info->u) {
> info->control = usb_ifnum_to_if(dev->udev, 0);
> info->data = usb_ifnum_to_if(dev->udev, 1);
> if (!info->control || !info->data) {
> dev_dbg(&intf->dev,
> - "activesync: master #0/%p slave #1/%p\n",
> + "rndis: master #0/%p slave #1/%p\n",
> info->control,
> info->data);
> goto bad_desc;
> @@ -316,7 +317,6 @@ void usbnet_cdc_unbind(struct usbnet *dev, struct
> usb_interface *intf)
> }
> EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
>
> -?
> /*-
> *
> * Communications Device Class, Ethernet Control model
> --
> 1.5.2.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Fix sparse warning: returning void-valued expression
> From [EMAIL PROTECTED] Sat Dec 22 13:52:53 2007 > From: Bjorge Dijkstra <[EMAIL PROTECTED]> > To: [EMAIL PROTECTED] > Cc: netdev@vger.kernel.org, [EMAIL PROTECTED] > Subject: [PATCH 1/8] Fix sparse warning: returning void-valued expression > Date: Sat, 22 Dec 2007 22:51:27 +0100 > > rndis_unbind and usbnet_cdc_unbind don't return anything. > > Signed-off-by: Bjorge Dijkstra <[EMAIL PROTECTED]> Acked-by: David Brownell <[EMAIL PROTECTED]> > --- > drivers/net/usb/rndis_host.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c > index 1ebe325..96ef6a9 100644 > --- a/drivers/net/usb/rndis_host.c > +++ b/drivers/net/usb/rndis_host.c > @@ -585,7 +585,7 @@ static void rndis_unbind(struct usbnet *dev, struct > usb_interface *intf) > kfree(halt); > } > > - return usbnet_cdc_unbind(dev, intf); > + usbnet_cdc_unbind(dev, intf); > } > > /* > -- > 1.5.2.5 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
On Wednesday 07 November 2007, Adrian Bunk wrote: > On Wed, Nov 07, 2007 at 02:34:52PM -0800, David Brownell wrote: > > > > But on the other hand, it seems that only the ASIX code will work > > > > right; the DM9601 and MCS7830 Kconfig is different/wrong. > > > > > > I'm not seeing the problem. > > > > > > Which configuration will be handled wrongly? > > > > Notice how only the ASIX kconfig depended on NET_ETHERNET... > > since MII depends on NET_ETHERNET, and (last I knew) the > > reverse dependencies didn't capture the complete dependency > > tree, selecting only MII would leave out some stuff. > > Except for one s390 net driver (I'll check why it's doing this) the > NET_ETHERNET option does not influence what code is being generated - > it's just a Kconfig-internal option allowing to disable a huge bunch > of drivers at once. Drivers like ... AX88xxx, DM9601, and MCS7830!! Except as it turns out, only the first one behaves as intended. You can tell it's a problem by the way it's inconsistent, regardless of the details of the problem. :) - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
> > But on the other hand, it seems that only the ASIX code will work > > right; the DM9601 and MCS7830 Kconfig is different/wrong. > > I'm not seeing the problem. > > Which configuration will be handled wrongly? Notice how only the ASIX kconfig depended on NET_ETHERNET... since MII depends on NET_ETHERNET, and (last I knew) the reverse dependencies didn't capture the complete dependency tree, selecting only MII would leave out some stuff. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] usbnet.c: check for the right MII variable
On Friday 02 November 2007, Adrian Bunk wrote:
> <-- snip -->
>
>
> This patch fixes the following compile error with CONFIG_MII=m,
> CONFIG_USB_USBNET=y, CONFIG_USB_USBNET_MII=n:
This is the patch I liked better, if there was going to be
one going upstream without additional test from me ... sorry,
the patch you appear to have accepted was the worst of the
three patches in flight.
>
> <-- snip -->
>
> ...
> LD .tmp_vmlinux1
> drivers/built-in.o: In function `usbnet_set_settings':
> (.text+0xf1876): undefined reference to `mii_ethtool_sset'
> drivers/built-in.o: In function `usbnet_get_settings':
> (.text+0xf1836): undefined reference to `mii_ethtool_gset'
> drivers/built-in.o: In function `usbnet_get_link':
> (.text+0xf18d6): undefined reference to `mii_link_ok'
> drivers/built-in.o: In function `usbnet_nway_reset':
> (.text+0xf18f6): undefined reference to `mii_nway_restart'
> make: *** [.tmp_vmlinux1] Error 1
>
> <-- snip -->
>
> This bug was introduced by commit 18ee91fa9815fa3bb4e51cdcb8229bd0a0f11a70
> and reported by Toralf Förster.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
>
> BTW:
> The Kconfig part of this patch is not really required, but testing for
> #if defined(CONFIG_USB_USBNET_MII) || defined(CONFIG_USB_USBNET_MII_MODULE)
> would look needlessly ugly.
>
> drivers/net/usb/Kconfig |5 ++---
> drivers/net/usb/usbnet.c |7 +++
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> a421e4910eb30b140a315e274632e87c7a218df6
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 5a96d74..9261371 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -94,12 +94,11 @@ config USB_RTL8150
> module will be called rtl8150.
>
> config USB_USBNET_MII
> - tristate
> - default n
> + bool
>
> config USB_USBNET
> tristate "Multi-purpose USB Networking Framework"
> - select MII if USB_USBNET_MII != n
> + select MII if USB_USBNET_MII
> ---help---
> This driver supports several kinds of network links over USB,
> with "minidrivers" built around a common network driver core
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index acd5f1c..7393ab0 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -683,8 +683,7 @@ done_nopm:
> * they'll probably want to use this base set.
> */
>
> -#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
> -#define HAVE_MII
> +#ifdef CONFIG_USB_USBNET_MII
>
> int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
> {
> @@ -744,7 +743,7 @@ int usbnet_nway_reset(struct net_device *net)
> }
> EXPORT_SYMBOL_GPL(usbnet_nway_reset);
>
> -#endif /* HAVE_MII */
> +#endif /* CONFIG_USB_USBNET_MII */
>
> void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo
> *info)
> {
> @@ -776,7 +775,7 @@ EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
>
> /* drivers may override default ethtool_ops in their bind() routine */
> static struct ethtool_ops usbnet_ethtool_ops = {
> -#ifdef HAVE_MII
> +#ifdef CONFIG_USB_USBNET_MII
> .get_settings = usbnet_get_settings,
> .set_settings = usbnet_set_settings,
> .get_link = usbnet_get_link,
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
On Wednesday 07 November 2007, David Miller wrote:
> David, I hate to say this and point you out like this, but you are a
> real cancer for bug fixes to USB things in the kernel,
You didn't hate it enough to find a way to deal with your issue
that doesn't involve namecalling or other flamage, I'll note.
I know, I know ... you wouldn't want anyone to get the mistaken
impression that LKML is one of those easy-to-get-along-on mailing
lists. Namecalling helps prevent anyone from ever thinking that!
> If I had a nickel for every patch from someone else you grinded into
> the ground and stalled I'd truly be a millionare.
Hyperbole helps too...
Twenty million patches, surely from some large fraction of a
million kernel patch submitters ... what a crazy dream!
(An enviable one, maybe; but way below the last statistics
on the subject which I read.)
As for "grind into the ground and stall" ... clearly that's
not how I'd describe things.
> You absolutely stifle development progress. I thought my OHCI
> deadlock patch was an isolated case (and nothing is still applied,
> which is just awesome, my original patch was posted more than a month
> ago),
I'm not sure why the patch I signed off on didn't merge either;
that was specificially related to the OHCI part of that problem.
Hmm, digging through my mail I see several other patches doing
the same thing were also floating around. None of them had any
improvement over what I had signed off on. Maybe there was just
too much confusion for Greg to want to merge the one which I'd
already signed off on...
It hasn't been removed from my merge queue, since it's not yet
shown up from kernel.org ... which means that it'd be one of
the patches to be resubmitted before we get much deeper into
this particular RC series. In fact, I just resent it (with a
few minor tweaks, essentially comment updates).
Of course, *YOU* are the roadblock on the more generic side of
that problem. I don't recall hearing back from you about the
minor/obvious update to the HUB+EHCI patch adding a msleep()
to bypass the hardware race you reported.
That is, other than your refusal to even try letting the three
or more clock domains finish synchronizing, before releasing
that lock and then starting something that relied on that synch
having finished...
> but you're doing the same exact thing to Adrian here too.
Hmm, and there I was prepared to sign off on Adrian's last patch.
True, I hadn't yet done so. But there were several workable fixes
in hand, plus a trivial workaround ... I just didn't make time to
try that one out over the weekend.
I'm glad you grabbed the patch I would have signed off on, if I'd
had time to verify it.
> You want to see things fixed your way. But you can get away with the
> if, and only if, you can spend every day working on your own version
> of fixes when you don't like the submitters version.
You know, that's hardly a standard kernel-wide policy for how
maintainers work ... except maybe ones in "supported" areas,
and not always even then.
Though I certainly know why it could seem attactive to want that
treatment for patches you submit. I know many of us would just love
to get that level of response/support for everything we submit.
I don't always get it; in some areas, it *never* happens.
In fact, it's pretty routine to have patches wait for a while
before they get merged, or even sometimes reviewed ... where
"a while" will sometimes cover many (annoying) months.
Also, some maintainers demand many iterations of patches, without
doing *any* fixup themselves. It's as if ... hmm ... maybe there
were only so many hours per day going into that such stuff? Or as
if people had different priorities? Bizarre notions, I know!!
> But unlike me
> you don't have that luxury so you have to give patch submitters a
> larger level of freedom and, plainly, just "let go".
Same goes for maintainers too, you know. If you're still
wondering about a patch, a private "ping" is common practice;
far more so than public vilification.
In fact, it's best if you never use a public vilification step.
Certainly not until after conventional measures ("ping!") fail
repeatedly, and bad faith has been clearly shown. (Neither of
which apply in this case.)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
On Friday 02 November 2007, Adrian Bunk wrote: > This approach has two disadvantages: > - it's complicated No more so than the problem itself. > - the MII stuff is an implementation detail, and we shouldn't bother > the user with it (especially since we can do better) That's a Kconfig policy that's not always followed. In this case, I was getting fed up with "select". It so rarely does what it needs to do, and I've started to think it'd be better to just always avoid that fragility than battle it. > If you want to keep the #ifdef's, what's the problem with the second > patch I proposed to fix this bug? For one thing, I didn't see it until after I posted this one... other than that, the basic approach could well be fine; I didn't go through it in detail. But on the other hand, it seems that only the ASIX code will work right; the DM9601 and MCS7830 Kconfig is different/wrong. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
On Thursday 01 November 2007, Adrian Bunk wrote:
> The following combination of options is simply an unusual one:
>
> CONFIG_MII=m
> CONFIG_USB_USBNET=y
> CONFIG_USB_USBNET_MII=n
I though that had been fixed for ages ...
This should do a better job of it.
- Dave
== CUT HERE
Simplify handling of the MII-dependent usbnet based adapters: stick
to forward dependencies, and explicitly handle the core dependency.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/usb/Kconfig | 23 ---
drivers/net/usb/usbnet.c |9 -
2 files changed, 20 insertions(+), 12 deletions(-)
--- a.orig/drivers/net/usb/Kconfig 2007-10-21 10:35:16.0 -0700
+++ a/drivers/net/usb/Kconfig 2007-11-02 11:32:15.0 -0700
@@ -93,13 +93,8 @@ config USB_RTL8150
To compile this driver as a module, choose M here: the
module will be called rtl8150.
-config USB_USBNET_MII
- tristate
- default n
-
config USB_USBNET
tristate "Multi-purpose USB Networking Framework"
- select MII if USB_USBNET_MII != n
---help---
This driver supports several kinds of network links over USB,
with "minidrivers" built around a common network driver core
@@ -131,11 +126,19 @@ config USB_USBNET
To compile this driver as a module, choose M here: the
module will be called usbnet.
+# usbnet core will support MII when MII is static, or both are modules
+config USB_NET_MII
+ tristate
+ depends on USB_USBNET && NET_ETHERNET && (MII = y || MII = USB_USBNET)
+ default MII
+
+comment "MII support is needed for most Ethernet adapters"
+ depends on USB_USBNET && USB_NET_MII=n
+
config USB_NET_AX8817X
tristate "ASIX AX88xxx Based USB 2.0 Ethernet Adapters"
- depends on USB_USBNET && NET_ETHERNET
+ depends on USB_USBNET && USB_NET_MII
select CRC32
- select USB_USBNET_MII
default y
help
This option adds support for ASIX AX88xxx based USB 2.0
@@ -188,9 +191,8 @@ config USB_NET_CDCETHER
config USB_NET_DM9601
tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
- depends on USB_USBNET
+ depends on USB_USBNET && USB_NET_MII
select CRC32
- select USB_USBNET_MII
help
This option adds support for Davicom DM9601 based USB 1.1
10/100 Ethernet adapters.
@@ -224,8 +226,7 @@ config USB_NET_PLUSB
config USB_NET_MCS7830
tristate "MosChip MCS7830 based Ethernet adapters"
- depends on USB_USBNET
- select USB_USBNET_MII
+ depends on USB_USBNET && USB_NET_MII
help
Choose this option if you're using a 10/100 Ethernet USB2
adapter based on the MosChip 7830 controller. This includes
--- a.orig/drivers/net/usb/usbnet.c 2007-10-13 15:16:10.0 -0700
+++ a/drivers/net/usb/usbnet.c 2007-11-02 11:39:59.0 -0700
@@ -682,10 +682,17 @@ done_nopm:
/* ethtool methods; minidrivers may need to add some more, but
* they'll probably want to use this base set.
*/
+#undef HAVE_MII
-#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#if defined(CONFIG_MII)
#define HAVE_MII
+#elif defined(CONFIG_MII_MODULE) && defined(MODULE)
+#define HAVE_MII
+#endif
+
+#ifdef HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] let USB_USBNET always select MII
On Thursday 01 November 2007, Adrian Bunk wrote: > All this USB_USBNET_MII trickery is simply not worth it considering how > few code it saves. Depends on what systems you're talking about. Forcing unused code into the kernel is not free, especially if that's made into a design policy and applied repeatedly to many subsystems. > As a side effect, this also fixes the following compile error reported > by Toralf Förster: Why not just fix the thing which changed and broke the build? Or if reverse dependencies can't be made to work sanely, then have those Ethernet-adapter minidrivers depend on NET_ETHERNET and then select MII. (To make the relationships be simple enough that current Kconfig can handle them.) I have a fair number of usbnet devices. Not one of them needs MII or NET_ETHERNET. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
On Thursday 01 November 2007, Randy Dunlap wrote: > The MII functions aren't available unless NET_ETHERNET=y. > Howver, the MII functions aren't always needed... > > David, any ideas on this one? It's been several years since I looked at this. It used to behave just fine. Something must have changed in the not-too-distant past to have broken this mechanism... > config USB_USBNET > tristate "Multi-purpose USB Networking Framework" > + depends on NET_ETHERNET if USB_USBNET_MII != n > select MII if USB_USBNET_MII != n > > would be handy. But invalid. > > Hm, wait. Haven't we seen this before and decided that MII should > be made more generally available? I.e., not depend on NET_ETHERNET? Some of us keep wanting to see "select" work properly, not omitting dependencies... Re interdependencies MII and NET_ETHERNET, I'll leave that up to the netedev folk. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH] USB: net: Fix asix read transfer buffer allocations.
> From [EMAIL PROTECTED] Tue Oct 23 10:51:00 2007
> Date: Tue, 23 Oct 2007 21:40:18 +0400
> From: Valentine Barshak <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: netdev@vger.kernel.org
> Subject: [linux-usb-devel] [PATCH] USB: net: Fix asix read transfer buffer
> allocations.
>
> On systems with noncoherent cache, allocating dma buffers
> on the stack for USB IN transfers causes kernel crash,
> because usb map_urb_for_dma() code calls dma_map_single(),
> that invalidates data cache for DMA_FROM_DEVICE transfer direction
> and causes stack data loss if transfer size is less than cache line
> and not cache-line aligned. This patch makes asix usb network
> driver allocate USB IN transfer buffers with kmalloc instead of
> directly using variables on stack. It also sets data parameter to NULL
> for zero-length transfers and uses ETH_ALEN size for allocating MAC
> address buffer.
Looks plausible to me, on a quick scan, but you should CC Dave Hollis
for updates to this drver ... last I knew, he was still maintaining
this code. (Though he's not listed in MAINTAINERS...)
This is missing a Signed-Off-By line ...
> diff -pruN linux-2.6.orig/drivers/net/usb/asix.c
> linux-2.6/drivers/net/usb/asix.c
> --- linux-2.6.orig/drivers/net/usb/asix.c 2007-10-23 20:52:11.0
> +0400
> +++ linux-2.6/drivers/net/usb/asix.c 2007-10-23 20:57:38.0 +0400
> @@ -568,15 +568,23 @@ static void asix_set_multicast(struct ne
> static int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
> {
> struct usbnet *dev = netdev_priv(netdev);
> + void *buf;
> u16 res;
>
> + buf = kmalloc(2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> mutex_lock(&dev->phy_mutex);
> asix_set_sw_mii(dev);
> asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
> - (__u16)loc, 2, (u16 *)&res);
> + (__u16)loc, 2, buf);
> asix_set_hw_mii(dev);
> mutex_unlock(&dev->phy_mutex);
>
> + res = *((u16 *)buf);
> + kfree(buf);
> +
> devdbg(dev, "asix_mdio_read() phy_id=0x%02x, loc=0x%02x,
> returns=0x%04x", phy_id, loc, le16_to_cpu(res & 0x));
>
> return le16_to_cpu(res & 0x);
> @@ -622,13 +630,22 @@ static void
> asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
> {
> struct usbnet *dev = netdev_priv(net);
> + void *buf;
> u8 opt;
>
> - if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) {
> + buf = kmalloc(1, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, buf) < 0) {
> wolinfo->supported = 0;
> wolinfo->wolopts = 0;
> + kfree(buf);
> return;
> }
> + opt = *((u8 *)buf);
> + kfree(buf);
> +
> wolinfo->supported = WAKE_PHY | WAKE_MAGIC;
> wolinfo->wolopts = 0;
> if (opt & AX_MONITOR_MODE) {
> @@ -644,7 +661,6 @@ asix_set_wol(struct net_device *net, str
> {
> struct usbnet *dev = netdev_priv(net);
> u8 opt = 0;
> - u8 buf[1];
>
> if (wolinfo->wolopts & WAKE_PHY)
> opt |= AX_MONITOR_LINK;
> @@ -654,7 +670,7 @@ asix_set_wol(struct net_device *net, str
> opt |= AX_MONITOR_MODE;
>
> if (asix_write_cmd(dev, AX_CMD_WRITE_MONITOR_MODE,
> - opt, 0, 0, &buf) < 0)
> + opt, 0, 0, NULL) < 0)
> return -EINVAL;
>
> return 0;
> @@ -820,7 +836,7 @@ static int ax88172_bind(struct usbnet *d
> for (i = 2; i >= 0; i--) {
> if ((ret = asix_write_cmd(dev, AX_CMD_WRITE_GPIOS,
> (gpio_bits >> (i * 8)) & 0xff, 0, 0,
> - buf)) < 0)
> + NULL)) < 0)
> goto out2;
> msleep(5);
> }
> @@ -831,7 +847,7 @@ static int ax88172_bind(struct usbnet *d
> /* Get the MAC address */
> memset(buf, 0, ETH_ALEN);
> if ((ret = asix_read_cmd(dev, AX88172_CMD_READ_NODE_ID,
> - 0, 0, 6, buf)) < 0) {
> + 0, 0, ETH_ALEN, buf)) < 0) {
> dbg("read AX_CMD_READ_NODE_ID failed: %d", ret);
> goto out2;
> }
> @@ -909,7 +925,7 @@ static int ax88772_bind(struct usbnet *d
>
> usbnet_get_endpoints(dev,intf);
>
> - buf = kmalloc(6, GFP_KERNEL);
> + buf = kmalloc(ETH_ALEN, GFP_KERNEL);
> if(!buf) {
> dbg ("Cannot allocate memory for buffer");
> ret = -ENOMEM;
> @@ -923,7 +939,7 @@ static int ax88772_bind(struct usbnet *d
> /* 0x10 is the phy id of the embedded 10/100 ethernet phy */
> embd_phy = ((asix_get_phy_addr(dev) & 0x1f) == 0x10 ? 1 : 0);
> if ((ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT,
> - embd_p
Re: [patch]support for USB autosuspend in the asix driver
On Monday 13 August 2007, Jeff Garzik wrote: > Oliver Neukum wrote: > > Hi, > > > > this implements support for USB autosuspend in the asix USB ethernet > > driver. > > > > Regards > > Oliver > > Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]> > > David, does this look OK to you? I never saw much comment from anybody, > and cannot really comment on the USB parts. I didn't see anything obviously wrong. I presume Oliver tested this with actual ASIX hardware ... assuming that, it's OK by me. - Dave > > Jeff > > > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH]: cdc-subset to support new vendor/product ID
On Friday 06 July 2007, jing xiang wrote:
> Hi,
>
> This patch is for cdc subset to support Mavell vendor/product ID.
>
> Jing Xiang
>
> Signed-off-by: Jing Xiang<[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>
>
> diff -uNpr linux-2.6.21.5/driver/usb/net/cdc_subset.c
> linux-2.6.21.5.t/driver/usb/net/cdc_subset.c
> --- linux-2.6.21.5/driver/usb/net/cdc_subset.c 2007-06-14
> 18:34:05.0 +0800
> +++ linux-2.6.21.5.t/driver/usb/net/cdc_subset.c2007-06-28
> 15:55:56.290598304 +0800
>
> @@ -305,6 +305,9 @@ static const struct usb_device_id produc
>USB_DEVICE (0x8086, 0x07d3),// "blob" bootloader
>.driver_info = (unsigned long) &blob_info,
> }, {
> + USB_DEVICE (0x1286, 0x8001),// "blob" bootloader
> + .driver_info = (unsigned long) &blob_info,
> +}, {
>// Linux Ethernet/RNDIS gadget on pxa210/25x/26x, second config
>// e.g. Gumstix, current OpenZaurus, ...
>USB_DEVICE_VER (0x0525, 0xa4a2, 0x0203, 0x0203),
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Cleanup usbnet_probe() return value handling
On Monday 02 July 2007, Peter Korsgaard wrote: > usbnet_probe() handles a positive return value from the driver bind() > function as success, but will later only setup the status handler if the > return value was zero, leading to confusion. Patch adjusts this to accept > positive values as success in both checks. > > Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> Signed-off-by: David Brownell <[EMAIL PROTECTED]> ... though I'd adjust comments to say "non-negative" rather than "positive". Most folks won't say that zero is positive. > --- > drivers/net/usb/usbnet.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6.22-rc7/drivers/net/usb/usbnet.c > === > --- linux-2.6.22-rc7.orig/drivers/net/usb/usbnet.c > +++ linux-2.6.22-rc7/drivers/net/usb/usbnet.c > @@ -1208,7 +1208,7 @@ > status = 0; > > } > - if (status == 0 && dev->status) > + if (status >= 0 && dev->status) > status = init_status (dev, udev); > if (status < 0) > goto out3; > > -- > Bye, Peter Korsgaard > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Cleanup usbnet_probe() return value handling
On Monday 02 July 2007, Peter Korsgaard wrote:
> usbnet_probe() handles a positive return value from the driver bind()
> function as success, but will later only setup the status handler if the
> return value was zero, leading to confusion. Patch adjusts this to only
> accept 0 as success in both checks.
I'd rather see the later test updated to match this one.
(Good catch!)
The return convention is "negative means error". There's
code in USB which has multiple nonnegative success codes.
In particular usb_control_msg(), which would very naturally
used as the body of a bind() method, returns negative or
the number of bytes transferred.
- Dave
> Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]>
> ---
> drivers/net/usb/usbnet.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.22-rc7/drivers/net/usb/usbnet.c
> ===
> --- linux-2.6.22-rc7.orig/drivers/net/usb/usbnet.c
> +++ linux-2.6.22-rc7/drivers/net/usb/usbnet.c
> @@ -1182,7 +1182,7 @@
> // NOTE net->name still not usable ...
> if (info->bind) {
> status = info->bind (dev, udev);
> - if (status < 0)
> + if (status != 0)
> goto out1;
>
> // heuristic: "usb%d" for links we know are two-host,
>
> --
> Bye, Peter Korsgaard
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2.6.22-rc6] net/usb/cdc_ether minor sparse cleanup
Remove an "sparse" warning about a shadowed variable name.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
drivers/net/usb/cdc_ether.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- g26.orig/drivers/net/usb/cdc_ether.c2007-07-01 11:40:37.0
-0700
+++ g26/drivers/net/usb/cdc_ether.c 2007-07-01 11:40:48.0 -0700
@@ -144,14 +144,14 @@ int usbnet_generic_cdc_bind(struct usbne
* modem interface from an RNDIS non-modem.
*/
if (rndis) {
- struct usb_cdc_acm_descriptor *d;
+ struct usb_cdc_acm_descriptor *acm;
- d = (void *) buf;
- if (d->bmCapabilities) {
+ acm = (void *) buf;
+ if (acm->bmCapabilities) {
dev_dbg(&intf->dev,
"ACM capabilities %02x, "
"not really RNDIS?\n",
- d->bmCapabilities);
+ acm->bmCapabilities);
goto bad_desc;
}
}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Update MAINTAINERS for USB network devices
> > > > @@ -3610,15 +3610,14 @@ > > > > USB CDC ETHERNET DRIVER > > > > P: Greg Kroah-Hartman > > > > I think that may refer to the old cdc ethernet driver > > though ... Greg? The new one, in the usbnet framework, > > is a very different beast... > > Yeah, this is the cdc_acm driver that is still in the USB drivers/ > directory tree as it is a USB class driver that shows up as a tty device > to userspace. It should not be moved to the networking list unless no > one minds that I never see any queries about it :) Then it should say "CDC ACM" not "CDC ETHERNET" ... ;) - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Update MAINTAINERS for USB network devices
On Wednesday 27 June 2007, Jeff Garzik wrote: > Peter Korsgaard wrote: > > Questions regarding the USB network drivers should now go to netdev. > > > > Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> > > --- > > MAINTAINERS | 13 + > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > Index: linux-2.6.22-rc6/MAINTAINERS > > === > > --- linux-2.6.22-rc6.orig/MAINTAINERS > > +++ linux-2.6.22-rc6/MAINTAINERS > > @@ -3610,15 +3610,14 @@ > > USB CDC ETHERNET DRIVER > > P: Greg Kroah-Hartman I think that may refer to the old cdc ethernet driver though ... Greg? The new one, in the usbnet framework, is a very different beast... > > M: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > +L: netdev@vger.kernel.org > > S: Maintained > > W: http://www.kroah.com/linux-usb/ > > > > USB DAVICOM DM9601 DRIVER > > P: Peter Korsgaard > > M: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > +L: netdev@vger.kernel.org > > W: http://www.linux-usb.org/usbnet > > S: Maintained > > > > @@ -3702,8 +3701,7 @@ > > USB PEGASUS DRIVER > > P: Petko Manolov > > M: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > +L: netdev@vger.kernel.org > > W: http://pegasus2.sourceforge.net/ > > S: Maintained > > > > @@ -3717,8 +3715,7 @@ > > USB RTL8150 DRIVER > > P: Petko Manolov > > M: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > +L: netdev@vger.kernel.org > > W: http://pegasus2.sourceforge.net/ > > S: Maintained > > > > @@ -3829,7 +3826,7 @@ > > USB "USBNET" DRIVER FRAMEWORK > > P: David Brownell > > M: [EMAIL PROTECTED] > > -L: [EMAIL PROTECTED] > > +L: netdev@vger.kernel.org > > W: http://www.linux-usb.org/usbnet > > S: Maintained > > Quite true, but for courtesy's sake you should keep the relevant > maintainers in the loop, and get their ACKs, when you start changing > their MAINTAINERS entries. Acked-by-Me. > > Jeff > > > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [Ipw2100-devel] [RFC] Runtime power management on ipw2100
On Thursday 08 February 2007 1:01 am, Zhu Yi wrote: > A generic requirement for dynamic power management is the hardware > resource should not be touched when you put it in a low power state. That is in no way a "generic" requirement. It might apply specifically to one ipw2100 low power state ... but "in general" devices may support more than one low power state, with different levels of functionality. Not all of those levels necessarily disallow touching the hardware. > But I think > freeing the irq handler before suspend should be the right way to go. Some folk like that model a lot for shared IRQs. It shouldn't matter for non-sharable ones. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 9/10][RFC] aio: usb gadget remove aio file ops
On Tuesday 16 January 2007 1:13 am, Nate Diller wrote:
> On 1/15/07, David Brownell <[EMAIL PROTECTED]> wrote:
> > What's needed is an async, non-sleeeping, interface ... with I/O
> > overlap. That's antithetical to using read()/write() calls, so
> > your proposed approach couldn't possibly work.
>
> haha, wow ok you convinced me :)
Good. :)
> I got a bit impatient when I was working on this, it took some time
> just to figure out the intention of the code, and I'm trying to hold
> to a bit of a schedule here. Without any clear (to me) reason, I
> didn't want to spend a lot of effort fixing this up.
Thing is, it's not OK to break other subsystems like that.
> There's really no big difference between the usb drivers here and the
> disk I/O scheduler queue, AFAICT,
The disk scheduler queue is more complex, as I understand things,
since it can combine operations. For USB, "combining" would break
essential semantics relied on by both sides of the transaction.
Maybe the best way to view this is to accept that with USB, all
scheduler operations (e.g. for bandwidth reservation management)
are out of scope of the AIO request model. AIO requests are no
more (or less) than "append this to the endpoint's I/O queue",
with the (host side) I/O scheduling handled separately.
> so it seems like the solution I want
> is to do a kmap() on the user buffer and then do the I/O straight out
> of that. That will eliminate the need for the bounce buffer. I'll
> post a new version along with the iodesc changes later this week.
Sounds more complex, but it would be nice to have that code become
zero-copy instead of single-copy. That'd let some platforms work
with high bandwidth ISO from userspace, which previously would not
have had enough CPU bandwidth. ("High bandwidth" means sustained
8-24 MByte/sec data streaming. Processing pixels at that rate may
require a companion DSP...) Testing will be different issue though.
- Dave
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 0/10][RFC] aio: make struct kiocb private
On Monday 15 January 2007 8:25 pm, Nate Diller wrote: > I don't think we should be waiting on sync I/O > at the *top* of the call stack, like with wait_on_sync_kiocb(), I'd > say the best place to wait is at the *bottom*, down in the I/O > scheduler. Erm ... *what* I/O scheduler? These I/O requests may go directly to the end of the hardware I/O queue, which already has an I/O model where each request can correspond directly to a KIOCB. And which does not include any synchronous primitives. No such scheduler has previously been, or _should_ be, required. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 9/10][RFC] aio: usb gadget remove aio file ops
On Monday 15 January 2007 5:54 pm, Nate Diller wrote: > This removes the aio implementation from the usb gadget file system. NAK. I see a deep mis-understanding here. > Aside > from making very creative (!) use of the aio retry path, it can't be of any > use performance-wise Other than the basic win of letting one userspace thread keep an I/O stream active while at the same time processing the data it reads or writes?? That's the "async" part of AIO. There's a not-so-little thing called "I/O overlap" ... which is the only way to prevent wasting bandwidth between (non-cacheable) I/O requests, and thus is the only way to let userspace code achieve anything close to the maximum I/O bandwidth the hardware can achieve. We want to see the host side "usbfs" evolve to support AIO like this too, for the same reasons. (Currently it has fairly ugly AIO code that looks unlike any other AIO code in Linux. Recent updates to support a file-per-endpoint device model are a necessary precursor to switching over to standard AIO syscalls.) > because it always kmalloc()s a bounce buffer for the > *whole* I/O size. By and large that's a negligible factor compared to being able to achieve I/O overlap. ISTR the reason for not doing fancy DMA magic was that the cost of this style AIO was under 1 KByte object code on ARM, which was easy to justify ... while DMA magic to do that sort of stuff would be much fatter, as well as more error prone. (And that's why the "creative" use of the retry path. As I've observed before, "retry" is a misnomer in the general sense of an async I/O framework. It's more of a semi-completion callback; I/O can't in general be "retried" on error or fault, and even in the current usage it's not really a "retry".) Now that high speed peripheral hardware is becoming more common on embedded Linuxes -- TI has DaVinci, OMAP 2430, TUSB6010 (as found in the new Nokia 800 tablets); Atmel AVR32 AP7000; at least a couple parts that should be able to use the same musb_hdrc driver as those TI parts; and a few other chips I've heard of -- there may be some virtue in eliminating the memcpy, since those CPUs don't have many MIPS to waste. (Iff the memcpy turns out to be a real issue...) > Perhaps the only reason to keep it around is the ability > to cancel I/O requests, which only applies when using the user space async > I/O interface. It's good to have almost the complete kernel API functionality exposed to userspace, and having I/O cancelation is an inevitable consequence of a complete AIO framework ... but that particular issue was not a driving concern. The reason for AIO is to have a *STANDARD* userspace interface for *ASYNC I/O* which otherwise can't exist. You know, the kind of I/O interface that can't be implemented with read() and write() syscalls, which for non-buffered I/O necessarily preclude all I/O overlap. AIO itself is a direct match to most I/O frameworks' primitives. (AIOCB being directly analagous to peripheral side "struct usb_request" and host side "struct urb".) You know, I've always thought that one reason the AIO discussions seemed strange is that they weren't really focussed on I/O (the lowlevel after-the-caches stuff) so much as filesystems (several layers up in the stack, with intervening caching frameworks). The first several implementations of AIO that I saw were restricted to "real" I/O and not applicable to disk backed files. So while I was glad the Linux approach didn't make that mistake, it's seemed that it might be wanting to make a converse mistake: neglecting I/O that isn't aimed at data stored on disks. > I highly doubt that is enough incentive to justify the extra > complexity here or in user-space, so I think it's a safe bet to remove this. > If that feature still desired, it would be possible to implement a sync > interface that does an interruptible sleep. What's needed is an async, non-sleeeping, interface ... with I/O overlap. That's antithetical to using read()/write() calls, so your proposed approach couldn't possibly work. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 4/10][RFC] aio: convert aio_complete to file_endio_t
On Monday 15 January 2007 5:54 pm, Nate Diller wrote:
> --- a/drivers/usb/gadget/inode.c 2007-01-12 14:42:29.0 -0800
> +++ b/drivers/usb/gadget/inode.c 2007-01-12 14:25:34.0 -0800
> @@ -559,35 +559,32 @@ static int ep_aio_cancel(struct kiocb *i
> return value;
> }
>
> -static ssize_t ep_aio_read_retry(struct kiocb *iocb)
> +static int ep_aio_read_retry(struct kiocb *iocb)
> {
> struct kiocb_priv *priv = iocb->private;
> - ssize_t len, total;
> - int i;
> + ssize_t total;
> + int i, err = 0;
>
> /* we "retry" to get the right mm context for this: */
>
> /* copy stuff into user buffers */
> total = priv->actual;
> - len = 0;
> for (i=0; i < priv->nr_segs; i++) {
> ssize_t this = min((ssize_t)(priv->iv[i].iov_len), total);
>
> if (copy_to_user(priv->iv[i].iov_base, priv->buf, this)) {
> - if (len == 0)
> - len = -EFAULT;
> + err = -EFAULT;
Discarding the capability to report partial success, e.g. that the first N
bytes were properly transferred? I don't see any virtue in that change.
Quite the opposite in fact.
I think you're also expecting that if N bytes were requested, that's always
how many will be received. That's not true for packetized I/O such as USB
isochronous transfers ... where it's quite legit (and in some cases routine)
for the other end to send packets that are shorter than the maximum allowed.
Sending a zero length packet is not the same as sending no packet at all,
for another example.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Network drivers that don't suspend on interface down
On Wednesday 20 December 2006 11:08 pm, Stephen Hemminger wrote: > David Brownell wrote: > > Hmm, this reminds me of a thread from last summer, following up on > > some PM discussions at OLS. Thread "Runtime power management for > > network interfaces", at the end of July. > > > > > > > >> 2) Network device infrastructure should make it easier for devices: > >> bring interface down on suspend and bring it up after resume > >> (if it was running when suspended). This would allow many devices to > >> have no suspend/resume hook; except those that have some better power > >> control over hardware. > >> > > > > The _intent_ of the class suspend() and resume() methods is to let > > infrastructure (the network stack was explicitly mentioned!) handle > > pretty much everything except putting the hardware in low power > > modes ... which last step might, for PCI devices at least, most > > naturally be done in suspend_late(). That way it'd be decoupled > > cleanly from anything else. > > > The class methods don't work right for that because the physical class > (PCI) gets called before the virtual class (network devices). I'd say they don't work just now because the virtual class code just doesn't get called ... at least, without someone setting up a field (device.class) that's flagged as "optional" and might be disappearing. But if you read the PM code, you'll observe that the class suspend method gets called BEFORE the bus/device suspend method. And that's how it's documented in Documentation/power/devices.txt too. ... However notice that "interface down" operations won't have that particular problem, they have net_device.class_dev.dev already ready for whatever PM operation is appropriate. - Dave > > Now, I recently tried refreshing a patch that used those class > > suspend() and resume() methods, and for some reason they're not > > getting called. I believe they used to get called, although it's > > true their parameter wasn't very useful ... it was called with the > > underlying device, not the class_device holding state that the > > class driver manages. > > > > I just wanted to point out that yes, this ground has been covered > > before, with some agreement on that approach. It'd be good to see > > it pursued. :) > > > > - Dave > > > > > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Network drivers that don't suspend on interface down
Hmm, this reminds me of a thread from last summer, following up on some PM discussions at OLS. Thread "Runtime power management for network interfaces", at the end of July. > 2) Network device infrastructure should make it easier for devices: > bring interface down on suspend and bring it up after resume > (if it was running when suspended). This would allow many devices to > have no suspend/resume hook; except those that have some better power > control over hardware. The _intent_ of the class suspend() and resume() methods is to let infrastructure (the network stack was explicitly mentioned!) handle pretty much everything except putting the hardware in low power modes ... which last step might, for PCI devices at least, most naturally be done in suspend_late(). That way it'd be decoupled cleanly from anything else. Now, I recently tried refreshing a patch that used those class suspend() and resume() methods, and for some reason they're not getting called. I believe they used to get called, although it's true their parameter wasn't very useful ... it was called with the underlying device, not the class_device holding state that the class driver manages. I just wanted to point out that yes, this ground has been covered before, with some agreement on that approach. It'd be good to see it pursued. :) - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Thursday 02 November 2006 6:27 pm, Adrian Bunk wrote: > It seems to lack the "select MII" at the USB_RTL8150 option that was in > Randy's first patch? I was just addressing the usbnet issues ... that driver doesn't use the usbnet framework. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Tuesday 31 October 2006 5:23 pm, Adrian Bunk wrote: > select MII if USB_NET_AX8817X!=n || USB_NET_MCS7830!=n Thing is, I'm seeing that get morphed inside Kconfig to "select MII" in some cases ... the "if x != n" gets ignored, MII can't be deselected. That looks to me like a Kconfig dependency engine bug, so I'm just noting it here rather than fixing it. I guess it's not quite enough of a Prolog engine ... ;) - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wednesday 01 November 2006 11:15 pm, Greg KH wrote:
> Argh, there were just too many different versions of these patches
> floating around. Can you resend the final versions please?
This should replace BOTH of Randy's patches. It addresses all the
issues I've heard raised, and resolves the regresssion introduced
when adding the mcs7830 minidriver.
- Dave
CUT HERE
Fix mcs7830 patch
The recent mcs7830 update to make the MII support sharable goofed various
pre-existing configurations in two ways:
- it made the usbnet infrastructure reference MII symbols even
when they're not needed in the kernel being built
- it didn't enable MII along with the mcs7830 minidriver
This patch fixes these two problems.
However, there does seem to be a Kconfig reverse dependency bug in that MII
gets wrongly enabled in some cases (like USBNET=y and USBNET_MII=n); I think
I've noticed that same problem in other situations too. So the result can
mean kernels being bloated by stuff that's needlessly enabled ... better
than wrongly being disabled, but contributing to bloat.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Index: at91/drivers/usb/net/Kconfig
===
--- at91.orig/drivers/usb/net/Kconfig 2006-11-02 10:58:49.0 -0800
+++ at91/drivers/usb/net/Kconfig2006-11-02 12:10:13.0 -0800
@@ -92,8 +92,13 @@ config USB_RTL8150
To compile this driver as a module, choose M here: the
module will be called rtl8150.
+config USB_USBNET_MII
+ tristate
+ default n
+
config USB_USBNET
tristate "Multi-purpose USB Networking Framework"
+ select MII if USBNET_MII != n
---help---
This driver supports several kinds of network links over USB,
with "minidrivers" built around a common network driver core
@@ -129,7 +134,7 @@ config USB_NET_AX8817X
tristate "ASIX AX88xxx Based USB 2.0 Ethernet Adapters"
depends on USB_USBNET && NET_ETHERNET
select CRC32
- select MII
+ select USB_USBNET_MII
default y
help
This option adds support for ASIX AX88xxx based USB 2.0
@@ -210,6 +215,7 @@ config USB_NET_PLUSB
config USB_NET_MCS7830
tristate "MosChip MCS7830 based Ethernet adapters"
depends on USB_USBNET
+ select USB_USBNET_MII
help
Choose this option if you're using a 10/100 Ethernet USB2
adapter based on the MosChip 7830 controller. This includes
Index: at91/drivers/usb/net/usbnet.c
===
--- at91.orig/drivers/usb/net/usbnet.c 2006-11-02 10:58:49.0 -0800
+++ at91/drivers/usb/net/usbnet.c 2006-11-02 11:09:29.0 -0800
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return "usbnet" */
- strncpy (info->driver, driver_name, sizeof info->driver);
- strncpy (info->version, DRIVER_VERSION, sizeof info->version);
- strncpy (info->fw_version, dev->driver_info->description,
- sizeof info->fw_version);
- usb_make_path (dev->udev, info->bus_info, sizeof info->bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev->msg_enable;
+ if (!dev->mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(&dev->mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev->msg_enable = level;
+ /* REVISIT don't always return "usbnet" */
+ strncpy (info->driver, driver_name, sizeof info->driver);
+ strncpy (info->version, DRIVER_VERSION, sizeof info->version);
+ strncpy (info->fw_ver
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
> > ... > > depends on MII if MII != n > > > > except that Kconfig doesn't comprehend conditionals like that. > > You can express this in Kconfig: > depends MII || MII=n Except that: Warning! Found recursive dependency: USB_USBNET USB_NET_AX8817X MII USB_USBNET I think this is another case where Kconfig gets in the way and forces introduction of a pseudovariable. I'll give that a try. > But my suggestion was: > #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) && defined(MODULE)) > > Or simply select MII ... Nope; those both prevent completely legit configurations. MII is not required, except for those two adapter options. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
> > +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) > > +#define HAVE_MII > >... > > This seems to cause a CONFIG_USB_USBNET=y, CONFIG_MII=m breakage > (as already described earlier in this thread)? Well, "alluded to" not described. Fixable by the equivalent of config USB_USBNET ... depends on MII if MII != n except that Kconfig doesn't comprehend conditionals like that. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Saturday 28 October 2006 2:13 pm, Christoph Hellwig wrote: > > I still don't quite like the approach. What about simply putting > the mii using functions into usbnet-mii.c and let makefile doing > all the work? This would require a second set of ethtool ops, > but I'd actually consider that a cleanup, as it makes clear which > one we're using and allows to kill all the checks for non-mii > hardware in the methods. Feel free to do such a patch yourself, so long as the MII doesn't go into a separate module. You'll have to also modify the two Ethernet adapters currently using that usbnet core code (and MII). That'd still feel like needless complexity to me, though. So unless you're keen on doing that quickly, I'd say to just merge the two existing patches and be done with it. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Saturday 28 October 2006 4:21 am, Christoph Hellwig wrote:
> This is really awkward and against what we do in any other driver.
Awkward, yes -- which is why I posted the non-awkward version,
which is repeated below. (No thanks to "diff" for making the
patch ugly though; the resulting code is clean and non-awkward,
moving that function helped.)
Against what other drivers do? Since "usbnet.c" is infrastructure
code, not a driver, your comment can't apply. Infrastructure uses
conditional compilation routinely in such cases.
But remember that the actual drivers follow the standard convention
("select MII") given Randy's patch #1 of 2.
- Dave
-
The usbnet infrastructure must not reference MII symbols unless they're
provided in the kernel being built. This extends also to the ethtool
hooks that reference those symbols.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Index: g26/drivers/usb/net/usbnet.c
===
--- g26.orig/drivers/usb/net/usbnet.c 2006-10-24 18:29:28.0 -0700
+++ g26/drivers/usb/net/usbnet.c2006-10-25 19:07:16.0 -0700
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return "usbnet" */
- strncpy (info->driver, driver_name, sizeof info->driver);
- strncpy (info->version, DRIVER_VERSION, sizeof info->version);
- strncpy (info->fw_version, dev->driver_info->description,
- sizeof info->fw_version);
- usb_make_path (dev->udev, info->bus_info, sizeof info->bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev->msg_enable;
+ if (!dev->mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(&dev->mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev->msg_enable = level;
+ /* REVISIT don't always return "usbnet" */
+ strncpy (info->driver, driver_name, sizeof info->driver);
+ strncpy (info->version, DRIVER_VERSION, sizeof info->version);
+ strncpy (info->fw_version, dev->driver_info->description,
+ sizeof info->fw_version);
+ usb_make_path (dev->udev, info->bus_info, sizeof info->bus_info);
}
-EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-int usbnet_nway_reset(struct net_device *net)
+u32 usbnet_get_msglevel (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- if (!dev->mii.mdio_write)
- return -EOPNOTSUPP;
+ return dev->msg_enable;
+}
+EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
- return mii_nway_restart(&dev->mii);
+void usbnet_set_msglevel (struct net_device *net, u32 level)
+{
+ struct usbnet *dev = netdev_priv(net);
+
+ dev->msg_enable = level;
}
-EXPORT_SYMBOL_GPL(usbnet_nway_reset);
+EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
/* drivers may override default ethtool_ops in their bind() routine */
static struct ethtool_ops usbnet_ethtool_ops = {
+#ifdef HAVE_MII
.get_settings = usbnet_get_settings,
.set_settings = usbnet_set_settings,
- .get_drvinfo= usbnet_get_drvinfo,
.get_link = usbnet_get_link,
.nway_reset = usbnet_nway_reset,
+#endif
+ .get_drvinfo= usbnet_get_drvinfo,
.get_msglevel = usbnet_get_msglevel,
.set_msglevel = usbnet_set_msglevel,
};
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
On Wednesday 25 October 2006 10:05 pm, Randy.Dunlap wrote: > > > > ... MII should still depend on ETHERNET, right? > > Just not limited to 10/100 Ethernet. > > There is no such config symbol. NET_ETHERNET means 10/100 ethernet. > Gigabit ethernet doesn't use the ETHERNET symbol (and doesn't use > this flavor of MII IIRC). Ah, you're right -- sorry. Only Kconfig and net/ipv4/arp.c even look for that config symbol. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wednesday 25 October 2006 4:58 pm, Randy Dunlap wrote:
> On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote:
>
> > Instead, "usbnet.c" should #ifdef the relevant ethtool hooks
> > according to CONFIG_MII ... since it's completely legit to
> > use usbnet with peripherals that don't need MII.
I had in mind something simpler -- #ifdeffing the entire functions,
as in this patch. It looks more complicated than it is, because
"diff" gets confused by moving two functions earlier in the file.
(Thanks for starting this, Randy ... these two patches should be merged
before RC4 ships.)
- Dave
The usbnet infrastructure must not reference MII symbols unless they're
provided in the kernel being built. This extends also to the ethtool
hooks that reference those symbols.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Index: g26/drivers/usb/net/usbnet.c
===
--- g26.orig/drivers/usb/net/usbnet.c 2006-10-24 18:29:28.0 -0700
+++ g26/drivers/usb/net/usbnet.c2006-10-25 19:07:16.0 -0700
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return "usbnet" */
- strncpy (info->driver, driver_name, sizeof info->driver);
- strncpy (info->version, DRIVER_VERSION, sizeof info->version);
- strncpy (info->fw_version, dev->driver_info->description,
- sizeof info->fw_version);
- usb_make_path (dev->udev, info->bus_info, sizeof info->bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev->msg_enable;
+ if (!dev->mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(&dev->mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev->msg_enable = level;
+ /* REVISIT don't always return "usbnet" */
+ strncpy (info->driver, driver_name, sizeof info->driver);
+ strncpy (info->version, DRIVER_VERSION, sizeof info->version);
+ strncpy (info->fw_version, dev->driver_info->description,
+ sizeof info->fw_version);
+ usb_make_path (dev->udev, info->bus_info, sizeof info->bus_info);
}
-EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-int usbnet_nway_reset(struct net_device *net)
+u32 usbnet_get_msglevel (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- if (!dev->mii.mdio_write)
- return -EOPNOTSUPP;
+ return dev->msg_enable;
+}
+EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
- return mii_nway_restart(&dev->mii);
+void usbnet_set_msglevel (struct net_device *net, u32 level)
+{
+ struct usbnet *dev = netdev_priv(net);
+
+ dev->msg_enable = level;
}
-EXPORT_SYMBOL_GPL(usbnet_nway_reset);
+EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
/* drivers may override default ethtool_ops in their bind() routine */
static struct ethtool_ops usbnet_ethtool_ops = {
+#ifdef HAVE_MII
.get_settings = usbnet_get_settings,
.set_settings = usbnet_set_settings,
- .get_drvinfo= usbnet_get_drvinfo,
.get_link = usbnet_get_link,
.nway_reset = usbnet_nway_reset,
+#endif
+ .get_drvinfo= usbnet_get_drvinfo,
.get_msglevel = usbnet_get_msglevel,
.set_msglevel = usbnet_set_msglevel,
};
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
On Wednesday 25 October 2006 4:59 pm, Randy Dunlap wrote: > On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote: > > > The other parts are right, this isn't. > > > > Instead, "usbnet.c" should #ifdef the relevant ethtool hooks > > according to CONFIG_MII ... since it's completely legit to > > use usbnet with peripherals that don't need MII. > > Ugh. OK. How's this? (2 patches) Looks about right, but ... > However, the MII config symbol should not be in the 10/100 Ethernet > menu, so that other drivers can use (enable) it or so that users > can enable it without needing to enable 10/100 Ethernet. ... MII should still depend on ETHERNET, right? Just not limited to 10/100 Ethernet. > --- linux-2619-rc3-pv.orig/drivers/net/Kconfig > +++ linux-2619-rc3-pv/drivers/net/Kconfig > @@ -145,6 +145,13 @@ config NET_SB1000 > > source "drivers/net/arcnet/Kconfig" > > +config MII > + tristate "Generic Media Independent Interface device support" > + help > + Most ethernet controllers have MII transceiver either as an external > + or internal device. It is safe to say Y or M here even if your > + ethernet card lacks MII. > + > source "drivers/net/phy/Kconfig" > > # > @@ -180,14 +187,6 @@ config NET_ETHERNET > kernel: saying N will just cause the configurator to skip all > the questions about Ethernet network cards. If unsure, say N. > > -config MII > - tristate "Generic Media Independent Interface device support" > - depends on NET_ETHERNET > - help > - Most ethernet controllers have MII transceiver either as an external > - or internal device. It is safe to say Y or M here even if your > - ethernet card lack MII. > - > source "drivers/net/arm/Kconfig" > > config MACE > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
> @@ -94,6 +95,7 @@ config USB_RTL8150 > > config USB_USBNET > tristate "Multi-purpose USB Networking Framework" > + select MII > ---help--- > This driver supports several kinds of network links over USB, > with "minidrivers" built around a common network driver core The other parts are right, this isn't. Instead, "usbnet.c" should #ifdef the relevant ethtool hooks according to CONFIG_MII ... since it's completely legit to use usbnet with peripherals that don't need MII. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
On Sunday 15 October 2006 6:10 pm, Andrew Morton wrote: > - A printk if something went bad Where "bad" would I hope exclude cases where the device doesn't support MWI ... that is, the "go faster if you can" advice from the driver, where it isn't an error to run into the common case of _this_ implementation not happening to be able to issue MWI cycles. The other cases, where something actually went wrong, would be reasonable for emitting KERN_ERR or KERN_WARNING messages. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bulk] Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
> > If the drivers doesn't care and if it makes no difference to performance > > then just delete the call to pci_set_mwi(). > > > > But if MWI _does_ make a difference to performance then we should tell > > someone that it isn't working rather than silently misbehaving? To repeat: it's not "misbehaving" since support for MWI cycles is completely optional. It'd be more interesting to get messages in the cases that it can be enabled, since typically it can't be. > That sounds like we need a printk inside pci_set_mwi then, rather than > adding a printk to every single caller. Maybe wrapped in #ifdef CONFIG_SPAM_MY_KERNEL_LOG_MESSAGES ... :) - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
> > I agree that set_mwo() should set MWI if possible, and fail cleanly > > if it couldn't (for whatever reason). Thing is, choosing to treat > > that as an error must be the _driver's_ choice ... it'd be wrong to force > > that policy into the _interface_ by forcing must_check etc. > > No. If pci_set_mwi() detects an unexpected error then the driver should > take some action: report it, recover from it, fail to load, etc. If the > driver fails to do any of this then it's a buggy driver. But what is an "unexpected" "error"? Not every fault that's unexpected is an error; consider a page fault. Consider also kfree(NULL). The same motivation for drivers not needing to validate that parameter is behind arguing that device drivers should not need to poke around in PCI config space to verify that the device supports MWI; and should have the flexibility to ignore the return code, just like kfree() returns no value. > You, the driver author _do not know_ what pci_set_mwi() does at present, on > all platforms, nor do you know what it does in the future. I know that it enables MWI accesses ... or fails. Beyond that, there should be no reason to care. If the hardware can use a lower-overhead type of PCI bus cycle, I want it to do so. If not, no sweat. > This is not a terribly important issue, and it is far from the worst case > of missed error-checking which we have in there. The reason I think it's important enough to continue this discussion is that as it currently stands, it's a good example of a **BAD** interface design ... since it's pointlessly marked as must_check. (See appended patch to fix that issue.) If you wouldn't want all callers of kfree() to say "if (ptr) kfree(ptr);"; why then would anyone want to demand ... read the pci config space byte (and cleanly handle errors) ... ... check for the MWI bit ... ... if it's set (so we "expect" this next call to succeed) then: ... call pci_set_mwi() ... ... test set_mwi() value ... ... ignore that value ... where the first three lines duplicate code _inside_ pci_set_mwi(), and the last two lines are obviously a pure waste of code and effort. I'd want to know the criteria by which "if(ptr)" is judged a waste of effort in all callers, but that more extensive PCI configspace logic was not... - Dave CUT HERE Remove bogus annotation of pci_set_mwi() as __must_check. It's completely reasonable for drivers to not care about the return code, when all they're doing is enabling an optional performance-improving mechanism that's often not even available. Signed-off-by: David Brownell <[EMAIL PROTECTED]> --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -499,7 +499,7 @@ int __must_check pci_enable_device_bars( void pci_disable_device(struct pci_dev *dev); void pci_set_master(struct pci_dev *dev); #define HAVE_PCI_SET_MWI -int __must_check pci_set_mwi(struct pci_dev *dev); +int pci_set_mwi(struct pci_dev *dev); void pci_clear_mwi(struct pci_dev *dev); void pci_intx(struct pci_dev *dev, int enable); int pci_set_dma_mask(struct pci_dev *dev, u64 mask); - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
> > Most drivers should be able to say "enable MWI if possible, but > > don't worry if it's not possible". Only a few controllers need > > additional setup to make MWI actually work ... if they couldn't > > do that setup, that'd be worth a warning before they backed off > > to run in a non-MWI mode. > > > > So the semantics of pci_set_mwi() are "try to set MWI if this > platform/device supports it". Not what I said ... that's what the _driver_ usually wants to do, which is different from the step implemented by set_mwi(). What Alan Cox said is a better paraphrase: > MWI is an "extra cheese" option not a "no pizza" case Or "sorry, that car is not available in olive, just burgundy." Not: > In that case its interface is misdesigned, because it doesn't discriminate > between "yes-it-does/no-it-doesn't" (which we don't want to report, because > either is expected and legitimate) and "something screwed up", which we do > want to report, because it is always unexpected. You mis-understand. It's completely legit for the driver not to care. I agree that set_mwo() should set MWI if possible, and fail cleanly if it couldn't (for whatever reason). Thing is, choosing to treat that as an error must be the _driver's_ choice ... it'd be wrong to force that policy into the _interface_ by forcing must_check etc. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
(From Alan Cox:) > The underlying bug is that someone marked pci_set_mwi must-check, that's > wrong for most of the drivers that use it. If you remove the must check > annotation from it then the problem and a thousand other spurious > warnings go away. Yes, there seems to be abuse of this new "must_check" feature. (From Andrew Morton:) > But if MWI _does_ make a difference to performance then we should tell > someone that it isn't working rather than silently misbehaving? Thing is, a "difference to performance (alone)" != "misbehavior". If it affected correctness, then a warning would be appropriate. Most drivers should be able to say "enable MWI if possible, but don't worry if it's not possible". Only a few controllers need additional setup to make MWI actually work ... if they couldn't do that setup, that'd be worth a warning before they backed off to run in a non-MWI mode. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bulk] Re: [PATCH 1/2] [PCI] Check that MWI bit really did get set
> But the only effect of returning EINVAL is a printk (for this particular > driver): > > /* PCI Memory-Write-Invalidate cycle support is optional (uncommon) */ > retval = pci_set_mwi(pdev); > if (!retval) > ehci_dbg(ehci, "MWI active\n"); Erm, I've lost context here but it's completely legit for hardware to NOT support MWI, so it is in no way an error if it's not set. (Memory-Write-Invalidate is just a more efficient way to write data that may be cached; if the device can't issue those cycles, there's no loss of correctness.) Since it's not an error, there should be no such printk ... which is exactly how it's coded above. Who is issuing the printk on a non-error code path?? - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Runtime power management for network interfaces
On Monday 31 July 2006 9:17 am, Stephen Hemminger wrote: > On Tue, 25 Jul 2006 11:59:52 -0400 (EDT) > Alan Stern <[EMAIL PROTECTED]> wrote: > > > During a Power Management session at the Ottawa Linux Symposium, it was > > generally agreed that network interface drivers ought to automatically > > suspend their devices (if possible) whenever: > > > > (1) The interface is ifconfig'ed down, or > > > > (2) No link is available. > > This is hard because most of the power may be consumed by the PHY interface > and it needs to be alive to see link. True only for #2, yes? I think #1 could be adopted pretty widely, but no driver I've yet come across implements that policy. I think maybe Don Becker didn't have power management on the brain nearly enough. :) > > Has any progress been made in this direction? If not, a natural approach > > would be to start with a reference implementation in one driver which > > could then be copied to other drivers. > > > > The problem is not generic, it really is specific to each device. For #2, yes. Much less so for #1; if the hardware has a low power mode, there's no point in being in any other mode when the interface is down. This might actually be a good time to start rethinking power management for network interfaces. Upcoming kernels (see the MM tree) have new class methods for suspend() and resume(), with the notion that they should be offloading tasks from drivers. For network links, that would most naturally be netif_device_detach() on suspend, etc; if netdevices were to provide suspend() and resume() methods, those could be called via class suspend/resume as well as when they're configured down/up. Just an idea of course ... but it might well be possible that some changes like that would be a nice incremental power savings on many systems, while simplifying some tasks that often confuse driver writers. > We have all the necessary infrastructure to do the right thing in the network > device driver, but in many cases we don't have the code or the technical > information > to do proper power management. I think that's more true for wakeup events than for PM in general. After all, quite a lot of network drivers do have suspend() methods that do something even if it's just going into PCI_D3, and resume() methods that are fully capable of re-initializing from power-off. - Dave - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Runtime power management for network interfaces
On Tuesday 25 July 2006 8:59 am, Alan Stern wrote:
> During a Power Management session at the Ottawa Linux Symposium, it was
> generally agreed that network interface drivers ought to automatically
> suspend their devices (if possible) whenever:
>
> (1) The interface is ifconfig'ed down, or
>
> (2) No link is available.
>
> Presumably (1) should be easy enough to implement.
I can't imagine that there are many network drivers that couldn't
put the hardware in some kind of low power mode at that point.
All PCI drivers can certainly set_pci_power_state(pdev, PCI_D3hot)
and save a certain amount of power. Of course, that means coping
with a possible device reset when going to PCI_D0, and I can imagine
some devices might prefer to use PCI_D2 rather than reload firmware.
Embedded chips probably have an easier time with this; just turn
off all the clocks going to that controller and its PHY.
> (2) might or might not
> be feasible, depending on how much WOL support is available.
A network adapter without PHY support in the WOL hardware wouldn't be
able to implement (2). And in today's systems, where we can't rely on
ACPI+PCI to morph PME# to pci_driver.resume() during runtime suspend,
not all platforms can implement (2). Embedded chips probably have an
easier time here ... I know for a fact that at91_ether can do that sort
of thing easily, at least on boards where the PHY IRQ is hooked up, so
that the driver doesn't need to poll link status.
The USB analogy here is that _some_ external transceivers (PHY) can
report link state changes via IRQs, supporting a mode where everything
else is powered off until the link can be active, at which point the
controller can come out of its low power state. Those are of course
transceivers intended for use in systems with itty bitty batteries.
(I keep thinking wakeup event handling is pretty weak in Linux today.
As you know, we're still shaking the USB HCDs into shape there, since
some system configs have issues. Network adapters seem to be another
major use case for wakeup events in Linux, and I get the impression
they're not as widely used -- or functional -- as would be good.)
> (It might
> not be feasible at all for wireless networking.) Still, there can be no
> question that it would be a Good Thing for laptops to power-down their
> ethernet controllers when the network cable is unplugged.
>
> Has any progress been made in this direction? If not, a natural approach
> would be to start with a reference implementation in one driver which
> could then be copied to other drivers.
>
> Any suggestions?
My initial thought was that network drivers could be refactored so
that they have ifsuspend() and ifresume() methods to put the hardware
into the low power state.
The remove(), open() and resume() methods would call ifresume(); probe(),
close() and suspend() would call ifsuspend() ... thatid give (1). And
for hardware supporting (2) there'd be some housekeeping for the WOL support
during suspend() and resume(), in addition to netif_device_{de,at}tach().
For hardware where a PHY can report link state without requiring an active
Ethernet controller (e.g. at91_ether for sure) some events could trigger
ifresume()/ifsuspend() when the interface is active.
- Dave
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH RFC] ZyDAS ZD1211 USB-WLAN driver
On Saturday 03 June 2006 3:25 pm, Oliver Neukum wrote:
> Am Samstag, 3. Juni 2006 21:35 schrieb Daniel Drake:
> > Oliver Neukum wrote:
> > > +static int read_mac_addr(struct zd_chip *chip, u8 *mac_addr)
> > > +{
> > > + static const zd_addr_t addr[2] = { CR_MAC_ADDR_P1, CR_MAC_ADDR_P2 };
> > > + return _read_mac_addr(chip, mac_addr, (const zd_addr_t *)addr);
> > > +}
> > >
> > > Why on the stack?
> >
> > Technically it's not on the stack because it is static const (it goes in
> > rodata), but I don't think that this invalidates your point. What's the
> > alternative? kmalloc and kfree every time?
>
> In this case rodata will work. However, if you ever switch to direct DMA
> it will fail. I really did overlook the const keyword.
On some platforms rodata will work; but it's not guaranteed on any.
See Documentation/DMA-mapping.txt and read the little section at the
top explaining what types of memory can be DMA'd from ... neither stack,
nor BSS, nor data, nor rodata, nor text are OK.
- Dave
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
