On 2/3/24 04:53, Phillip Susi wrote: > Damien Le Moal <dlemoal@xxxxxxxxxx> writes: > >> Yes, but only for drives that report full identify data when PUIS is enabled. >> For drives that report incomplete identify data, we have no choice but to wake >> them up. And yes, we need integration with runtime pm to set the >> initial power > > Why was that again? I think you said something about needing to set the > speed correctly so you at least need to know if this drive requires a > lower speed than the other in the PATA master/slave pair? Wouldn't that > only require the speed information, not all identify data? See ata_dev_revalidate() and ata_dev_configure() and all the drive features that are checked using the identify data. We need to preserve that to ensure that nothing changed on the drive so that its representation in libata is kept in sync with the drive config. That is why drive starting with PUIS and not giving full identify data *must* be woken up, which is the current libata behavior. >> state of the drive to standby (instead of "on") for both the ata device and its >> scsi device. > > You mean if the whole device hierarchy were changed so that instead of > the scsi_host being a child of the port with the links and devices > hanging off to the side, the scsi_host would be the child of the ata device? You do not need to change the hierarchy of devices. An ata_dev is already a child of its scsi_dev. So if you want to set the ata device to runtime suspend state, you have to have the parent in the same state too. runtime suspend work top-to-bottom in the device chain. You cannot have random device in the middle of the chain going to suspend without the devices above it also being suspended. Also, the user does not use ata devices directly. They use the scsi device representing the ata device. You must thus have that in sync with the ata device state. >> I need to check that. I think there may be a better/easier way to get the >> current power state of a drive. Will get back to you on that. > > That would be good. At one point that was the way I found, and also I > think it was in the SAT-3 spec that is how REQUEST SENSE should be > implemented for ATA disks. TEST UNIT READY is the command to use. I need to check the specs again about how it reports the device state though. I think it is through sense data. The scsi disk driver issues that command already to check that the drive is spun-up. See sd_revalidate_disk() calling sd_spinup_disk(). So that will also need to change to be aware of the initial power state to not spinup the drive if not wanted. That will need to be done extremely carefully though to only affect ata devices with PUIS. Likely some additional scsi device flag will be needed for this. I have not looked into that yet. -- Damien Le Moal Western Digital Research
