linux-kernel - Re: [PATCH 41/86] pata_efar: unify code for programming PIO and MWDMA timings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B0EC8F5.9040409@ru.mvista.com>
Date:	Thu, 26 Nov 2009 21:29:09 +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 41/86] pata_efar: unify code for programming PIO and MWDMA
 timings

Bartlomiej Zolnierkiewicz wrote:

> It results in ~9% decrease in the driver LOC count and also ~6%
> decrease in the driver binary size (as measured on x86-32).

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> ---
>  drivers/ata/pata_it8213.c |   87 +++++++++++++++-------------------------------
>  1 file changed, 29 insertions(+), 58 deletions(-)

> Index: b/drivers/ata/pata_it8213.c
> ===================================================================
> --- a/drivers/ata/pata_it8213.c
> +++ b/drivers/ata/pata_it8213.c
> @@ -61,20 +61,9 @@ static int it8213_cable_detect(struct at
>  	return ATA_CBL_PATA80;
>  }
>  
> -/**
> - *	it8213_set_piomode - Initialize host controller PATA PIO timings
> - *	@ap: Port whose timings we are configuring
> - *	@adev: Device whose timings we are configuring
> - *
> - *	Set PIO mode for device, in host controller PCI config space.
> - *
> - *	LOCKING:
> - *	None (inherited from caller).
> - */
> -
> -static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
> +static void it8213_set_timings(struct ata_port *ap, struct ata_device *adev,
> +			       u8 pio, bool use_mwdma)

    Perhaps 'set_mwdma' would be a better name...

>  {
> -	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
>  	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
>  	u8 master_port		= ap->port_no ? 0x42 : 0x40;
>  	u16 master_data;
> @@ -92,13 +81,18 @@ static void it8213_set_piomode (struct a
>  			    { 2, 1 },
>  			    { 2, 3 }, };
>  
> -	if (pio > 1)
> +	if (pio > 1 || use_mwdma)
>  		control |= 1;	/* TIME */
> -	if (ata_pio_need_iordy(adev))	/* PIO 3/4 require IORDY */
> +	if (ata_pio_need_iordy(adev) || use_mwdma)

    I believe this "IORDY for MWDMA" stupidity results from the table 35 in 
ICH PRD which for some reason insists on setting IE bit with DMA, and I 
believe this is wrong -- IORDY shouldn't have anything to with DMA.

>  		control |= 2;	/* IE */
>  	/* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
>  	if (adev->class != ATA_DEV_ATA)
>  		control |= 4;	/* PPE */
> +	/* If the drive MWDMA is faster than it can do PIO then
> +	   we must force PIO into PIO0 */
> +	if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))

    Parens not needed...

> +		/* Enable DMA timing only */
> +		control |= 8;	/* PIO cycles in PIO0 */
>  
>  	pci_read_config_word(dev, master_port, &master_data);

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/