lkml.org 
[lkml]   [2008]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 5/5] falconide/q40ide: add ->atapi_*put_bytes and ->ata_*put_data methods
    On Mon, Apr 07, 2008 at 09:13:42PM +0200, Bartlomiej Zolnierkiewicz wrote:

    >
    > v2:
    > * Add 'struct request *rq' argument to ->ata_{in,out}put_data methods
    > and don't byte-swap disk fs requests (we shouldn't un-swap fs requests
    > because fs itself is stored byte-swapped on the disk) - this is how
    > things were done before the patch (ideally device mapper should be
    > used instead but it would break existing setups and would have some
    > performance impact).

    I like the part about checking 'struct request *rq', will make a clean
    distinction between ide command data and ide disk/medium data which is badly
    needed.

    However, not only FS data is byteswapped, complete disk including partition
    table and everything else is. Will "rq->cmd_type == REQ_TYPE_FS" also catch
    all these cases?



    > Index: b/drivers/ide/legacy/falconide.c
    > ===================================================================
    > --- a/drivers/ide/legacy/falconide.c
    > +++ b/drivers/ide/legacy/falconide.c
    > @@ -44,6 +44,36 @@
    > int falconide_intr_lock;
    > EXPORT_SYMBOL(falconide_intr_lock);
    >
    > +static void falconide_atapi_input_bytes(ide_drive_t *drive, void *buf,
    > + unsigned int len)
    > +{
    > + insw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
    > +}
    > +
    > +static void falconide_atapi_output_bytes(ide_drive_t *drive, void *buf,
    > + unsigned int len)
    > +{
    > + outsw_swapw(drive->hwif->io_ports.data_addr, buf, (len + 1) / 2);
    > +}
    > +
    > +static void falconide_ata_input_data(ide_drive_t *drive, struct request *rq,
    > + void *buf, unsigned int wcount)
    > +{
    > + if (drive->media == ide_disk && rq && rq->cmd_type == REQ_TYPE_FS)
    > + return insw(drive->hwif->io_ports.data_addr, buf, wcount * 2);
    > +
    > + falconide_atapi_input_bytes(drive, buf, wcount * 4);
    > +}

    this will still do double swapping for disk access, although this is very
    easier to fix once the distinction between ide and disk data works fine.

    So IMHO despite the little concerns this is the way to go.

    Richard


    \
     
     \ /
      Last update: 2008-04-08 11:39    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean