Hi,
After several iterations I have now finally made my PCIe parallel port
option card based on the Oxford Semiconductor OXPCIe952 ASIC work with my
RISC-V system.
This has turned out a generic incompatibility issue between our driver
and somewhat quirky OxSemi hardware giving the same symptoms with an x86
system as well, where the driver tries to use the Parallel Port FIFO mode,
which is indeed documented by the relevant datasheets as not supported by
OxSemi hardware.
Additionally the hardware requires a specific bit pattern to be written
into low 5 bits of ECR while poking at the mode in the high 3 bits, and
while the OXPCIe952 implementation does not appear sensitive to it older
ones seem to and may lock up according to our own history.
This small patch series addresses these problems in 6 incremental steps.
See individual change descriptions for further details. I have verified
these changes with and w/o PARPORT_PC_FIFO and PARPORT_1284 options set.
Please apply.
Maciej
Add a mode mask field for PCI devices and use `__parport_pc_probe_port'
in place of `parport_pc_probe_port' to apply it.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/parport/parport_pc.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
linux-parport-pc-pci-mode-mask.diff
Index: linux-macro/drivers/parport/parport_pc.c
===================================================================
--- linux-macro.orig/drivers/parport/parport_pc.c
+++ linux-macro/drivers/parport/parport_pc.c
@@ -2659,6 +2659,9 @@ static struct parport_pc_pci {
/* -1 if not there, >6 for offset-method (max BAR is 6) */
} addr[4];
+ /* Bit field of parport modes to exclude. */
+ unsigned int mode_mask;
+
/* If set, this is called immediately after pci_enable_device.
* If it returns non-zero, no probing will take place and the
* ports will not be used. */
@@ -2862,9 +2865,10 @@ static int parport_pc_pci_probe(struct p
id->vendor, id->device, io_lo, io_hi, irq);
}
data->ports[count] =
- parport_pc_probe_port(io_lo, io_hi, irq,
- PARPORT_DMA_NONE, &dev->dev,
- IRQF_SHARED);
+ __parport_pc_probe_port(io_lo, io_hi, irq,
+ PARPORT_DMA_NONE, &dev->dev,
+ IRQF_SHARED,
+ cards[i].mode_mask, 0);
if (data->ports[count])
count++;
}
Rename `parport_pc_probe_port' to `__parport_pc_probe_port' and add a
`mode_mask' parameter so that callers can specify a mask of unsupported
modes to exclude even if mode probing seems to indicate otherwise. Add
a `parport_pc_probe_port' wrapper with an implicit mask of 0 for the
current callers to use.
No functional change at this point, but the configuration of data write
handlers is now no longer intertwined with determination and reporting
of available modes.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/parport/parport_pc.c | 45 +++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 12 deletions(-)
linux-parport-pc-mode-mask.diff
Index: linux-macro/drivers/parport/parport_pc.c
===================================================================
--- linux-macro.orig/drivers/parport/parport_pc.c
+++ linux-macro/drivers/parport/parport_pc.c
@@ -2000,11 +2000,12 @@ static int parport_dma_probe(struct parp
static LIST_HEAD(ports_list);
static DEFINE_SPINLOCK(ports_lock);
-struct parport *parport_pc_probe_port(unsigned long int base,
- unsigned long int base_hi,
- int irq, int dma,
- struct device *dev,
- int irqflags)
+static struct parport *__parport_pc_probe_port(unsigned long int base,
+ unsigned long int base_hi,
+ int irq, int dma,
+ struct device *dev,
+ int irqflags,
+ unsigned int mode_mask)
{
struct parport_pc_private *priv;
struct parport_operations *ops;
@@ -2116,18 +2117,28 @@ struct parport *parport_pc_probe_port(un
p->dma != PARPORT_DMA_NOFIFO &&
priv->fifo_depth > 0 && p->irq != PARPORT_IRQ_NONE) {
p->modes |= PARPORT_MODE_ECP | PARPORT_MODE_COMPAT;
+ if (p->dma != PARPORT_DMA_NONE)
+ p->modes |= PARPORT_MODE_DMA;
+ } else
+ /* We can't use the DMA channel after all. */
+ p->dma = PARPORT_DMA_NONE;
+#endif /* Allowed to use FIFO/DMA */
+
+ p->modes &= ~mode_mask;
+
+#ifdef CONFIG_PARPORT_PC_FIFO
+ if ((p->modes & PARPORT_MODE_COMPAT) != 0)
p->ops->compat_write_data = parport_pc_compat_write_block_pio;
#ifdef CONFIG_PARPORT_1284
+ if ((p->modes & PARPORT_MODE_ECP) != 0)
p->ops->ecp_write_data = parport_pc_ecp_write_block_pio;
-#endif /* IEEE 1284 support */
- if (p->dma != PARPORT_DMA_NONE) {
+#endif
+ if ((p->modes & (PARPORT_MODE_ECP | PARPORT_MODE_COMPAT)) != 0) {
+ if ((p->modes & PARPORT_MODE_DMA) != 0)
pr_cont(", dma %d", p->dma);
- p->modes |= PARPORT_MODE_DMA;
- } else
+ else
pr_cont(", using FIFO");
- } else
- /* We can't use the DMA channel after all. */
- p->dma = PARPORT_DMA_NONE;
+ }
#endif /* Allowed to use FIFO/DMA */
pr_cont(" [");
@@ -2237,6 +2248,16 @@ do { \
platform_device_unregister(pdev);
return NULL;
}
+
+struct parport *parport_pc_probe_port(unsigned long int base,
+ unsigned long int base_hi,
+ int irq, int dma,
+ struct device *dev,
+ int irqflags)
+{
+ return __parport_pc_probe_port(base, base_hi, irq, dma,
+ dev, irqflags, 0);
+}
EXPORT_SYMBOL(parport_pc_probe_port);
void parport_pc_unregister_port(struct parport *p)
Provide an `ecr_writable' parameter to `__parport_pc_probe_port' so that
callers can specify a mask of bits to modify on ECR writes.
To avoid the need for separate bit set and bit clear masks always set
bit 0 whenever a non-zero mask has been set, as all the currently known
cases where a mask is required, that is Oxford Semiconductor devices, do
require this bit to be set. If further cases are discovered where the
bit is required to be clear, we can update code accordingly, but chances
are very low as the bit is supposed to be read-only[1].
Skip ECR probing, which can be problematic as the Oxford Semiconductor
OX12PCI840 part has been reported to lock up on setting bit 2, whenever
a non-zero mask has been requested by a port subdriver, assuming that
the ECR must be there if the subdriver has requested a specific way to
access it.
References:
[1] "Extended Capabilities Port Protocol and ISA Interface Standard",
Microsoft Corporation, Revision: 1.14, July 14, 1993, Table 14
"Extended Control Register"
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/parport/parport_pc.c | 46 +++++++++++++++++++++++++++----------------
include/linux/parport_pc.h | 3 ++
2 files changed, 32 insertions(+), 17 deletions(-)
linux-parport-pc-ecr-write.diff
Index: linux-macro/drivers/parport/parport_pc.c
===================================================================
--- linux-macro.orig/drivers/parport/parport_pc.c
+++ linux-macro/drivers/parport/parport_pc.c
@@ -106,15 +106,22 @@ static int pnp_registered_parport;
static void frob_econtrol(struct parport *pb, unsigned char m,
unsigned char v)
{
+ const struct parport_pc_private *priv = pb->physport->private_data;
+ unsigned char ecr_writable = priv->ecr_writable;
unsigned char ectr = 0;
+ unsigned char new;
if (m != 0xff)
ectr = inb(ECONTROL(pb));
- pr_debug("frob_econtrol(%02x,%02x): %02x -> %02x\n",
- m, v, ectr, (ectr & ~m) ^ v);
+ new = (ectr & ~m) ^ v;
+ if (ecr_writable)
+ /* All known users of the ECR mask require bit 0 to be set. */
+ new = (new & ecr_writable) | 1;
- outb((ectr & ~m) ^ v, ECONTROL(pb));
+ pr_debug("frob_econtrol(%02x,%02x): %02x -> %02x\n", m, v, ectr, new);
+
+ outb(new, ECONTROL(pb));
}
static inline void frob_set_mode(struct parport *p, int mode)
@@ -1479,21 +1486,24 @@ static int parport_ECR_present(struct pa
struct parport_pc_private *priv = pb->private_data;
unsigned char r = 0xc;
- outb(r, CONTROL(pb));
- if ((inb(ECONTROL(pb)) & 0x3) == (r & 0x3)) {
- outb(r ^ 0x2, CONTROL(pb)); /* Toggle bit 1 */
+ if (!priv->ecr_writable) {
+ outb(r, CONTROL(pb));
+ if ((inb(ECONTROL(pb)) & 0x3) == (r & 0x3)) {
+ outb(r ^ 0x2, CONTROL(pb)); /* Toggle bit 1 */
- r = inb(CONTROL(pb));
- if ((inb(ECONTROL(pb)) & 0x2) == (r & 0x2))
- goto no_reg; /* Sure that no ECR register exists */
- }
+ r = inb(CONTROL(pb));
+ if ((inb(ECONTROL(pb)) & 0x2) == (r & 0x2))
+ /* Sure that no ECR register exists */
+ goto no_reg;
+ }
- if ((inb(ECONTROL(pb)) & 0x3) != 0x1)
- goto no_reg;
+ if ((inb(ECONTROL(pb)) & 0x3) != 0x1)
+ goto no_reg;
- ECR_WRITE(pb, 0x34);
- if (inb(ECONTROL(pb)) != 0x35)
- goto no_reg;
+ ECR_WRITE(pb, 0x34);
+ if (inb(ECONTROL(pb)) != 0x35)
+ goto no_reg;
+ }
priv->ecr = 1;
outb(0xc, CONTROL(pb));
@@ -2005,7 +2015,8 @@ static struct parport *__parport_pc_prob
int irq, int dma,
struct device *dev,
int irqflags,
- unsigned int mode_mask)
+ unsigned int mode_mask,
+ unsigned char ecr_writable)
{
struct parport_pc_private *priv;
struct parport_operations *ops;
@@ -2054,6 +2065,7 @@ static struct parport *__parport_pc_prob
priv->ctr = 0xc;
priv->ctr_writable = ~0x10;
priv->ecr = 0;
+ priv->ecr_writable = ecr_writable;
priv->fifo_depth = 0;
priv->dma_buf = NULL;
priv->dma_handle = 0;
@@ -2256,7 +2268,7 @@ struct parport *parport_pc_probe_port(un
int irqflags)
{
return __parport_pc_probe_port(base, base_hi, irq, dma,
- dev, irqflags, 0);
+ dev, irqflags, 0, 0);
}
EXPORT_SYMBOL(parport_pc_probe_port);
Index: linux-macro/include/linux/parport_pc.h
===================================================================
--- linux-macro.orig/include/linux/parport_pc.h
+++ linux-macro/include/linux/parport_pc.h
@@ -26,6 +26,9 @@ struct parport_pc_private {
/* Whether or not there's an ECR. */
int ecr;
+ /* Bitmask of writable ECR bits. */
+ unsigned char ecr_writable;
+
/* Number of PWords that FIFO will hold. */
int fifo_depth;
Add a bitmask field specifying writable ECR bits for PCI devices and
apply it via `__parport_pc_probe_port'.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/parport/parport_pc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
linux-parport-pc-pci-ecr-write.diff
Index: linux-macro/drivers/parport/parport_pc.c
===================================================================
--- linux-macro.orig/drivers/parport/parport_pc.c
+++ linux-macro/drivers/parport/parport_pc.c
@@ -2662,6 +2662,10 @@ static struct parport_pc_pci {
/* Bit field of parport modes to exclude. */
unsigned int mode_mask;
+ /* If non-zero, sets the bitmask of writable ECR bits. In that
+ * case additionally bit 0 will be forcibly set on writes. */
+ unsigned char ecr_writable;
+
/* If set, this is called immediately after pci_enable_device.
* If it returns non-zero, no probing will take place and the
* ports will not be used. */
@@ -2868,7 +2872,8 @@ static int parport_pc_pci_probe(struct p
__parport_pc_probe_port(io_lo, io_hi, irq,
PARPORT_DMA_NONE, &dev->dev,
IRQF_SHARED,
- cards[i].mode_mask, 0);
+ cards[i].mode_mask,
+ cards[i].ecr_writable);
if (data->ports[count])
count++;
}
Complement commit 991214386dee ("parport: remove unused dead code from
lowlevel drivers") and remove a stale piece of commented-out code that
refers to a function removed with said commit.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/parport/parport_pc.c | 2 --
1 file changed, 2 deletions(-)
linux-parport-pc-ecp-read-block-pio-call.diff
Index: linux-macro/drivers/parport/parport_pc.c
===================================================================
--- linux-macro.orig/drivers/parport/parport_pc.c
+++ linux-macro/drivers/parport/parport_pc.c
@@ -2119,8 +2119,6 @@ struct parport *parport_pc_probe_port(un
p->ops->compat_write_data = parport_pc_compat_write_block_pio;
#ifdef CONFIG_PARPORT_1284
p->ops->ecp_write_data = parport_pc_ecp_write_block_pio;
- /* currently broken, but working on it.. (FB) */
- /* p->ops->ecp_read_data = parport_pc_ecp_read_block_pio; */
#endif /* IEEE 1284 support */
if (p->dma != PARPORT_DMA_NONE) {
pr_cont(", dma %d", p->dma);
On Wed, 16 Nov 2022, Maciej W. Rozycki wrote:
> This small patch series addresses these problems in 6 incremental steps.
> See individual change descriptions for further details. I have verified
> these changes with and w/o PARPORT_PC_FIFO and PARPORT_1284 options set.
> Please apply.
Ping for:
<https://lore.kernel.org/lkml/[email protected]/>.
Maciej