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? -- tejun - : 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
