Date:	Wed, 15 Dec 2010 14:17:21 -0800 (PST)
From:	Luben Tuikov <ltuikov@...oo.com>
To:	Matthew Dharm <mdharm-kernel@...-eyed-alien.net>,
	Ben Gamari <bgamari@...il.com>
Cc:	Alan Cox <alan@...rguk.ukuu.org.uk>, Greg KH <greg@...ah.com>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver

--- On Tue, 12/14/10, Ben Gamari <bgamari@...il.com> wrote:
> > > Your driver may be the best on the planet - who
> knows - 
> > Obviously he didn't review it.
> 
> This is may be true, perhaps he didn't look at the code.

"may be true". It is clear he didn't. What he wrote is quoted above.

> This is not
> because he doesn't have confidence that your driver is
> well-written or
> holds a grudge against you.

Have you been authorized to speak for him?

> The reason for this NAK is
> policy: a driver
> already exists. In the kernel we work with what already
> exists, even if
> it may be more difficult than the alternative.

Nothing can be further from the truth. If you have been long enough around Linux, you'd know that many times there would be two perfectly working drivers for the same device with hundreds of thousands of those devices out in the wild, in the kernel. You'd also know that sometimes a driver is submitted and accepted and in the kernel, but it is so badly written that it gets re-written by one of the resident "linux kernel engineers" and then the latter driver replaces the one already existing, etc, etc. Add to this that it is sometimes good to offer people a choice between a defunct and unmaintained driver and a working and maintained driver.

Now is the time to do the right thing and get a working and technically correct UAS driver, when there are no UAS devices out there.

> I looked at your code; from my uninformed perspective it
> looks very
> nice. You clearly do have a good understanding of the stack
> and your
> code looks refreshingly clean. That being said, a driver
> covering the
> same devices already exists in the tree. There may be good
> technical
> reasons why this driver is inferior, but these are merely
> reasons to fix
> the existing code, not rip it out to be replaced
> wholesale.

It's much easier, faster and foolproof to rip out a defunct driver than to submit 30+ patches that change 100% of the driver. Moreover when there are no devices for it yet.

> At this point you have two options: You can continue to
> argue that the
> world is against you,

I don't argue that nor do I think that.

> which as we seen makes neither side
> happy and only
> makes your life as a contributor harder, or you can try to
> work what you
> have into something that will fit with the existing
> codebase. I suggest
> the latter. Further arguing is not going to change this
> policy. That
> being said, your expertise is certainly needed and it is in
> everyone's
> best interest to ensure that the UAS driver in the kernel
> is the best
> that it can be.

The situation is as follows: the Linux band is more than happy to review patches to /their/ code, but when you submit a driver that replaces a defunct driver of theirs in the kernel, then all of a sudden no one can read code to give a technical review of your driver.  So they say "we're happy to accept patches to /our/ code even if it replaces 100% of it, but your driver which does this in one go is rejected, sorry" and "I've no idea if it is good or bad".

> Improving code is very useful.

Improving code can be useful, but not to the extent of replacing it 100% in one go in 30+ patches.

A patch is supposed to fix a single bug or a feature missed, etc. For example see my recent patches posted as well as examine git history, as well as examine my patches from before there was git (starting with the SCSI layer using the slab cache for struct scsi_cmnd allocation).

> History has taught us that
> flat-out
> rewriting components just makes things messier.

Nothing can be further from the truth. This isn't a whimsical rewrite. I tried to "fix" uas.c but it's just horrible and needs to be ripped out.  Just one patch of mine changed 86% of the code and that's NOT right. But with that defunct driver, there was just no other way. The terminology the driver uses show the writer is not familiar with the technology. The logistical flow of the code shows the writer isn't familiar with the way a SCSI Delivery Subsystem works, UAS and USB in general. I've outlined that in my 18 technical point review earlier in this thread.

The right thing was to write a UAS driver that is correct and that it works, thus uasp.c.

> He apologized because he knows you put a lot of time,
> effort, and

Again, you speak for other people. This isn't a good thing.

> thought into a piece of code which we cannot accept.

"we"? See when you write emails here, and pretend to be this benevolent selfless creature lacking all self-interest, you have to be more subtle and present an unbiased opinion. Using "we" isn't the way to do it.

> It is
> unfortunate
> that this is the case but such is the reality of
> large-scale software
> development.

uasp.c is 1141 lines, a featherweight in drivers class. There is no "large-scale software development". uasp.c is self-contained driver. It doesn't change or modify any of the rest of the kernel. So now you can see how there is NO "large-scale software development".  It doesn't affect the system other than when uasp.c is used people will notice that they can now use a UAS device.

> Easy question. See above. In short, there is no sense to
> give a
> technical review of code which we ultimately won't be able
> use.

You said it: "ultimately won't be able to use". Basically a decision has already been made. And here you have shown the problem: the biased opinion of the Linux band: you're welcome to improve /our/ code even it you completely replace it in one go in 30+ patches, but we're not accepting your driver, under any circumstances.  How about give people a choice?

> Sometimes this sort of thing happens. If I were you, I
> would accept what
> people have been telling you and try to integrate portions
> of your

People? You mean a two people who have known each other for over 10 years are close knit friends who work for the same company whose company sponsors their conferences and gives them equipment.

> patch into the existing UAS driver. This will cause the
> least pain/most
> benefit to all involved.

The least pain/most benefit is to replace the defunct uas.c with uasp.c, so people can test the UAS devices they are developing and when those devices are released on the market, people can use them.  See here: http://marc.info/?l=linux-scsi&m=129227488508299&w=2

   Luben

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists