| Message ID | 20230930191511.24994-4-linux@zary.sk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pata_parport: fix EXP Computer CD-865 with MC-1285B EPP cable | expand |
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@zary.sk> > --- > 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
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, + * 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 + * 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 + * to set nsect/lbal after reset, so don't waste too + * much time on it. We're gonna wait for !BSY anyway. + */ + for (i = 0; i < 2; i++) { + u8 nsect, lbal; + + nsect = pi->proto->read_regr(pi, 0, ATA_REG_NSECT); + lbal = pi->proto->read_regr(pi, 0, ATA_REG_LBAL); + if ((nsect == 1) && (lbal == 1)) + break; + ata_msleep(ap, 50); /* give drive a breather */ + } + + rc = ata_sff_wait_ready(link, deadline); + if (rc) { + if (rc != -ENODEV) + return rc; + ret = rc; + } + } + + /* is all this really necessary? */ + pata_parport_dev_select(ap, 0); + if (dev1) + pata_parport_dev_select(ap, 1); + if (dev0) + pata_parport_dev_select(ap, 0); + + return ret; +} + static int pata_parport_bus_softreset(struct ata_port *ap, unsigned int devmask, unsigned long deadline) { @@ -94,7 +157,7 @@ static int pata_parport_bus_softreset(struct ata_port *ap, unsigned int devmask, ap->last_ctl = ap->ctl; /* wait the port to become ready */ - return ata_sff_wait_after_reset(&ap->link, devmask, deadline); + return pata_parport_wait_after_reset(&ap->link, devmask, deadline); } static int pata_parport_softreset(struct ata_link *link, unsigned int *classes,
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@zary.sk> --- drivers/ata/pata_parport/pata_parport.c | 65 ++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-)