[lkml]   [2003]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: PATCH: CMD640 IDE chipset

On Wednesday 29 of October 2003 13:12, Stefan Talpalaru wrote:
> Hi Bartolomiej!


> --- Bartlomiej Zolnierkiewicz <> wrote:
> > Hi,
> >
> > Can you please drop all code-style changes (such as foo() -> foo ())
> sorry about that, I ran Lindent on it...

Please read Documentation/CodingStyle instead ;-).

> Please excuse me for sending this patch as an attachment,
> but as my mail account is Yahoo! and I'm too lazy to find a better
> sollution, I cannot get the patch through the web interface without
> breaking the lines.


> This patch integrates the CMD640 chipset support in the 2.4.22
> kernel. I was using it succesfully in the 2.2.x kernel series, but
> got no result in the 2.4.x kernels. After comparing the 2 versions,
> I noticed errors in the new version (outb_p() instead of outl_p())
> and also some useless code (the wrapers: __put_cmd640_reg() and
> __get_cmd640_reg() - which I removed and placed the locks where needed;

It seems Alexander already covered removal of wrappers...

> the pci_conf1() and pci_conf2() functions).

You can't remove them.

/* Find out what kind of PCI probing is supported otherwise
we break some Adaptec cards... */

> I also removed the CONFIG_BLK_DEV_CMD640_ENHANCED config option, as
> it
> makes little difference for the kernel size.
> The init_hwif_cmd640() function had to be rewritten because it is
> called once for each ide interface found, so the old way of addressing
> all the drives in one run was no longer working. Therefore, to not
> break all the code, came the need for a function that computes the
> index from the ide_drive_t* : calculate_index().

ide_probe_for_cmd640x() should be still be used instead.
By removing setup_device_ptrs() and moving this driver to generic PCI layer,
you broke support for VLB version of CMD640.

Also there is a comment in a cmd640.c:

* The CMD640x chip does not support DWORD config write cycles, but some
* of the BIOSes use them to implement the config services.

which worries me that it might be not safe to move this driver to generic
IDE PCI layer (at least for now).

> The code that handles PIO settings was rearanged in a new function:
> cmd640_tuneproc().

Is this really necessary, it is even harder to read it now...

Stefan, please rework your patch. Thanks.


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 13:58    [W:0.072 / U:3.504 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site