[PATCH] 2.4.10-ac11 parport_pc.c bugfix

[PATCH] 2.4.10-ac11 parport_pc.c bugfix

[Thomas Hood]

This fix makes the parport driver print the correct dma number
and makes explicit a couple of type casts.  Applies cleanly against
-ac11         // Thomas Hood

The patch:
--- linux-2.4.10-ac10/drivers/parport/parport_pc.c	Mon Oct  8 22:41:14 2001
+++ linux-2.4.10-ac10-fix/drivers/parport/parport_pc.c	Tue Oct  9 19:36:58 2001
@@ -2826,7 +2826,7 @@
 	if ( UNSET(dev->irq_resource[0]) ) {
 		irq = PARPORT_IRQ_NONE;
 	} else {
-		if ( dev->irq_resource[0].start == -1 ) {
+		if ( dev->irq_resource[0].start == (unsigned long)-1 ) {
 			irq = PARPORT_IRQ_NONE;
 			printk(", irq disabled");
 		} else {
@@ -2838,12 +2838,12 @@
 	if ( UNSET(dev->dma_resource[0]) ) {
 		dma = PARPORT_DMA_NONE;
 	} else {
-		if ( dev->dma_resource[0].start == -1 ) {
+		if ( dev->dma_resource[0].start == (unsigned long)-1 ) {
 			dma = PARPORT_DMA_NONE;
 			printk(", dma disabled");
 		} else {
 			dma = dev->dma_resource[0].start;
-			printk(", dma %d",irq);
+			printk(", dma %d",dma);
 		}
 	}
 

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[J . A . Magallon]

On 20011011 Thomas Hood wrote:
> 	} else {
>-		if ( dev->irq_resource[0].start == -1 ) {
>+		if ( dev->irq_resource[0].start == (unsigned long)-1 ) {
                                                   ^^^^^^^^^      ^
Uh ?

Perhaps I miss some black magic in kernel programming, but could not this
be written much cleaner like

>+		if ( dev->irq_resource[0].start == ~0U ) {

-- 
J.A. Magallon                           #  Let the source be with you...        
mailto:jamagallon@able.es
Mandrake Linux release 8.2 (Cooker) for i586
Linux werewolf 2.4.10-ac11-beo #2 SMP Thu Oct 11 02:41:04 CEST 2001 i686

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[Thomas Hood]

I guess the question is: Which way is more portable?  Is
"(unsigned long)-1" liable to turn out as something other than
~0U?

If your way of expressing it is more portable then we should
make the change ... BOTH in pnp_bios.c and in parport_pc.c .

Opinions?

--
Thomas


On Thu, 2001-10-11 at 09:40, J . A . Magallon wrote:
> 
> On 20011011 Thomas Hood wrote:
> > 	} else {
> >-		if ( dev->irq_resource[0].start == -1 ) {
> >+		if ( dev->irq_resource[0].start == (unsigned long)-1 ) {
>                                                    ^^^^^^^^^      ^
> Uh ?
> 
> Perhaps I miss some black magic in kernel programming, but could not this
> be written much cleaner like
> 
> >+		if ( dev->irq_resource[0].start == ~0U ) {
> 
> -- 
> J.A. Magallon                           #  Let the source be with you...        
> mailto:jamagallon@able.es
> Mandrake Linux release 8.2 (Cooker) for i586
> Linux werewolf 2.4.10-ac11-beo #2 SMP Thu Oct 11 02:41:04 CEST 2001 i686
> 

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[Bob McElrath]

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

Thomas Hood [jdthood@mail.com] wrote:
> I guess the question is: Which way is more portable?  Is
> "(unsigned long)-1" liable to turn out as something other than
> ~0U?
> 
> If your way of expressing it is more portable then we should
> make the change ... BOTH in pnp_bios.c and in parport_pc.c .
> 
> Opinions?

unsigned long is 64-bits on 64-bit archs, and so (unsigned long)-1 will be
different than on intel...

I think that's ~0UL?

Maybe this is why I can't get pnp stuff to work on my alpha?

Cheers,
-- Bob

Bob McElrath (rsmcelrath@students.wisc.edu) 
Univ. of Wisconsin at Madison, Department of Physics

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

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[bill davidsen]

In article <1002808349.10317.7.camel@thanatos> you write:
| I guess the question is: Which way is more portable?  Is
| "(unsigned long)-1" liable to turn out as something other than
| ~0U?
| 
| If your way of expressing it is more portable then we should
| make the change ... BOTH in pnp_bios.c and in parport_pc.c .
| 
| Opinions?

Does this address any bug present without the cast? If the expression
left of == is unsigned long, ~0U will work, as will (unsigned)~0. But
more to the point, do you mean "minus one" or "all ones" here? While
Linux does not currently run on any machine which is not 2's complement,
I think if you want all ones you should use (unsigned long)~0 as most
portable.

Yes, I have programmed 1's complement machines :-(

-- 
bill davidsen <davidsen@tmr.com>
 "If I were a diplomat, in the best case I'd go hungry.  In the worst
  case, people would die."
		-- Robert Lipe

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[Thomas Hood]

On Thu, 2001-10-11 at 18:41, bill davidsen wrote:
> Does this address any bug present without the cast?

No.  There's no bug.  The debate is over whether or not
it's naughty to cast -1 to unsigned long.

--
Thomas

Re: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

[J.D. Hood]

The fact that (unsigned long)-1 and ~0U have different 
values on different arches isn't a problem.

What would be a problem is if (unsigned long)-1 were liable
to be given different values by different compilers, and
especially if (unsigned long)-1 were liable to be cast to
some number between 0 and 15.  But I assume that the type
casting rules of C prohibit this---that (unsigned long)-1
is assured to be some very large positive integer.  (Indeed,
the largest.)  However I'm uncertain enough about this
assumption that I raise the present question about it.

Assuming that the assumption is correct [:)], the patch is fine
as it is and should go in.

BTW what problem is it you are having with PnP BIOS on your arch?
Can you refer me to a past thread title?

--
Thomas

____________________________________________________________
Do You Yahoo!?
Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
or your free @yahoo.ie address at http://mail.yahoo.ie