Re: [PATCH v3 4/8] ata: libata: Add ata_force_get_fe_for_dev() helper

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

On 12/1/25 21:35, Niklas Cassel wrote:
> Add ata_force_get_fe_for_dev() helper to get the struct ata_force_ent for
> a struct ata_device.
> 
> Use the helper in ata_force_quirks().
> The helper will also be used in follow up commits.
> 
> No functional change intended.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

A few nits below. With these addressed, feel free to add:

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> -/**
> - *	ata_force_quirks - force quirks according to libata.force
> - *	@dev: ATA device of interest
> - *
> - *	Force quirks according to libata.force and whine about it.
> - *	For consistency with link selection, device number 15 selects
> - *	the first device connected to the host link.
> - *
> - *	LOCKING:
> - *	EH context.
> - */
> -static void ata_force_quirks(struct ata_device *dev)
> +static const struct ata_force_ent
> +*ata_force_get_fe_for_dev(struct ata_device *dev)

The "*" should be on the previous line as it is part of the type definition for
the return value.

>  {
>  	int devno = dev->link->pmp + dev->devno;
>  	int alt_devno = devno;
>  	int i;
> +	const struct ata_force_ent *fe;

Nit: move this as the first declaration so that there is not a long declaration
line after a short one. Yes, this is 100% visual only, not a bug :)

>  
>  	/* allow n.15/16 for devices attached to host port */
>  	if (ata_is_host_link(dev->link))
>  		alt_devno += 15;
>  
>  	for (i = 0; i < ata_force_tbl_size; i++) {
> -		const struct ata_force_ent *fe = &ata_force_tbl[i];
> +		fe = &ata_force_tbl[i];
>  

Maybe drop the blank line here ?

>  		if (fe->port != -1 && fe->port != dev->link->ap->print_id)
>  			continue;



-- 
Damien Le Moal
Western Digital Research