lkml.org 
[lkml]   [2005]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH] libata ATAPI alignment
    On Fri, Jul 29, 2005 at 01:06:54AM -0400, Jeff Garzik wrote:
    >
    > So, one thing that's terribly ugly about SATA ATAPI is that we need to
    > pad DMA transfers to the next 32-bit boundary, if the length is not
    > evenly divisible by 4.
    >
    > Messing with the scatterlist to accomplish this is terribly ugly
    > no matter how you slice it. One way would be to create my own
    > scatterlist, via memcpy and then manual labor. Another way would be
    > to special case a pad buffer, appending it onto the end of various
    > scatterlist code.
    >
    > Complicating matters, we currently must support two methods of data
    > buffer submission: a single kernel virtual address, or a struct
    > scatterlist.
    >
    > Review is requested by any and all parties, as well as suggestions for
    > a prettier approach.
    >
    > This is one of the last steps needed to get ATAPI going.
    >

    Hello, Jeff, Jens & Alan.

    I've rewritten the patch to fix some bugs and make it a bit (IMHO)
    simpler to use.

    Bug fixes...

    * When copying a sg, original implementation just kmap'ed sg->page
    which can cause trouble as a sg can span more than a page.

    * In ata_sg_clean(), the original implementation accesses last sg by
    indexing w/ (qc->n_elem - 1). This is incorrect as qc->n_elem
    could have been shrunk by dma_map_sg() in ata_sg_setup().

    * In the original code (before Jeff's patch), sata_sx4 used
    sg[i].legnth to calculate total_len. This is wrong for the same
    reason as above and Jeff's patch fixes it. I separated out this
    fix just to clarify.

    Changes...

    * Instead of adding pad sg handling to each fill_sg function,
    ata_for_each_sg() macro is added. Normal sg entries and
    qc->pad_sgent are setup by sg_setup routines and fill_sg functions
    can just iterate w/ ata_for_each_sg() without caring about padding.

    * hw_max_segments is automatically decremented in slave_config if
    attached device is ATAPI.

    Questions/suggestions...

    * I didn't include AHCI_MAX_SG change as it looked a bit more out of
    place w/ other changes to ahci gone. Also, I'm curious about how
    meaningful increasing AHCI_MAX_SG is. We're currently setting
    max_phys_segments to LIBATA_MAX_PRD, which is 128, and AFAIK max hw
    segments higher than phys segments is meaningless (phys segs are
    merged into hw segs and one phys segment cannot be splitted into
    two hw segs). Am I missing something here?

    * About core routines/callbacks. Currently, libata-core file
    contains actual libata core routines and all helper functions for
    taskfile controllers. ata_ prefix is also shared by both function
    categories. This is a bit confusing, IMO.

    I think ata_port_start should allocate stuff necessary for libata
    core and call ->port_start callback and similary for ata_port_stop,
    and current ata_port_start/stop need to be renamed to something
    like ata_tf_port_start/stop, such that we don't have to allocate
    data structures needed by libata core in specific drivers (ahci in
    this case).

    I've tested both sg and non-sg paths with sg_test_rwbuf. When
    testing sg path, I've commented out sg.c:L1951 (sg_build_indirect:L10)
    to prevent it padding transfer size to 512byte boundary. Read/write
    padding in both paths work.

    Two patches will follow this mail. The first one fixes sata_sx4 as
    mentioned above and the second implements atapi align.

    Thanks.

    --
    tejun
    -
    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-08-07 07:53    [W:5.224 / U:0.828 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site