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
