public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Andrew Morton <akpm@osdl.org>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
	Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>
Subject: Re: [PATCH] CRIS architecture update
Date: Thu, 03 Jun 2004 11:41:47 -0400	[thread overview]
Message-ID: <40BF46BB.1080909@pobox.com> (raw)
In-Reply-To: <200406031419.i53EJgVI027812@hera.kernel.org>

Linux Kernel Mailing List wrote:
> ChangeSet 1.1726.1.144, 2004/06/01 08:52:29-07:00, akpm@osdl.org
> 
> 	[PATCH] CRIS architecture update
> 	
> 	From: "Mikael Starvik" <mikael.starvik@axis.com>
> 	
> 	- Lots of fixes from 2.4.
> 	
> 	- Updated for 2.6.6.
> 	
> 	- Added IDE driver
> 	
> 	Signed-off-by: Andrew Morton <akpm@osdl.org>
> 	Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Who reviewed the ethernet driver?
Who reviewed the IDE driver?
Has Bart seen this new driver?
Why was this committed without first being run by the subsystem maintainers?

In ethernet, the MII phy probe is wrong (don't need at id 0 first), it 
should be using linux/mii.h bits rather than inventing its own, 
del_timer versus del_timer_sync is questionable, the newly-added full 
duplex handling seems to be incorrect (ref drivers/net/mii.c and drivers 
that use mii_if->full_duplex), and cosmetic issues I won't bother to 
mention.

In IDE, it uses virt_to_page() when building the scatter/gather list -- 
something that IMO should not have been allowed in -mm much less 
mainline -- and other yuckiness.  In the same function, it's 
questionable whether or not it breaks with lba48.  etrax_dma_intr 
doesn't appear to check for some DMA engine conditions that code 
comments elsewhere in the driver indicate _do_ occur.  The 
ATAPI-specific e100_start_dma code doesn't look like it will work with 
all current ATAPI drivers (ide-{cdrom,floppy,tape,scsi,...}.c).

I'm happy that the CRIS people resurfaced, too, but that's no reason to 
just shove an unreviewed patch into mainline.

	Jeff



       reply	other threads:[~2004-06-03 15:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200406031419.i53EJgVI027812@hera.kernel.org>
2004-06-03 15:41 ` Jeff Garzik [this message]
     [not found] <BFECAF9E178F144FAEF2BF4CE739C668D0BC1E@exmail1.se.axis.com>
2004-06-04  5:46 ` [PATCH] CRIS architecture update Mikael Starvik
2004-06-04 16:57   ` Sam Ravnborg
2004-06-04 23:25   ` Bartlomiej Zolnierkiewicz
     [not found] <BFECAF9E178F144FAEF2BF4CE739C668D47E56@exmail1.se.axis.com>
2004-06-07  5:42 ` Mikael Starvik
     [not found] <BFECAF9E178F144FAEF2BF4CE739C668D47C24@exmail1.se.axis.com>
2004-06-07  5:53 ` Mikael Starvik
2004-06-07 18:43   ` Sam Ravnborg
2004-06-07 19:00     ` Bartlomiej Zolnierkiewicz

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=40BF46BB.1080909@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=B.Zolnierkiewicz@elka.pw.edu.pl \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@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