Re: [PATCH v2 2/3] ata: libata-eh: Remove ata_do_eh()

[Niklas Cassel <cassel@kernel.org>]

On Tue, Jul 08, 2025 at 02:34:14PM +0200, Niklas Cassel wrote:
> On Tue, Jul 08, 2025 at 04:36:47PM +0900, Damien Le Moal wrote:
> 
> I think it would be nicer to just do:
> 
> if ((hardreset == sata_std_hardreset ||
>      hardreset == sata_sff_hardreset) && !sata_scr_valid(&ap->link))
>      link->flags |= ATA_LFLAG_NO_HRST;
> 
> since ata_eh_reset() will already do
> hardreset = NULL; if that flag is set.
> 
> This way, we can also simplify the function signature of ata_eh_recover() to:
> 
> int ata_eh_recover(struct ata_port *ap, bool use_pmp_ops,
>                    struct ata_link **r_failed_link)
> 
> and ata_eh_reset() to:
> 
> int ata_eh_reset(struct ata_link *link, int classify, bool use_pmp_ops)

Even nicer would be create a new:

ata_reset_port_ops {
        ata_prereset_fn_t       prereset;
        ata_reset_fn_t          softreset;
        ata_reset_fn_t          hardreset;
        ata_postreset_fn_t      postreset;
}

And then modify:

struct ata_port_operations {
	...
	struct ata_reset_port_ops *reset_port_ops;
	struct ata_reset_port_ops *reset_port_pmp_ops;
	...
}


and then modify ata_eh_recover() to:
int ata_eh_recover(struct ata_port *ap, struct ata_reset_port_ops *ops,
                   struct ata_link **r_failed_link)


and then modify ata_eh_reset() to:

int ata_eh_reset(struct ata_link *link, int classify, struct ata_reset_port_ops *ops)


Kind regards,
Niklas