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

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

    Hi,

    > --- Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> 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.

    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 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.

    cheers,
    --bartlomiej

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

    \
     
     \ /
      Last update: 2005-03-22 13:58    [W:0.024 / U:0.092 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site