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

[Damien Le Moal <dlemoal@kernel.org>]

On 2025/07/14 16:56, Niklas Cassel wrote:
> 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.

If you can fix it up when applying, that would be great. Thanks.

> 
> 
> Kind regards,
> Niklas


-- 
Damien Le Moal
Western Digital Research