Tejun Heo wrote:
On Mon, Mar 13, 2006 at 05:13:36AM -0500, Jeff Garzik wrote:
Tejun Heo wrote:
On Mon, Mar 13, 2006 at 04:52:28AM -0500, Jeff Garzik wrote:
Tejun Heo wrote:
On Mon, Mar 13, 2006 at 03:29:24AM -0500, Jeff Garzik wrote:
Tejun Heo wrote:
Add per-dev pio/mwdma/udma_mask. All transfer mode limits used to be
applied to ap->*_mask which unnecessarily restricted other devices
sharing the port. This change will also benefit later EH speed down
and hotplug.
Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
I don't see much value in the separation. Rather than 3 separate
masks, it seems like this patch would be simplified if you simply added
dev->xfer_mask.
The thing is that ap->*_mask's are separated the same way and all
masking constants are defined as such. e.g.
ap->udma_mask &= ATA_UDMA5;
or
ap->udma_mask &= ATA_UDMA_MASK_40C;
Making dev->*_mask's the same enables share those constants and code
convention. So, things to consider here are...
1. Port xfer masks are defined as three separate masks.
2. All the constants are defined according to that.
3. Three separate masks are easier to deal with for LLDD's.
Separate masks is better for the LLDD interface, but the packed version
seems superior for internal libata use.
Yeah, dev->*_mask's will be used by LLDD's. Actually, in patch #4 of
this series, sata_sil's ->dev_config() does exactly that. Also, mode
mask filtering can be done by diddling dev->*_mask's.
No reason why ATA_UDMA_MASK_40C can't simply operate on a packed
xfer_mask variable, for example.
Because it's used to filter ap->*_mask and to allow LLDD's use the
same constants on dev->*_mask's.
Do you think LLDD's shouldn't access dev->*_mask's?
When I spoke of "LLDD API", I largely meant the pio_mask/etc. in
ata_port_info. That's the easiest way to present such information to
driver maintainers.
OTOH, at runtime ->dev_config() and friends should probably just
manipulate dev->xfer_mask.
I see.
So, ata_port_info uses pio/mwdma/udma_mask and ata_port and ata_device
use single xfer_mask and all the masking constants are converted to
mask single xfer_mask. Sounds good to you?
Well, if I read him correctly, Alan seems to have weighted in on the
side of pio/mwdma/udma_mask.
Maybe we could go with your original patches' direction for now, and
then move to a consolidated xfer_mask internally later.
Jeff
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
