Re: [PATCH 3/4] pata_parport: add custom version of wait_after_reset

[Sergey Shtylyov]

On 9/30/23 10:15 PM, Ondrej Zary wrote:

> Some parallel adapters (e.g. EXP Computer MC-1285B EPP Cable) return
> bogus values when there's no master device present. This can cause
> reset to fail, preventing the lone slave device (such as EXP Computer
> CD-865) from working.
> 
> Add custom version of wait_after_reset that ignores master failure when
> a slave device is present. The custom version is also needed because
> the generic ata_sff_wait_after_reset uses direct port I/O for slave
> device detection.
> 
> Signed-off-by: Ondrej Zary <linux@xxxxxxx>
> ---
>  drivers/ata/pata_parport/pata_parport.c | 65 ++++++++++++++++++++++++-
>  1 file changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> index cf87bbb52f1f..b3db953e615a 100644
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
> @@ -80,6 +80,69 @@ static bool pata_parport_devchk(struct ata_port *ap, unsigned int device)
>  	return (nsect == 0x55) && (lbal == 0xaa);
>  }
>  
> +static int pata_parport_wait_after_reset(struct ata_link *link,
> +					 unsigned int devmask,
> +					 unsigned long deadline)
> +{
> +	struct ata_port *ap = link->ap;
> +	struct pi_adapter *pi = ap->host->private_data;
> +	unsigned int dev0 = devmask & (1 << 0);
> +	unsigned int dev1 = devmask & (1 << 1);
> +	int rc, ret = 0;
> +
> +	ata_msleep(ap, ATA_WAIT_AFTER_RESET);
> +
> +	/* always check readiness of the master device */
> +	rc = ata_sff_wait_ready(link, deadline);
> +	/* some adapters return bogus values if master device is not present,

   The multiline comments should start with /* on its own line.
   Have you run scripts/checkpatch.pl on the patches?

> +	 * so don't abort now if a slave device is present
> +	 */
> +	if (rc) {
> +		if (!dev1)
> +			return rc;
> +		ret = -ENODEV;
> +	}
> +
> +	/* if device 1 was found in ata_devchk, wait for register

   Likewise here...

> +	 * access briefly, then wait for BSY to clear.
> +	 */
> +	if (dev1) {
> +		int i;
> +
> +		pata_parport_dev_select(ap, 1);
> +
> +		/* Wait for register access.  Some ATAPI devices fail

   And here...

[...]

MBR, Sergey