2001-10-11 02:21:04

by Thomas Hood

[permalink] [raw]
Subject: [PATCH] 2.4.10-ac11 parport_pc.c bugfix

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);
}
}



2001-10-11 13:40:35

by J.A. Magallon

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


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:[email protected]
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

2001-10-11 13:53:25

by Thomas Hood

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

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:[email protected]
> 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
>


2001-10-11 14:52:59

by Bob McElrath

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

Thomas Hood [[email protected]] 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 ([email protected])
Univ. of Wisconsin at Madison, Department of Physics


Attachments:
(No filename) (615.00 B)
(No filename) (240.00 B)
Download all attachments

2001-10-11 18:35:32

by Thomas Hood

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

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

2001-10-11 22:41:22

by Bill Davidsen

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

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 <[email protected]>
"If I were a diplomat, in the best case I'd go hungry. In the worst
case, people would die."
-- Robert Lipe

2001-10-12 01:47:26

by Thomas Hood

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

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