On 10/19/22 18:32, John Garry wrote: > On 18/10/2022 16:04, Niklas Cassel wrote: >>> Hi guys, >>> >>> In the hisi_sas driver there are times in which we need to issue an ATA >>> software reset. For this we use hisi_sas_softreset_ata_disk() -> >>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow >>> task" >>> mechanism to issue the command. >>> >>> I would like if libata provided such a function to issue a software >>> reset, >>> such that we can send the command as an ATA queued command. >>> >>> The problem is that often when we would want to issue this software >>> reset >>> the associated ata port is frozen, like in ATA EH, and so we cannot >>> issue >>> ATA queued commands - internal or normal - at that time. >>> >>> Is there any way to solve this? Or I am just misunderstanding how and >>> when >>> ATA queued commands can and should be used? >>> >> Hello John, > > Hi Niklas, > >> >> See the kdoc above __ata_port_freeze(): >> "This function is called when HSM violation or some other >> condition disrupts normal operation of the port. Frozen port >> is not allowed to perform any operation until the port is >> thawed, which usually follows a successful reset. > > ok, I see. > >> >> ap->ops->freeze() callback can be used for freezing the port >> hardware-wise (e.g. mask interrupt and stop DMA engine). If a >> port cannot be frozen hardware-wise, the interrupt handler >> must ack and clear interrupts unconditionally while the port >> is frozen." >> >> >> ata_port_operations.qc_issue() is obviously an operation on the port, >> so it makes sense that it is not allowed. > > hmmm..ok, then. > > >> Interrupts are also usually masked or disabled at this time, so we >> won't get an IRQ with the completion. > > Doesn't this policy really just depend on the host controller driver? > >> >> Perhaps one could argue that there could be an API to execute a polled >> command. But if the port is in a bad state, > e.g. a HSM error (RDY bit >> is not set), issuing a command would likely fail anyway, regardless if >> using polling or IRQs. >> >> >>> I assume that ata_port_operations.softreset callback requires a >>> method to be >>> able to issue the softreset directly from the driver, like >>> ahci_softreset() >>> -> ahci_do_softreset() -> ahci_exec_polled_cmd(). >> Yes, looking .softreset in a few ata drivers, they all seem issue the >> softreset directly from the driver. >> (e.g. ahci_do_softreset() calls ahci_exec_polled_cmd() which just always >> uses bit 0 in PORT_CMD_ISSUE, so it ignores hw_tag.) >> >> But I don't think that I fully understand your problem. >> >> hisi_sas_softreset_ata_disk() -> sas_execute_ata_cmd() -> >> sas_execute_tmf() >> calls lldd_execute_task() (hisi_sas_queue_command()) and then calls >> waits for completion. >> >> How is this different from e.g. the libahci case? > > The difference really comes down to the controller programming interface. > > For ahci we have a MMIO interface to issue the software reset command. > > For my SAS controller of interest, there is no such MMIO interface. To > issue the reset we build a h2d fis with a SRST set, and send on the > controller ring buffer like any other IO. > > As I mentioned, we can set the SRST for the h2d fis on the HW interface > without issue, and it works fine. The problem for me is that the command > comes via libsas/driver, and I would like it to come from libata such > that it has a ATA queued command associated. But then we have the > problem that the port is frozen at such times that we want to issue this > command. Yeah, qc is too high level for this to work. But we could probably do something generic at TF or FIS level. libata-sata.c has already some code in that area, something for a "reset TF/FIS" would fit in that file too. libahci could also use that too. > >> Doesn't this end up being the same as resetting the port directly from >> the >> driver? (if we ignore all the callbacks) >> Or do you actually get stuck on a ata_port_is_frozen() check somewhere? > > > Thanks, > John -- Damien Le Moal Western Digital Research
