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

    > > Hi Bartolomiej!
    > Hi,
    > > --- Bartlomiej Zolnierkiewicz <>
    > > > 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 ;-).

    I've read it and I got the impression that a code cleanup is in order
    but nevermind, I left it as wrongly indented as it was.

    > > 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.
    > Okay.
    > > 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
    > 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... */

    OK, you are right, i am wrong. This functions are back.

    > > I also removed the CONFIG_BLK_DEV_CMD640_ENHANCED config option,
    > > 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
    > > 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.

    I disagree.

    > By removing setup_device_ptrs() and moving this driver to generic PCI
    > you broke support for VLB version of CMD640.

    I don't have a VLB version to test it on, but by reading the code I
    that it will work just fine.
    Anyway, if I'm wrong I should get a prize for breaking something that
    allready broken :-)))) .

    > Also there is a comment in a cmd640.c:
    > /*
    > * The CMD640x chip does not support DWORD config write cycles, but
    > * of the BIOSes use them to implement the config services.
    > */
    > which worries me that it might be not safe to move this driver to
    > IDE PCI layer (at least for now).

    Don't worry man, just read the code and you shall find peace of

    > > The code that handles PIO settings was rearanged in a new
    > > cmd640_tuneproc().
    > Is this really necessary, it is even harder to read it now...

    It is necessary, unless the purpose of this piece of code is

    > Stefan, please rework your patch. Thanks.

    If you say that the only way I will get this driver fixed is to keep
    ugly then I will send you a lame patch that does just that.

    > cheers,
    > --bartlomiej


    Stefan Talpalaru

    Do you Yahoo!?
    Protect your identity with Yahoo! Mail AddressGuard
    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.025 / U:55.588 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site