On 1/8/24 22:39, Phillip Susi wrote: > Damien Le Moal <dlemoal@xxxxxxxxxx> writes: > >> Please use full 72-char lines for commit messages. The commit message also does >> not clearly describe what the patch does (completely silent on forcing the drive >> to sleep). > > It currently doesn't put it to sleep. > >>> +#if 0 >>> + ata_tf_init(dev, &tf); >>> + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; >>> + tf.protocol = ATA_PROT_NODATA; >>> + tf.command = ATA_CMD_SLEEP; >>> + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); >>> + ata_dev_info(dev, "PuiS detected, putting drive to sleep"); >> >> I already commented that this is not following the ACS specifications and thus >> should not be done. So again, nack. > > It is #if 0'd out. I also addressed this in the cover letter. Sure, > this shouldn't be done by default, but I don't see a problem with > leaving it as an option that can be activated by those whose drives > don't have a problem with this. We never add dead code. And code under a "#if 0" is by design dead... So please do not do that. -- Damien Le Moal Western Digital Research
