Re: [PATCH 1/2] parport_pc: after superio probing restore original register values

Re: [PATCH 1/2] parport_pc: after superio probing restore original register values

[Jeff Garzik]

Alan Cox wrote:
> From: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
> 
> CONFIG_PARPORT_PC_SUPERIO probes for various superio chips by writing
> byte sequences to a set of different potential I/O ranges.  But the
> probed ranges are not exclusive to parallel ports.  Some of our boards
> just happen to have a watchdog in one of them.  Took us almost a week
> to figure out why some distros reboot without warning after running
> flawlessly for 3 hours.  For exactly 170 = 0xAA minutes, that is ...
> 
> Fixed by restoring original values after probing.  Also fixed too small
> request_region() in detect_and_report_it87().
> 
> Signed-off-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> Cc: <stable@kernel.org>
> ---
> 
>  drivers/parport/parport_pc.c |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)

Acked-by: Jeff Garzik <jgarzik@redhat.com>

Re: [PATCH 2/2] parport_pc: set properly the dma_mask for parport_pc device

[Jeff Garzik]

Alan Cox wrote:
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> 
> parport_pc_probe_port() creates the own 'parport_pc' device if the
> device argument is NULL. Then parport_pc_probe_port() doesn't
> initialize the dma_mask and coherent_dma_mask of the device and calls
> dma_alloc_coherent with it. dma_alloc_coherent fails because
> dma_alloc_coherent() doesn't accept the uninitialized dma_mask:
> 
> http://lkml.org/lkml/2009/6/16/150
> 
> Long ago, X86_32 and X86_64 had the own dma_alloc_coherent
> implementations; X86_32 accepted a device having dma_mask that is not
> initialized however X86_64 didn't. When we merged them, we chose to
> prohibit a device having dma_mask that is not initialized. I think
> that it's good to require drivers to set up dma_mask (and
> coherent_dma_mask) properly if the drivers want DMA.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Reported-by: Malcom Blaney <malcolm.blaney@maptek.com.au>
> Tested-by: Malcom Blaney <malcolm.blaney@maptek.com.au>
> Cc: stable@kernel.org
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> ---
> 
>  drivers/parport/parport_pc.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Jeff Garzik <jgarzik@redhat.com>

[PATCH 0/2] Parallel port fixes for 2.6.31

[Alan Cox]

---

FUJITA Tomonori (1):
      parport_pc: set properly the dma_mask for parport_pc device

Jens Rottmann (1):
      parport_pc: after superio probing restore original register values


 drivers/parport/parport_pc.c            |   34 +

[PATCH 1/2] parport_pc: after superio probing restore original register values

[Alan Cox]

From: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>

CONFIG_PARPORT_PC_SUPERIO probes for various superio chips by writing
byte sequences to a set of different potential I/O ranges.  But the
probed ranges are not exclusive to parallel ports.  Some of our boards
just happen to have a watchdog in one of them.  Took us almost a week
to figure out why some distros reboot without warning after running
flawlessly for 3 hours.  For exactly 170 = 0xAA minutes, that is ...

Fixed by restoring original values after probing.  Also fixed too small
request_region() in detect_and_report_it87().

Signed-off-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: <stable@kernel.org>
---

 drivers/parport/parport_pc.c |   31 +++++++++++++++++++++++++------
 1 files changed, 25 insertions(+), 6 deletions(-)


diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 151bf5b..7f1cca7 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -1471,11 +1471,13 @@ static void __devinit decode_smsc(int efer, int key, int devid, int devrev)
 
 static void __devinit winbond_check(int io, int key)
 {
-	int devid, devrev, oldid, x_devid, x_devrev, x_oldid;
+	int origval, devid, devrev, oldid, x_devid, x_devrev, x_oldid;
 
 	if (!request_region(io, 3, __func__))
 		return;
 
+	origval = inb(io); /* Save original value */
+
 	/* First probe without key */
 	outb(0x20, io);
 	x_devid = inb(io + 1);
@@ -1495,6 +1497,8 @@ static void __devinit winbond_check(int io, int key)
 	oldid = inb(io + 1);
 	outb(0xaa, io);    /* Magic Seal */
 
+	outb(origval, io); /* in case we poked some entirely different hardware */
+
 	if ((x_devid == devid) && (x_devrev == devrev) && (x_oldid == oldid))
 		goto out; /* protection against false positives */
 
@@ -1505,11 +1509,15 @@ out:
 
 static void __devinit winbond_check2(int io, int key)
 {
-	int devid, devrev, oldid, x_devid, x_devrev, x_oldid;
+	int origval[3], devid, devrev, oldid, x_devid, x_devrev, x_oldid;
 
 	if (!request_region(io, 3, __func__))
 		return;
 
+	origval[0] = inb(io); /* Save original values */
+	origval[1] = inb(io + 1);
+	origval[2] = inb(io + 2);
+
 	/* First probe without the key */
 	outb(0x20, io + 2);
 	x_devid = inb(io + 2);
@@ -1528,6 +1536,10 @@ static void __devinit winbond_check2(int io, int key)
 	oldid = inb(io + 2);
 	outb(0xaa, io);    /* Magic Seal */
 
+	outb(origval[0], io); /* in case we poked some entirely different hardware */
+	outb(origval[1], io + 1);
+	outb(origval[2], io + 2);
+
 	if (x_devid == devid && x_devrev == devrev && x_oldid == oldid)
 		goto out; /* protection against false positives */
 
@@ -1538,11 +1550,13 @@ out:
 
 static void __devinit smsc_check(int io, int key)
 {
-	int id, rev, oldid, oldrev, x_id, x_rev, x_oldid, x_oldrev;
+	int origval, id, rev, oldid, oldrev, x_id, x_rev, x_oldid, x_oldrev;
 
 	if (!request_region(io, 3, __func__))
 		return;
 
+	origval = inb(io); /* Save original value */
+
 	/* First probe without the key */
 	outb(0x0d, io);
 	x_oldid = inb(io + 1);
@@ -1566,6 +1580,8 @@ static void __devinit smsc_check(int io, int key)
 	rev = inb(io + 1);
 	outb(0xaa, io);    /* Magic Seal */
 
+	outb(origval, io); /* in case we poked some entirely different hardware */
+
 	if (x_id == id && x_oldrev == oldrev &&
 	    x_oldid == oldid && x_rev == rev)
 		goto out; /* protection against false positives */
@@ -1602,11 +1618,12 @@ static void __devinit detect_and_report_smsc(void)
 static void __devinit detect_and_report_it87(void)
 {
 	u16 dev;
-	u8 r;
+	u8 origval, r;
 	if (verbose_probing)
 		printk(KERN_DEBUG "IT8705 Super-IO detection, now testing port 2E ...\n");
-	if (!request_region(0x2e, 1, __func__))
+	if (!request_region(0x2e, 2, __func__))
 		return;
+	origval = inb(0x2e);		/* Save original value */
 	outb(0x87, 0x2e);
 	outb(0x01, 0x2e);
 	outb(0x55, 0x2e);
@@ -1626,8 +1643,10 @@ static void __devinit detect_and_report_it87(void)
 		outb(r | 8, 0x2F);
 		outb(0x02, 0x2E);	/* Lock */
 		outb(0x02, 0x2F);
+	} else {
+		outb(origval, 0x2e);	/* Oops, sorry to disturb */
 	}
-	release_region(0x2e, 1);
+	release_region(0x2e, 2);
 }
 #endif /* CONFIG_PARPORT_PC_SUPERIO */
 

[PATCH 2/2] parport_pc: set properly the dma_mask for parport_pc device

[Alan Cox]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

parport_pc_probe_port() creates the own 'parport_pc' device if the
device argument is NULL. Then parport_pc_probe_port() doesn't
initialize the dma_mask and coherent_dma_mask of the device and calls
dma_alloc_coherent with it. dma_alloc_coherent fails because
dma_alloc_coherent() doesn't accept the uninitialized dma_mask:

http://lkml.org/lkml/2009/6/16/150

Long ago, X86_32 and X86_64 had the own dma_alloc_coherent
implementations; X86_32 accepted a device having dma_mask that is not
initialized however X86_64 didn't. When we merged them, we chose to
prohibit a device having dma_mask that is not initialized. I think
that it's good to require drivers to set up dma_mask (and
coherent_dma_mask) properly if the drivers want DMA.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Reported-by: Malcom Blaney <malcolm.blaney@maptek.com.au>
Tested-by: Malcom Blaney <malcolm.blaney@maptek.com.au>
Cc: stable@kernel.org
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/parport/parport_pc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 7f1cca7..1032d5f 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -2290,6 +2290,9 @@ struct parport *parport_pc_probe_port(unsigned long int base,
 		if (IS_ERR(pdev))
 			return NULL;
 		dev = &pdev->dev;
+
+		dev->coherent_dma_mask = DMA_BIT_MASK(24);
+		dev->dma_mask = &dev->coherent_dma_mask;
 	}
 
 	ops = kmalloc(sizeof(struct parport_operations), GFP_KERNEL);