linux-kernel - Re: [PATCH] pata_it821x: sync with IDE it821x driver

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 for Android: free password hash cracker in your pocket

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

Message-Id: <200706101840.40460.bzolnier@gmail.com>
Date:	Sun, 10 Jun 2007 18:40:40 +0200
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Jeff Garzik <jeff@...zik.org>, linux-ide@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pata_it821x: sync with IDE it821x driver


Hi Alan!

On Sunday 10 June 2007, Alan Cox wrote:
> > @@ -258,8 +259,14 @@ static void it821x_passthru_set_piomode(
> >  	static const u8 pio_want[]    = { ATA_66, ATA_66, ATA_66, ATA_66, ATA_ANY };
> >  
> >  	struct it821x_dev *itdev = ap->private_data;
> > +	struct ata_device *pair = ata_dev_pair(adev);
> >  	int unit = adev->devno;
> > -	int mode_wanted = adev->pio_mode - XFER_PIO_0;
> > +	int mode_wanted = adev->pio_mode;
> > +
> > +	if (pair && adev->pio_mode > pair->pio_mode)
> > +		mode_wanted = pair->pio_mode;
> > +
> > +	mode_wanted -= XFER_PIO_0;
> 
> NAK this too

OK, I'm able to understand the meaning of "NAK" [1] but "this too"?

[1] http://ozlabs.org/~rusty/index.cgi/2007/05/04

;)

> Please see it821x_program.
> 
> it821x_passthru_set_piomode computes the required mode and the preferred
> clock for the mode
> it821x_clock_strategy the computes the best clock to use
> it821x_program the programs the device
> 
> Whenever we issue a command it821x_passthru_dev_select then does a timing
> mode switch and loads the PIO timing before doing the dev_select. The
> same is also done when doing a qc_issue_prot so each qc_issue will have
> the right timings loaded for the device.
> 
> So if you've got a problem case your diagnosis isn't one that makes a lot
> of sense, and the fix is clearly wrong but I've not seen most of the
> discussion between you and whoever you worked with to judge why.
> 
> One obvious possibility is that we are not currently doing timing
> clipping for the address setup and merge for the 8bit timings. However
> the docs I have strongly imply that the IT821x PIO/MWDMA timing register
> is for data cycles only.
> 
> The other would be that there is some kind of pattern of interaction
> between the ATA midlayer and the driver causing the wrong timings to get
> loaded at some point.
> 
> Your "fix" would happen to work for both of these cases, but is wrong for
> both of them.
> 
> If the register is switching all the timings then the fix is to switch
> the PIO timing loaded to the lowest of the two initially (in _program)
> and flip it back to the correct value at the start of the data transfer
> sequence. We can then flip it back next qc_issue or indeed at the end of
> the data xfer
> 
> If we've got an interaction problem then we need proper traces with
> LIBATA debug turned right up so we can fix the actual case where the
> switching occurs wrongly.

PIO fix was directly ported from my it821x.c patch.  I now see that thanks
to it821x_passthru_dev_select() it is not really needeed in pata_it821x.c.

OTOH DMA fixes are pata_it821x.c unique, I will respin the patch.

Bart
-
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/