From: "\"L. Alberto Giménez\"" <agimenez@sysvalve.es>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-usb@vger.kernel.org, oliver@neukum.org,
linville@tuxdriver.com, j.dumon@option.com,
steve.glendinning@smsc.com, davem@davemloft.net, gregkh@suse.de,
dgiagio@gmail.com, dborca@yahoo.com
Subject: Re: [PATCHv3] drivers/net/usb: Add new driver ipheth
Date: Fri, 02 Apr 2010 19:15:12 +0200 [thread overview]
Message-ID: <4BB62620.3070807@sysvalve.es> (raw)
In-Reply-To: <1270077538.8653.484.camel@localhost>
On 04/01/2010 01:18 AM, Ben Hutchings wrote:
> On Wed, 2010-03-31 at 21:42 +0200, L. Alberto Giménez wrote:
> [...]
>> --- /dev/null
>> +++ b/drivers/net/usb/ipheth.c
> [...]
Hi Ben,
Upstream has fixed several errors pointed out by you and Oliver (thanks
for that), but some of them are still pending.
I will send patches on top of my last driver submission (if the proper
way would be resubmit the whole code, please tell me. Anyway I need to
clarify some doubts...
>> + usb_fill_bulk_urb(dev->tx_urb, udev,
>> + usb_sndbulkpipe(udev, dev->bulk_out),
>> + dev->tx_buf, IPHETH_BUF_SIZE,
>> + ipheth_sndbulk_callback,
>> + dev);
>> + dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +
>> + retval = usb_submit_urb(dev->tx_urb, GFP_ATOMIC);
>> + if (retval) {
>> + err("%s: usb_submit_urb: %d", __func__, retval);
>> + dev->stats.tx_errors++;
>> + dev_kfree_skb_irq(skb);
>> + } else {
>> + net->trans_start = jiffies;
>
> No longer needed.
What is not longer needed? The assignment, the whole "else" branch? If
the assignment is what is not needed, can I just remove that line, right?
> [...]
>> +#ifdef HAVE_NET_DEVICE_OPS
>> +static const struct net_device_ops ipheth_netdev_ops = {
>> + .ndo_open = &ipheth_open,
>> + .ndo_stop = &ipheth_close,
>> + .ndo_start_xmit = &ipheth_tx,
>> + .ndo_tx_timeout = &ipheth_tx_timeout,
>> + .ndo_get_stats = &ipheth_stats,
>> +};
>> +#endif
>
> Remove the #ifdef, there is no question whether we have net_device_ops.
Ok, I will just remove both #ifdefs, but why is that? Maybe in previous
versions of the kernel the net_device_ops struct was introduced and now
it's present no matter how you configure your kernel?
> I have no idea about USB so I haven't checked the USB API usage at all.
I think that Greg is the maintainer for the USB subsystem, so if he has
no further commets, I will try to submit fixes for both your and
Oliver's comments along with the upstream developers.
Thanks for your comments.
Best regards,
--
L. Alberto Giménez
JabberID agimenez@jabber.sysvalve.es
GnuPG key ID 0x3BAABDE1
next prev parent reply other threads:[~2010-04-02 17:15 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-30 21:34 [PATCH] Staging: Add new driver ipheth L. Alberto Giménez
2010-03-30 21:34 ` L. Alberto Giménez
2010-03-30 21:45 ` Greg KH
2010-03-30 21:58 ` "L. Alberto Giménez"
2010-03-30 22:11 ` Greg KH
2010-03-31 14:33 ` Pavel Machek
2010-03-31 14:44 ` Matthew Garrett
2010-03-31 19:47 ` "L. Alberto Giménez"
2010-03-30 23:01 ` [PATCH] drivers/net/usb: " L. Alberto Giménez
2010-03-30 23:25 ` Greg KH
2010-03-31 19:42 ` [PATCHv3] " L. Alberto Giménez
2010-03-31 20:33 ` Oliver Neukum
2010-03-31 21:38 ` "L. Alberto Giménez"
2010-04-02 18:23 ` "L. Alberto Giménez"
2010-04-04 7:24 ` Oliver Neukum
2010-04-05 18:51 ` "L. Alberto Giménez"
2010-03-31 23:18 ` Ben Hutchings
2010-03-31 23:25 ` Greg KH
2010-03-31 23:28 ` Ben Hutchings
2010-04-02 17:15 ` "L. Alberto Giménez" [this message]
2010-04-02 17:21 ` Ben Hutchings
2010-04-02 17:53 ` "L. Alberto Giménez"
2010-04-02 18:35 ` Ben Hutchings
2010-04-07 22:11 ` [PATCH Resubmission] " L. Alberto Giménez
2010-04-07 22:37 ` Roland Dreier
2010-04-08 6:35 ` Oliver Neukum
2010-04-13 8:15 ` David Miller
2010-04-13 19:03 ` "L. Alberto Giménez"
2010-04-13 21:29 ` David Miller
2010-04-15 19:46 ` [PATCH Resubmission v2] " L. Alberto Giménez
2010-04-16 6:44 ` David Miller
2010-04-18 18:35 ` [PATCH] " L. Alberto Giménez
2010-04-21 14:15 ` Diego Giagio
2010-04-22 5:44 ` David Miller
2010-03-31 22:10 [PATCHv3] " Daniel Borca
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BB62620.3070807@sysvalve.es \
--to=agimenez@sysvalve.es \
--cc=bhutchings@solarflare.com \
--cc=davem@davemloft.net \
--cc=dborca@yahoo.com \
--cc=dgiagio@gmail.com \
--cc=gregkh@suse.de \
--cc=j.dumon@option.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=netdev@vger.kernel.org \
--cc=oliver@neukum.org \
--cc=steve.glendinning@smsc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).