Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply)

[Tejun Heo <htejun@gmail.com>]

Alan wrote:
>> * Control bits in the timing register wasn't cleared properly while
>>   programming PIO mode.
> 
> Yep and if the BIOS programmed the slave into DMA that might not be ideal.

How so?  The bit will be programmed by set_dmamode() right after
set_piomode() is complete.

>> * MWDMA mode programming cleared the wrong part of control bits.  I
>>   think this can fix your problem.
> 
> Your change does nothing here.
> 
> 	0x0F + 1 * 0xE1 = 0xF0
> 
> It's just a construct to avoid the use of the ugly C "?" operator. The
> rest of the code uses ? so the change makes sense for style, but it
> doesn't appear to be a bug.

(0x0F + 0xE1 * ap->port_no)

1. when ap->port_no == 0
	(0x0f + 0xe1 * 0) == 0x0f
2. when ap->port-no == 1
	(0x0f + 0xe1 * 1) == 0xf0

(ap->port_no ? 0x0f : 0xf0)

1. when ap->port_no == 0
	(0 ? 0x0f : 0xf0) == 0xf0
2. when ap->port_no == 1
	(1 ? 0x0f : 0xf0) == 0x0f

See the difference?  Smart one liners are dangerous.  ?* is _much_
better than cryptic arithmetic.

>> * MWDMA mode programming cleared udma_mask even when the controller
>>   doesn't support UDMA.  This doesn't matter for your case.
> 
> Or on the actual hardware.

I was trying to make it more consistent with pio counterpart.  We can
remove if() from set_piomode too.  Let's just keep things in sync
between stuff including ide piix driver.

-- 
tejun