[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <49AFCFDB.9080203@ru.mvista.com>
Date: Thu, 05 Mar 2009 16:12:59 +0300
From: Sergei Shtylyov <sshtylyov@...mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] ide: merge ->atapi_*put_bytes and ->ata_*put_data
methods
Hello.
Bartlomiej Zolnierkiewicz wrote:
> * Merge ->atapi_{in,out}put_bytes and ->ata_{in,out}put_data methods
> into new ->{in,out}put_data methods which take number of bytes to
> transfer as an argument and always do padding.
> While at it:
> * Use 'hwif' or 'drive->hwif' instead of 'HWIF(drive)'.
> There should be no functional changes caused by this patch (all users
> of ->ata_{in,out}put_data methods were using multiply-of-4 word counts).
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Congratulations -- you introduced a bug with this patch. ;-)
> Index: b/drivers/ide/ide-iops.c
> ===================================================================
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -191,34 +191,45 @@ static void ata_vlb_sync(ide_drive_t *dr
>
> /*
> * This is used for most PIO data transfers *from* the IDE interface
> + *
> + * These routines will round up any request for an odd number of bytes,
> + * so if an odd len is specified, be sure that there's at least one
> + * extra byte allocated for the buffer.
> */
> -static void ata_input_data(ide_drive_t *drive, void *buffer, u32 wcount)
> +static void ata_input_data(ide_drive_t *drive, void *buf, unsigned int len)
> {
> ide_hwif_t *hwif = drive->hwif;
> struct ide_io_ports *io_ports = &hwif->io_ports;
> + unsigned long data_addr = io_ports->data_addr;
> u8 io_32bit = drive->io_32bit;
>
> + len++;
> +
Now where is the symmetric len++ in ata_output_data()?
> if (io_32bit) {
> if (io_32bit & 2) {
> unsigned long flags;
>
> local_irq_save(flags);
> ata_vlb_sync(drive, io_ports->nsect_addr);
> - hwif->INSL(io_ports->data_addr, buffer, wcount);
> + hwif->INSL(data_addr, buf, len / 4);
> local_irq_restore(flags);
> } else
> - hwif->INSL(io_ports->data_addr, buffer, wcount);
> + hwif->INSL(data_addr, buf, len / 4);
> +
> + if ((len & 3) >= 2)
> + hwif->INSW(data_addr, (u8 *)buf + (len & ~3), 1);
> } else
> - hwif->INSW(io_ports->data_addr, buffer, wcount << 1);
> + hwif->INSW(data_addr, buf, len / 2);
> }
>
> /*
> * This is used for most PIO data transfers *to* the IDE interface
> */
> -static void ata_output_data(ide_drive_t *drive, void *buffer, u32 wcount)
> +static void ata_output_data(ide_drive_t *drive, void *buf, unsigned int len)
> {
> ide_hwif_t *hwif = drive->hwif;
> struct ide_io_ports *io_ports = &hwif->io_ports;
> + unsigned long data_addr = io_ports->data_addr;
> u8 io_32bit = drive->io_32bit;
>
> if (io_32bit) {
Patch coming. It turned out to be really good thing that I decided to look
into these methods...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists