Re: [PATCH] new mac_scsi driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2008-04-25 at 11:06 +1000, Finn Thain wrote:

On Thu, 24 Apr 2008, James Bottomley wrote:

On Thu, 2008-04-24 at 23:08 +0200, Geert Uytterhoeven wrote:
On Thu, 24 Apr 2008, Finn Thain wrote:
Replace the mac_esp driver with a new one based on the esp_scsi 
core.

For esp_scsi: add support for sync transfers for the PIO mode, add a 
new esp_driver_ops method to get the maximum dma transfer size (like 
the old NCR53C9x driver), and some cleanups.

Thanks!

I added this patch to my series, after fixing the few checkpatch.pl 
issues and adding a test for MACH_IS_MAC() to esp_mac_probe().

I got the tabs and spaces thing.  I'm not too concerned about the 
assignment in conditional, but I'm happy to go whichever way the author 
does.

FWIW, I happen to disagree with checkpatch about the spaces following tabs 
thing but I'm happy to follow convention. It did pick up on some other 
things that I overlooked (Geert has fixed them).


Could you repost please because I already have this queued, so I need a 
replacement (assuming everyone agrees).

Geert's version is fine with me:

http://linux-m68k-cvs.ubb.ca/~geert/linux-m68k-patches-2.6/m68k-new-mac_esp-driver.diff

It's a lot less easy to review stuff over the web (mainly, I'm afraid,
because people are lazy).

However, this:


+static int __devinit esp_mac_probe(struct platform_device *dev)
+{
+	struct scsi_host_template *tpnt = &scsi_esp_template;
+	struct Scsi_Host *host;
+	struct esp *esp;
+	int err;
+	int chips_present;
+	struct mac_esp_priv *mep;
+
+	if (!MACH_IS_MAC)
+		return -ENODEV;

Looks strange ... it seems you have to do this because macintosh_config
which is used later can be uninitialised (i.e. pointing to rubbish) if
it's not set?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux