mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Michael Leun <lkml20130126@newton.leun.net>
Cc: Freddy Xin <freddy@asix.com.tw>,
	netdev@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, louis@asix.com.tw,
	davem@davemloft.net, Support@asix.com.tw
Subject: Re: [PATCH, resubmit] ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver
Date: Mon, 28 Jan 2013 10:07:32 +0100	[thread overview]
Message-ID: <87zjztptez.fsf@nemi.mork.no> (raw)
In-Reply-To: <20130127001347.584711b2@xenia> (Michael Leun's message of "Sun, 27 Jan 2013 00:13:47 +0100")

Hello Michael,

Michael Leun <lkml20130126@newton.leun.net> writes:

> I would vote to not accept that driver for mainline as long as this
> issues are not fixed. 
>
> The vendor should not be able to claim "hooray, hooray, great device,
> we even have an driver in linux main line" when it is actually such an
> useless crap.

Well, that is fortunately not how these things work. The main goal is
getting the devices supported in the kernel. Bugs can be fixed.  If a
vendor can get any positive gain out of having a driver in mainline,
then that is good for everyone, isn't it?  Of course, we can all agree
that the effect of a *working* driver is more positive than a
non-working driver...

For now, the main focus should be fixing the issues which has been noted
during review.  Your testing feedback is of course very useful, but you
probably need to back them up with actual code change proposals if they
are going to be dealt with at this stage.

> Of course I'm offering to help with any information or testing, but
> unfortunately I do not have the knowhow to fix anything myself. 

I believe this is where you are totally wrong.  You obviuously have the
ability to create a few simple test cases for yourself and see if the
driver behaves as you expect.  That is very useful.

And you have a device.  That is also useful.

Now, the driver source code is available.  And there is another Asix
driver in the kernel which already has been cleaned up and can be used
as an example. And maybe even partly used for the new devices as well,
if the code is duplicated?  I have not looked at this in detail, but I
suspect that much of the problem with the ax88179_178a driver is that it
has completely ignored all the work that has gone into the asix driver
after it was mainlined.  I find it unlikely that there is no reusable
code in the asix_devices.c, asix_common.c and ax88172a.c files.  Trying
to rewrite ax88179_178a to share as much code as possible seems like the
best way to clean it up and fix bugs.

You have documents like
http://www.kernel.org/doc/Documentation/SubmittingPatches helping a lot
with cleanup jobs like this, because it is mostly about just following
that recipe.  If you do, then the code will be reviewable by the
community because the size will be reduced and the style readable.  And
the driver might even work, as the cleanup will quite possibly reveal a
number of bugs...

Please consider this option.  Reading and fixing up C code is not very
difficult. Getting this driver into the kernel in a good shape is mostly
just monkey work.  But some monkey has to do it :-)


Bjørn

  reply	other threads:[~2013-01-28  9:08 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-23  2:32 Freddy Xin
2013-01-23  4:15 ` David Miller
2013-01-26 23:13 ` Michael Leun
2013-01-28  9:07   ` Bjørn Mork [this message]
2013-01-28 13:36     ` Freddy
2013-01-28 14:24       ` Bjørn Mork
2013-01-28 16:42       ` Michael Leun
  -- strict thread matches above, loose matches on Subject: below --
2013-03-02 10:41 [PATCH " Freddy Xin
2013-03-03  6:44 ` David Miller
2013-02-27 12:01 Freddy Xin
2013-03-01 21:15 ` David Miller
2013-02-07 13:39 [PATCH, " Freddy Xin
2013-02-07 20:02 ` Ben Hutchings
2013-02-07 20:05   ` David Miller
2013-02-08  3:41   ` Freddy
2013-02-08 10:23   ` David Laight
2013-02-08 19:23     ` David Miller
2013-02-07  4:36 Freddy Xin
2013-02-07  5:45 ` Stephen Hemminger
2013-02-07  6:45   ` Freddy
2013-02-07  5:46 ` Stephen Hemminger
2013-02-07  6:50   ` Freddy
2013-02-03 14:21 Daniel J Blueman
2013-12-04 12:48 ` Freddy
2013-01-17  9:32 Freddy Xin
2013-01-18 19:23 ` David Miller
2013-01-18 20:29   ` Alan Stern

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=87zjztptez.fsf@nemi.mork.no \
    --to=bjorn@mork.no \
    --cc=Support@asix.com.tw \
    --cc=davem@davemloft.net \
    --cc=freddy@asix.com.tw \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lkml20130126@newton.leun.net \
    --cc=louis@asix.com.tw \
    --cc=netdev@vger.kernel.org \
    /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

all inboxes | Powered by JetHome®