2023-10-05 19:25:13

by Ondrej Zary

[permalink] [raw]
Subject: [PATCH v3 0/4] ata: pata_parport: fix EXP Computer CD-865 with MC-1285B EPP cable


Two bugfixes, one workaround and IDE command set registers support for fit3
driver. All needed for EXP Computer CD-865 drive with MC-1285B EPP cable to
work.

Signed-off-by: Ondrej Zary <[email protected]>
---
Changes in v3:
- add missing stable tag to first two patches
- drop cont_map in patch 4

drivers/ata/pata_parport/fit3.c | 14 +-----
drivers/ata/pata_parport/pata_parport.c | 77 ++++++++++++++++++++++++++++++++-
2 files changed, 77 insertions(+), 14 deletions(-)

--
Ondrej Zary


2023-10-05 19:25:14

by Ondrej Zary

[permalink] [raw]
Subject: [PATCH 4/4] ata: pata_parport: fit3: implement IDE command set registers

fit3 protocol driver does not support accessing IDE control registers
(device control/altstatus). The DOS driver does not use these registers
either (as observed from DOSEMU trace). But the HW seems to be capable
of accessing these registers - I simply tried bit 3 and it works!

The control register is required to properly reset ATAPI devices or
they will be detected only once (after a power cycle).

Tested with EXP Computer CD-865 with MC-1285B EPP cable and
TransDisk 3000.

Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/ata/pata_parport/fit3.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/pata_parport/fit3.c b/drivers/ata/pata_parport/fit3.c
index bad7aa920cdc..d2b81cf2e16d 100644
--- a/drivers/ata/pata_parport/fit3.c
+++ b/drivers/ata/pata_parport/fit3.c
@@ -9,11 +9,6 @@
*
* The TD-2000 and certain older devices use a different protocol.
* Try the fit2 protocol module with them.
- *
- * NB: The FIT adapters do not appear to support the control
- * registers. So, we map ALT_STATUS to STATUS and NO-OP writes
- * to the device control register - this means that IDE reset
- * will not work on these devices.
*/

#include <linux/module.h>
@@ -37,8 +32,7 @@

static void fit3_write_regr(struct pi_adapter *pi, int cont, int regr, int val)
{
- if (cont == 1)
- return;
+ regr += cont << 3;

switch (pi->mode) {
case 0:
@@ -59,11 +53,7 @@ static int fit3_read_regr(struct pi_adapter *pi, int cont, int regr)
{
int a, b;

- if (cont) {
- if (regr != 6)
- return 0xff;
- regr = 7;
- }
+ regr += cont << 3;

switch (pi->mode) {
case 0:
--
Ondrej Zary

2023-10-05 19:25:14

by Ondrej Zary

[permalink] [raw]
Subject: [PATCH 1/4] ata: pata_parport: fix pata_parport_devchk

There's a 'x' missing in 0x55 in pata_parport_devchk(), causing the
detection to always fail. Fix it.

Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
Cc: [email protected]
Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/ata/pata_parport/pata_parport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
index 1af64d435d3c..258d189f42e5 100644
--- a/drivers/ata/pata_parport/pata_parport.c
+++ b/drivers/ata/pata_parport/pata_parport.c
@@ -64,7 +64,7 @@ static bool pata_parport_devchk(struct ata_port *ap, unsigned int device)
pi->proto->write_regr(pi, 0, ATA_REG_NSECT, 0xaa);
pi->proto->write_regr(pi, 0, ATA_REG_LBAL, 0x55);

- pi->proto->write_regr(pi, 0, ATA_REG_NSECT, 055);
+ pi->proto->write_regr(pi, 0, ATA_REG_NSECT, 0x55);
pi->proto->write_regr(pi, 0, ATA_REG_LBAL, 0xaa);

nsect = pi->proto->read_regr(pi, 0, ATA_REG_NSECT);
--
Ondrej Zary

2023-10-05 19:26:35

by Ondrej Zary

[permalink] [raw]
Subject: [PATCH 2/4] ata: pata_parport: implement set_devctl

Add missing ops->sff_set_devctl implementation.

Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
Cc: [email protected]
Reviewed-by: Sergey Shtylyov <[email protected]>
Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/ata/pata_parport/pata_parport.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
index 258d189f42e5..cf87bbb52f1f 100644
--- a/drivers/ata/pata_parport/pata_parport.c
+++ b/drivers/ata/pata_parport/pata_parport.c
@@ -51,6 +51,13 @@ static void pata_parport_dev_select(struct ata_port *ap, unsigned int device)
ata_sff_pause(ap);
}

+static void pata_parport_set_devctl(struct ata_port *ap, u8 ctl)
+{
+ struct pi_adapter *pi = ap->host->private_data;
+
+ pi->proto->write_regr(pi, 1, 6, ctl);
+}
+
static bool pata_parport_devchk(struct ata_port *ap, unsigned int device)
{
struct pi_adapter *pi = ap->host->private_data;
@@ -252,6 +259,7 @@ static struct ata_port_operations pata_parport_port_ops = {
.hardreset = NULL,

.sff_dev_select = pata_parport_dev_select,
+ .sff_set_devctl = pata_parport_set_devctl,
.sff_check_status = pata_parport_check_status,
.sff_check_altstatus = pata_parport_check_altstatus,
.sff_tf_load = pata_parport_tf_load,
--
Ondrej Zary

2023-10-05 19:42:04

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 1/4] ata: pata_parport: fix pata_parport_devchk

On 10/5/23 10:24 PM, Ondrej Zary wrote:

> There's a 'x' missing in 0x55 in pata_parport_devchk(), causing the
> detection to always fail. Fix it.
>
> Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
> Cc: [email protected]
> Signed-off-by: Ondrej Zary <[email protected]>

Reviewed-by: Sergey Shtylyov <[email protected]>

[...]

MBR, Sergey

2023-10-05 19:45:47

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 1/4] ata: pata_parport: fix pata_parport_devchk

On 10/5/23 10:41 PM, Sergei Shtylyov wrote:
[...]

>> There's a 'x' missing in 0x55 in pata_parport_devchk(), causing the
>> detection to always fail. Fix it.
>>
>> Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
>> Cc: [email protected]
>> Signed-off-by: Ondrej Zary <[email protected]>
>
> Reviewed-by: Sergey Shtylyov <[email protected]>

Oops, replied from a wrong account... :-/

> [...]

MBR, Sergey

2023-10-05 19:54:05

by Sergey Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 4/4] ata: pata_parport: fit3: implement IDE command set registers

On 10/5/23 10:24 PM, Ondrej Zary wrote:

> fit3 protocol driver does not support accessing IDE control registers
> (device control/altstatus). The DOS driver does not use these registers
> either (as observed from DOSEMU trace). But the HW seems to be capable
> of accessing these registers - I simply tried bit 3 and it works!
>
> The control register is required to properly reset ATAPI devices or
> they will be detected only once (after a power cycle).
>
> Tested with EXP Computer CD-865 with MC-1285B EPP cable and
> TransDisk 3000.
>
> Signed-off-by: Ondrej Zary <[email protected]>

Reviewed-by: Sergey Shtylyov <[email protected]>

> ---
> drivers/ata/pata_parport/fit3.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/ata/pata_parport/fit3.c b/drivers/ata/pata_parport/fit3.c
> index bad7aa920cdc..d2b81cf2e16d 100644
> --- a/drivers/ata/pata_parport/fit3.c
> +++ b/drivers/ata/pata_parport/fit3.c
[...]
> @@ -59,11 +53,7 @@ static int fit3_read_regr(struct pi_adapter *pi, int cont, int regr)
> {
> int a, b;
>
> - if (cont) {
> - if (regr != 6)
> - return 0xff;
> - regr = 7;
> - }
> + regr += cont << 3;

So, is regr == 7 instead of 8 intentional?

[...]

MBR, Sergey

2023-10-05 20:46:54

by Ondrej Zary

[permalink] [raw]
Subject: Re: [PATCH 4/4] ata: pata_parport: fit3: implement IDE command set registers

On Thursday 05 October 2023 21:53:44 Sergey Shtylyov wrote:
> On 10/5/23 10:24 PM, Ondrej Zary wrote:
>
> > fit3 protocol driver does not support accessing IDE control registers
> > (device control/altstatus). The DOS driver does not use these registers
> > either (as observed from DOSEMU trace). But the HW seems to be capable
> > of accessing these registers - I simply tried bit 3 and it works!
> >
> > The control register is required to properly reset ATAPI devices or
> > they will be detected only once (after a power cycle).
> >
> > Tested with EXP Computer CD-865 with MC-1285B EPP cable and
> > TransDisk 3000.
> >
> > Signed-off-by: Ondrej Zary <[email protected]>
>
> Reviewed-by: Sergey Shtylyov <[email protected]>
>
> > ---
> > drivers/ata/pata_parport/fit3.c | 14 ++------------
> > 1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/ata/pata_parport/fit3.c b/drivers/ata/pata_parport/fit3.c
> > index bad7aa920cdc..d2b81cf2e16d 100644
> > --- a/drivers/ata/pata_parport/fit3.c
> > +++ b/drivers/ata/pata_parport/fit3.c
> [...]
> > @@ -59,11 +53,7 @@ static int fit3_read_regr(struct pi_adapter *pi, int cont, int regr)
> > {
> > int a, b;
> >
> > - if (cont) {
> > - if (regr != 6)
> > - return 0xff;
> > - regr = 7;
> > - }
> > + regr += cont << 3;
>
> So, is regr == 7 instead of 8 intentional?

The original code read ATA_REG_STATUS (7) in context 0 instead of ALT_STATUS (6) in context 1.

--
Ondrej Zary