Re: [PATCH] parport_pc.c PnP BIOS sanity check

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Thomas Hood]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

I'm still wondering why this function in parport_pc.c rejects dma
values of zero.  Is DMA0 not usable by the parallel port for some
reason?  I should think that if the PnP BIOS returns a dma of zero
then it means that the parallel port is using DMA0.  Sorry if I'm
being obtuse.                      // Thomas Hood 

parport_pc.c line 2799:
int init_pnp040x(struct pci_dev *dev)
{       int io,iohi,irq,dma;

        io=dev->resource[0].start;
        iohi=dev->resource[1].start;
        irq=dev->irq_resource[0].start;
        dma=dev->dma_resource[0].start;

        if(dma==0) dma=-1;             <--- Why?

        printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
                dev->name,dev->slot_name,io,iohi,irq,dma);
        if (parport_pc_probe_port(io,iohi,irq,dma,NULL))
                return 1;
        return 0;
}

[-- Attachment #2: patch-norejdma --]
[-- Type: text/plain, Size: 452 bytes --]

--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG	Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c	Mon Aug 13 17:40:38 2001
@@ -2797,8 +2797,6 @@
 	irq=dev->irq_resource[0].start;
 	dma=dev->dma_resource[0].start;
 
-	if (dma==0) dma=-1;
-
 	printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
 		dev->name,dev->slot_name,io,iohi,irq,dma);
 	if (parport_pc_probe_port(io,iohi,irq,dma,NULL))


[-- Attachment #3: patch-dmanone --]
[-- Type: text/plain, Size: 437 bytes --]

--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG	Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c	Fri Aug 10 18:19:55 2001
@@ -2797,7 +2797,7 @@
 	irq=dev->irq_resource[0].start;
 	dma=dev->dma_resource[0].start;
 
-	if (dma==0) dma=-1;
+	if (dma==0) dma=PARPORT_DMA_NONE;
 
 	printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
 		dev->name,dev->slot_name,io,iohi,irq,dma);

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Ignacio Vazquez-Abrams]

On Fri, 21 Sep 2001, Thomas Hood wrote:

> I'm still wondering why this function in parport_pc.c rejects dma
> values of zero.  Is DMA0 not usable by the parallel port for some
> reason?  I should think that if the PnP BIOS returns a dma of zero
> then it means that the parallel port is using DMA0.  Sorry if I'm
> being obtuse.                      // Thomas Hood

DMA0 is reserved for memory refresh. It _can't_ be used for anything else,
therefore a value of 0 is representative of no value whatsoever.

-- 
Ignacio Vazquez-Abrams  <ignacio@openservices.net>

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Alan Cox]

> therefore a value of 0 is representative of no value whatsoever.

This has been unsafe since about 1995, when DMA 0 became available as PC's
stopped using the ISA DMA engine for memory refresh (a very neat original PC
hack)

Alan

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Thomas Hood]

Thanks for the replies.

I'd like to underline Alan Cox's reply that DMA0 _is_ usable.
In fact, my sound chip is configured to use DMA0.

If (and only if) parport _cannot_ use DMA0 for some reason
then the sanity check is justified.  In that case I'd just ask
that a short comment be added to the code that gives the reason.

Cheers
Thomas

Ignacio Vazquez-Abrams wrote:
> DMA0 is reserved for memory refresh. It _can't_ be used for anything else,
> therefore a value of 0 is representative of no value whatsoever.

Alan Cox wrote:
> This has been unsafe since about 1995, when DMA 0 became available as PC's
> stopped using the ISA DMA engine for memory refresh (a very neat original PC
> hack)

Gunther Mayer wrote:
> 1)
>   I think I saw some BIOS report DMA0 for "none" (could even have
>   been ACPI which is returning PNP formatted legacy resource data).
> 2)
>   I have never seen DMA0 for parport configured by a BIOS.
> 3)
>   Try "lssuperio" if you want the real hardware thing.
> 
> This qualifies the code as it is as a sanity check.

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Philip Blundell]

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

>DMA0 is reserved for memory refresh. It _can't_ be used for anything else,
>therefore a value of 0 is representative of no value whatsoever.

On a PC/XT, perhaps.  On any other computer, nothing of the sort is guaranteed.

p.


[-- Attachment #2: Type: application/pgp-signature, Size: 237 bytes --]

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Gunther Mayer]

Thomas Hood wrote:
> 
> I'm still wondering why this function in parport_pc.c rejects dma
> values of zero.  Is DMA0 not usable by the parallel port for some
> reason?  I should think that if the PnP BIOS returns a dma of zero
> then it means that the parallel port is using DMA0.  Sorry if I'm
> being obtuse.                      // Thomas Hood

1)
  I think I saw some BIOS report DMA0 for "none" (could even have
  been ACPI which is returning PNP formatted legacy resource data).

2)
  I have never seen DMA0 for parport configured by a BIOS.

3)
  Try "lssuperio" if you want the real hardware thing.

This qualifies the code as it is as a sanity check.

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Thomas Hood]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

Alan Cox wrote:
> IRQ 0 is a legal valid IRQ. I suspect the problem is that pnpbios shouldnt
> be reporting an IRQ or we should be using some kind of NO_IRQ cookie

Okay, I accept that.  Then must we also say that DMA 0 is a
valid DMA?  If so, then unless I'm confused about something
the rejection of dma==0 should be eliminated:
--------------------------------------------------------------------------------
--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG  Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c       Mon Aug 13 17:40:38 2001
@@ -2797,8 +2797,6 @@
        irq=dev->irq_resource[0].start;
        dma=dev->dma_resource[0].start;

-       if (dma==0) dma=-1;
-
        printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
                dev->name,dev->slot_name,io,iohi,irq,dma);
        if (parport_pc_probe_port(io,iohi,irq,dma,NULL))
--------------------------------------------------------------------------------

Otherwise I think the following change should be made for the sake of
code clarity.  (PARPORT_DMA_NONE is defined as -1.)
--------------------------------------------------------------------------------
--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG  Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c       Fri Aug 10 18:19:55 2001
@@ -2797,7 +2797,7 @@
        irq=dev->irq_resource[0].start;
        dma=dev->dma_resource[0].start;

-       if (dma==0) dma=-1;
+       if (dma==0) dma=PARPORT_DMA_NONE;

        printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
                dev->name,dev->slot_name,io,iohi,irq,dma);
--------------------------------------------------------------------------------

--
Thomas Hood
jdthood_AT_yahoo.co.uk

[-- Attachment #2: patch-norejdma --]
[-- Type: text/plain, Size: 451 bytes --]

--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG	Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c	Mon Aug 13 17:40:38 2001
@@ -2797,8 +2797,6 @@
 	irq=dev->irq_resource[0].start;
 	dma=dev->dma_resource[0].start;
 
-	if (dma==0) dma=-1;
-
 	printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
 		dev->name,dev->slot_name,io,iohi,irq,dma);
 	if (parport_pc_probe_port(io,iohi,irq,dma,NULL))

[-- Attachment #3: patch-dmanone --]
[-- Type: text/plain, Size: 436 bytes --]

--- linux-2.4.7-ac10/drivers/parport/parport_pc.c_ORIG	Fri Aug 10 18:19:07 2001
+++ linux-2.4.7-ac10/drivers/parport/parport_pc.c	Fri Aug 10 18:19:55 2001
@@ -2797,7 +2797,7 @@
 	irq=dev->irq_resource[0].start;
 	dma=dev->dma_resource[0].start;
 
-	if (dma==0) dma=-1;
+	if (dma==0) dma=PARPORT_DMA_NONE;
 
 	printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
 		dev->name,dev->slot_name,io,iohi,irq,dma);

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Alan Cox]

> The following would seem to be required to protect against
> the case in which PnP BIOS reports an IRQ of 0 for a 
> parport with disabled IRQ.      // Thomas  jdthood_AT_yahoo.co.uk

IRQ 0 is a legal valid IRQ. I suspect the problem is that pnpbios shouldnt
be reporting an IRQ or we should be using some kind of NO_IRQ cookie

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Richard B. Johnson]

On Wed, 8 Aug 2001, Alan Cox wrote:

> > The following would seem to be required to protect against
> > the case in which PnP BIOS reports an IRQ of 0 for a 
> > parport with disabled IRQ.      // Thomas  jdthood_AT_yahoo.co.uk
> 
> IRQ 0 is a legal valid IRQ. I suspect the problem is that pnpbios shouldnt
> be reporting an IRQ or we should be using some kind of NO_IRQ cookie

IRQ0 will never by reported by a PCI bus device because it means that
no IRQ is used (they figured that IRQ0 would always be used for something
else). Maybe PnP BIOS also presumes this? If so, the use of IRQ0 to
mean "no IRQ" is valid, although misleading.

Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

    I was going to compile a list of innovations that could be
    attributed to Microsoft. Once I realized that Ctrl-Alt-Del
    was handled in the BIOS, I found that there aren't any.

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[H. Peter Anvin]

Followup to:  <E15UV8M-0005SE-00@the-village.bc.nu>
By author:    Alan Cox <alan@lxorguk.ukuu.org.uk>
In newsgroup: linux.dev.kernel
>
> > The following would seem to be required to protect against
> > the case in which PnP BIOS reports an IRQ of 0 for a 
> > parport with disabled IRQ.      // Thomas  jdthood_AT_yahoo.co.uk
> 
> IRQ 0 is a legal valid IRQ. I suspect the problem is that pnpbios shouldnt
> be reporting an IRQ or we should be using some kind of NO_IRQ cookie
>

IRQ 0 is hardwired to the system timer in PC systems, though, so it
could simply be assumed that IRQ 0 will never be used for any other
purposes.

Reminds me back in the days when you had to worry about DRQs as well;
DRQ 0 was hardwired in the original PC but then became available in
the AT; there was a whole bunch of things that assumed DRQ 0 wasn't
usable, even though it was perfectly fine.  Not to mention the
motherboard I had which would lock up solid if anything ever used
DRQ 5.

Good riddance, all this crap...

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<amsp@zytor.com>

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Russell King]

On Wed, Aug 08, 2001 at 02:58:12PM -0700, H. Peter Anvin wrote:
> IRQ 0 is hardwired to the system timer in PC systems, though, so it
                                         ^^^^^^^^^^^^^

Linux doesn't run on only PC systems though, and other systems use
IRQ0 as the (superio-based) parallel port IRQ.

> Good riddance, all this crap...

Indeed - please check the ARM port for our solution to this.  We've
had the NO_IRQ construct for literally years in include/asm-arm/irq.h:

#define NO_IRQ  ((unsigned int)(-1))

Naturally, a similar NO_DMA is defined in dma.h.  The sooner we can get
rid of the "IRQ0 cannot be used" crap from the kernel the better.

--
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] parport_pc.c PnP BIOS sanity check

[Eric W. Biederman]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Followup to:  <E15UV8M-0005SE-00@the-village.bc.nu>
> By author:    Alan Cox <alan@lxorguk.ukuu.org.uk>
> In newsgroup: linux.dev.kernel
> >
> > > The following would seem to be required to protect against
> > > the case in which PnP BIOS reports an IRQ of 0 for a 
> > > parport with disabled IRQ.      // Thomas  jdthood_AT_yahoo.co.uk
> > 
> > IRQ 0 is a legal valid IRQ. I suspect the problem is that pnpbios shouldnt
> > be reporting an IRQ or we should be using some kind of NO_IRQ cookie
> >
> 
> IRQ 0 is hardwired to the system timer in PC systems, though, so it
> could simply be assumed that IRQ 0 will never be used for any other
> purposes.
> 
> Reminds me back in the days when you had to worry about DRQs as well;
> DRQ 0 was hardwired in the original PC but then became available in
> the AT; there was a whole bunch of things that assumed DRQ 0 wasn't
> usable, even though it was perfectly fine.  Not to mention the
> motherboard I had which would lock up solid if anything ever used
> DRQ 5.
> 
> Good riddance, all this crap...

If we are going to list all of the silly assumptions, we still have
the assumption that 640KB-1MB on x86 cannot be used as ram.  It is
less painful but still annoying when you put perfectly valid ram
there.  :) 

Eric

[PATCH] parport_pc.c PnP BIOS sanity check

[Thomas Hood]

The following would seem to be required to protect against
the case in which PnP BIOS reports an IRQ of 0 for a 
parport with disabled IRQ.      // Thomas  jdthood_AT_yahoo.co.uk

--- linux-2.4.7-ac2/drivers/parport/parport_pc.c        Mon Jul 30 01:18:34 2001
+++ linux-2.4.7-ac2-jdth1/drivers/parport/parport_pc.c  Mon Jul 30 12:32:16 2001
@@ -2797,7 +2797,8 @@
        irq=dev->irq_resource[0].start;
        dma=dev->dma_resource[0].start;

-       if(dma==0) dma=-1;
+       if (dma==0) dma=PARPORT_DMA_NONE;
+       if (irq==0) irq=PARPORT_IRQ_NONE;

        printk(KERN_INFO "PnPBIOS: Parport found %s %s at io=%04x,%04x irq=%d dma=%d\n",
                dev->name,dev->slot_name,io,iohi,irq,dma);