Re: [PATCH v4 2/3] ata: libata-eh: Simplify reset operation management

[Niklas Cassel <cassel@kernel.org>]

On Mon, Jul 14, 2025 at 09:54:53AM +0900, Damien Le Moal wrote:
> Introduce struct ata_reset_operations to aggregate in a single structure
> the definitions of the 4 reset methods (prereset, softreset, hardreset
> and postreset) for a port. This new structure is used in struct ata_port
> to define the reset methods for a regular port (reset field) and for a
> port-multiplier port (pmp_reset field). A pointer to either of these
> fields replaces the 4 reset method arguments passed to ata_eh_recover()
> and ata_eh_reset().
> 
> The definition of the reset methods for all drivers is changed to use
> the reset and pmp_reset fields in struct ata_port_operations.
> 
> A large number of files is modifed, but no functional changes are
> introduced.
> 
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---

(snip)

> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> index 93ebf566b54e..8de63d889a68 100644
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
> @@ -321,8 +321,7 @@ static void pata_parport_drain_fifo(struct ata_queued_cmd *qc)
>  static struct ata_port_operations pata_parport_port_ops = {
>  	.inherits		= &ata_sff_port_ops,
>  
> -	.softreset		= pata_parport_softreset,
> -	.hardreset		= NULL,

I think you need to add .reset.hardreset = NULL, because pata_parport_port_ops
inherits ata_sff_port_ops, which does set hardreset, so I think this line will
clear the pointer.


> +	.reset.softreset	= pata_parport_softreset,
>  
>  	.sff_dev_select		= pata_parport_dev_select,
>  	.sff_set_devctl		= pata_parport_set_devctl,


Rest looks good to me:
Reviewed-by: Niklas Cassel <cassel@kernel.org>


Tell me if you want to fix it up when applying or if you want to send a new
version.


Kind regards,
Niklas