Alan wrote: > O> Wouldn't it be better to have ->determine_xfer_mask() and >> ->set_specific_mode() than having two somewhat overlapping callbacks? >> Or is there some problem that can't be handled that way? > > I'm not sure I follow what you are suggesting - can you explain further. > > Right now ->set_mode does all the policy management with regards to > picking the right modes which is sometimes done by the usual ATA rules > and sometimes by card specific code. > > ->set_specific_mode does no policy work but merely sets up a mode. > > The default behaviour of ->set_mode() is the ATA mode selection by best > mode available, and this function is normally not provided by a driver. > > The default behaviour of ->set_specific_mode() is to verify the mode is > valid then issue ->set_pio|dma_mode() (for both devices in case a timing > change on both is triggered). This function is overridable because of > things like IT821x where the IDE mode is imaginary. What I was thinking about was something like the following. * ops unsigned int (*determine_xfer_mask)(struct ata_device *dev); int (*set_specific_mode)(struct ata_device *dev, unsigned int xfer_mode); * during init and EH if (init) { ap->xfer_mask &= ops->determine_xfer_mask(dev); DETERMINE best_mode; } if (ap->ehi.target_mode && valid) mode = ap->ehi.target_mode; else mode = best_mode; rc = ops->set_specific_mode(dev, mode); * when the user issues SET_XFERMODE, in the issue path if (command is SET_XFERMODE) { if (mode is invalid) fail; ap->ehi.target_mode = user_specified_mode; ata_port_schedule_eh(ap); done; } To sum up, 1. separate supported mode detection and mode programming such that we can use the same programming path for both init and handling user-issued SET_XFERMODE. 2. group all mode programming callbacks (->set_piomode, ->set_dmamode and ->post_set_mode into ->set_specific_mode) into ->set_specific_mode to allow more flexibility and replace ->set_mode. 3. make sure all xfer mode programming is done in EH. this will ease support for weird controllers (e.g. cross-port synchronization). Thanks. -- tejun - To unsubscribe from this list: 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