On 5/11/23 07:52, Hannes Reinecke wrote:
> With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all
> libata drivers now have the error_handler() callback provided,
> so we can stop checking for non-existing error_handler callback.
>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> ---
> drivers/ata/libata-core.c | 158 ++++++++++++++------------------------
> drivers/ata/libata-eh.c | 148 +++++++++++++++--------------------
> drivers/ata/libata-sata.c | 5 +-
> drivers/ata/libata-scsi.c | 21 ++---
> drivers/ata/libata-sff.c | 32 +++-----
> 5 files changed, 139 insertions(+), 225 deletions(-)
>
[...]
> - case ATA_CMD_SLEEP:
> - dev->flags |= ATA_DFLAG_SLEEPING;
> + trace_ata_qc_complete_done(qc);
> + /* Some commands need post-processing after successful
> + * completion.
> + */
While at it, can you fix this comment style ?
It may actually fit on one line...
[...]
> + /* Exception might have happened after ->error_handler
> + * recovered the port but before this point. Repeat
> + * EH in such case.
> + */
Same here. Multi-line comment format...
[...]
> - /* end eh (clear host_eh_scheduled) while holding
> - * ap->lock such that if exception occurs after this
> - * point but before EH completion, SCSI midlayer will
> - * re-initiate EH.
> - */
> - ap->ops->end_eh(ap);
> + /* end eh (clear host_eh_scheduled) while holding
> + * ap->lock such that if exception occurs after this
> + * point but before EH completion, SCSI midlayer will
> + * re-initiate EH.
> + */
And here.
[...]
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index f3e7396e3191..bd2a754b645c 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1138,11 +1138,8 @@ EXPORT_SYMBOL_GPL(ata_sas_port_alloc);
> int ata_sas_port_start(struct ata_port *ap)
> {
> /*
> - * the port is marked as frozen at allocation time, but if we don't
> - * have new eh, we won't thaw it
> + * the port is marked as frozen at allocation time
> */
This one can be a single line comment, no ?
[...]
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 9d28badfe41d..aa286d48e847 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -883,31 +883,23 @@ static void ata_hsm_qc_complete(struct ata_queued_cmd *qc, int in_wq)
> {
> struct ata_port *ap = qc->ap;
>
> - if (ap->ops->error_handler) {
> - if (in_wq) {
> - /* EH might have kicked in while host lock is
> - * released.
> - */
> - qc = ata_qc_from_tag(ap, qc->tag);
> - if (qc) {
> - if (likely(!(qc->err_mask & AC_ERR_HSM))) {
> - ata_sff_irq_on(ap);
> - ata_qc_complete(qc);
> - } else
> - ata_port_freeze(ap);
> - }
> - } else {
> - if (likely(!(qc->err_mask & AC_ERR_HSM)))
> + if (in_wq) {
> + /* EH might have kicked in while host lock is
> + * released.
> + */
Comment format again.
Also, please add linux-scsi and John to the distribution list when resending.
--
Damien Le Moal
Western Digital Research
