2006-10-01 15:21:19

by Jeff Garzik

[permalink] [raw]
Subject: [PATCH] ISDN: mark as 32-bit only


Tons of ISDN drivers cast pointers to/from 32-bit values, which just
won't work on 64-bit.

Signed-off-by: Jeff Garzik <[email protected]>

diff --git a/drivers/isdn/Kconfig b/drivers/isdn/Kconfig
index c90afee..608588f 100644
--- a/drivers/isdn/Kconfig
+++ b/drivers/isdn/Kconfig
@@ -6,7 +6,7 @@ menu "ISDN subsystem"

config ISDN
tristate "ISDN support"
- depends on NET
+ depends on NET && 32BIT
---help---
ISDN ("Integrated Services Digital Networks", called RNIS in France)
is a special type of fully digital telephone service; it's mostly


2006-10-01 18:11:07

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

>
>Tons of ISDN drivers cast pointers to/from 32-bit values, which just
>won't work on 64-bit.

Should not that be fixed instead of restricting isdn to 32bit?
Though this is probably the best temporary workaround until someone can
fix up all the "tons".


Jan Engelhardt
--

2006-10-01 20:28:47

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Ar Sul, 2006-10-01 am 11:21 -0400, ysgrifennodd Jeff Garzik:
> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
> won't work on 64-bit.
>
> Signed-off-by: Jeff Garzik <[email protected]>

NAK

I've actually been spending some time reviewing these and the warnings
are for stupid things but not real 64/32 problems. I've got some diffs
that clean it up just by tidying up casts etc if anyone actually still
cares about the old ISDN code.

Alan

2006-10-01 23:22:56

by Tilman Schmidt

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

On Sun, 1 Oct 2006 11:21:16 -0400, Jeff Garzik wrote:
> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
> won't work on 64-bit.
>
> Signed-off-by: Jeff Garzik <[email protected]>
>
> diff --git a/drivers/isdn/Kconfig b/drivers/isdn/Kconfig
> index c90afee..608588f 100644
> --- a/drivers/isdn/Kconfig
> +++ b/drivers/isdn/Kconfig
> @@ -6,7 +6,7 @@ menu "ISDN subsystem"
>
> config ISDN
> tristate "ISDN support"
> - depends on NET
> + depends on NET && 32BIT
> ---help---
> ISDN ("Integrated Services Digital Networks", called RNIS in France)
> is a special type of fully digital telephone service; it's mostly

NAK. Just because many or even most of the ISDN drivers don't work
on 64 bit systems that's no reason to prevent using those that do.

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeoeffnet mindestens haltbar bis: (siehe Rueckseite)


Attachments:
signature.asc (253.00 B)
OpenPGP digital signature

2006-10-01 23:23:44

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Some suggestions on doing it better

- Clean up warnings in drivers/isdn by using long not int for the values
where we pass void * and cast to integer types. The code is ok (ok
passing the stuff this way isn't pretty but the code is valid). In all
the cases I checked out the right thing happens anyway but this removes
all the warnings.

Signed-off-by: Alan Cox <[email protected]>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/config.c linux-2.6.18-mm2/drivers/isdn/hisax/config.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/config.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/config.c 2006-09-25 12:20:16.000000000 +0100
@@ -1721,11 +1721,11 @@
hisax_b_sched_event(bcs, B_RCVBUFREADY);
break;
case PH_DATA | CONFIRM:
- bcs->tx_cnt -= (int) arg;
+ bcs->tx_cnt -= (long)arg;
if (test_bit(FLG_LLI_L1WAKEUP,&bcs->st->lli.flag)) {
u_long flags;
spin_lock_irqsave(&bcs->aclock, flags);
- bcs->ackcnt += (int) arg;
+ bcs->ackcnt += (long)arg;
spin_unlock_irqrestore(&bcs->aclock, flags);
schedule_event(bcs, B_ACKPENDING);
}
@@ -1789,7 +1789,7 @@

switch (pr) {
case PH_ACTIVATE | REQUEST:
- B_L2L1(b_if, pr, (void *) st->l1.mode);
+ B_L2L1(b_if, pr, (void *)(unsigned long)st->l1.mode);
break;
case PH_DATA | REQUEST:
case PH_PULL | INDICATION:
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c 2006-09-25 12:20:25.000000000 +0100
@@ -424,7 +424,7 @@
struct hfc4s8s_btype *bch = ifc->priv;
struct hfc4s8s_l1 *l1 = bch->l1p;
struct sk_buff *skb = (struct sk_buff *) arg;
- int mode = (int) arg;
+ long mode = (long) arg;
u_long flags;

switch (pr) {
@@ -914,7 +914,7 @@
struct sk_buff *skb;
u_char f1, f2;
u_char *cp;
- int cnt;
+ long cnt;

if (l1p->l1_state != 7)
return;
@@ -980,7 +980,8 @@
struct sk_buff *skb;
struct hfc4s8s_l1 *l1 = bch->l1p;
u_char *cp;
- int cnt, max, hdlc_num, ack_len = 0;
+ int cnt, max, hdlc_num;
+ long ack_len = 0;

if (!l1->enabled || (bch->mode == L1_MODE_NULL))
return;
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c 2006-09-25 12:20:25.000000000 +0100
@@ -970,7 +970,7 @@
break;
case (HW_TESTLOOP | REQUEST):
spin_lock_irqsave(&cs->lock, flags);
- switch ((int) arg) {
+ switch ((long) arg) {
case (1):
Write_hfc(cs, HFCSX_B1_SSL, 0x80); /* tx slot */
Write_hfc(cs, HFCSX_B1_RSL, 0x80); /* rx slot */
@@ -986,7 +986,7 @@
default:
spin_unlock_irqrestore(&cs->lock, flags);
if (cs->debug & L1_DEB_WARN)
- debugl1(cs, "hfcsx_l1hw loop invalid %4x", (int) arg);
+ debugl1(cs, "hfcsx_l1hw loop invalid %4lx", arg);
return;
}
cs->hw.hfcsx.trm |= 0x80; /* enable IOM-loop */
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c 2006-09-25 12:20:25.000000000 +0100
@@ -696,7 +696,7 @@
fifo->delete_flg = TRUE;
fifo->hif->l1l2(fifo->hif,
PH_DATA | CONFIRM,
- (void *) fifo->skbuff->
+ (void *) (unsigned long) fifo->skbuff->
truesize);
if (fifo->skbuff && fifo->delete_flg) {
dev_kfree_skb_any(fifo->skbuff);
@@ -1144,7 +1144,7 @@
set_hfcmode(hfc,
(fifo->fifonum ==
HFCUSB_B1_TX) ? 0 : 1,
- (int) arg);
+ (long) arg);
fifo->hif->l1l2(fifo->hif,
PH_ACTIVATE | INDICATION,
NULL);
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c linux-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c 2006-09-25 12:20:25.000000000 +0100
@@ -546,7 +546,7 @@
}
bcs->tx_cnt = 0;
bcs->tx_skb = NULL;
- B_L1L2(bcs, PH_DATA | CONFIRM, (void *) skb->truesize);
+ B_L1L2(bcs, PH_DATA | CONFIRM, (void *)(unsigned long)skb->truesize);
dev_kfree_skb_irq(skb);
}

@@ -635,7 +635,7 @@
hdlc_fill_fifo(bcs);
break;
case PH_ACTIVATE | REQUEST:
- mode = (int) arg;
+ mode = (long) arg;
DBG(4,"B%d,PH_ACTIVATE_REQUEST %d", bcs->channel + 1, mode);
modehdlc(bcs, mode);
B_L1L2(bcs, PH_ACTIVATE | INDICATION, NULL);
@@ -998,18 +998,15 @@

retval = pci_register_driver(&fcpci_driver);
if (retval)
- goto out;
+ return retval;
#ifdef __ISAPNP__
retval = pnp_register_driver(&fcpnp_driver);
- if (retval < 0)
- goto out_unregister_pci;
+ if (retval < 0) {
+ pci_unregister_driver(&fcpci_driver);
+ return retval;
+ }
#endif
return 0;
-
- out_unregister_pci:
- pci_unregister_driver(&fcpci_driver);
- out:
- return retval;
}

static void __exit hisax_fcpcipnp_exit(void)
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c linux-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c 2006-09-25 12:20:39.000000000 +0100
@@ -86,7 +86,7 @@
if (!skb->len) {
// Frame sent
b_out->tx_skb = NULL;
- B_L1L2(bcs, PH_DATA | CONFIRM, (void *) skb->truesize);
+ B_L1L2(bcs, PH_DATA | CONFIRM, (void *)(unsigned long) skb->truesize);
dev_kfree_skb_any(skb);

/* if (!(bcs->tx_skb = skb_dequeue(&bcs->sq))) { */
@@ -350,7 +350,7 @@
{
struct st5481_bcs *bcs = ifc->priv;
struct sk_buff *skb = arg;
- int mode;
+ long mode;

DBG(4, "");

@@ -360,8 +360,8 @@
bcs->b_out.tx_skb = skb;
break;
case PH_ACTIVATE | REQUEST:
- mode = (int) arg;
- DBG(4,"B%d,PH_ACTIVATE_REQUEST %d", bcs->channel + 1, mode);
+ mode = (long) arg;
+ DBG(4,"B%d,PH_ACTIVATE_REQUEST %ld", bcs->channel + 1, mode);
st5481B_mode(bcs, mode);
B_L1L2(bcs, PH_ACTIVATE | INDICATION, NULL);
break;
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c linux-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c
--- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c 2006-09-20 04:42:06.000000000 +0100
+++ linux-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c 2006-09-25 12:20:39.000000000 +0100
@@ -374,7 +374,7 @@
{
struct st5481_adapter *adapter = urb->context;
struct st5481_d_out *d_out = &adapter->d_out;
- int buf_nr;
+ long buf_nr;

DBG(2, "");

@@ -546,7 +546,7 @@
static void dout_complete(struct FsmInst *fsm, int event, void *arg)
{
struct st5481_adapter *adapter = fsm->userdata;
- int buf_nr = (int) arg;
+ long buf_nr = (long) arg;

usb_d_out(adapter, buf_nr);
}

2006-10-02 00:09:09

by Tilman Schmidt

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

On Sun, 01 Oct 2006 22:30:17 +0200, Alan Cox wrote:
> I've actually been spending some time reviewing these and the warnings
> are for stupid things but not real 64/32 problems. I've got some diffs
> that clean it up just by tidying up casts etc if anyone actually still
> cares about the old ISDN code.

Yes, the old ISDN code still has a considerable user base at least
here in Germany. There are many ISDN devices which are supported by
the old ISDN subsystem (ISDN4Linux) but not the new one (CAPI2.0).
(Some of them are supported by out-of-tree CAPI2.0 drivers like mISDN,
but still.) Also, CAPI2.0 so far doesn't have a driver interface
document (like Documentation/isdn/INTERFACE for ISDN4Linux), so the
old drivers may still be with us for some time to come.

Besides, Jeff's proposed patch would disable the new ISDN subsystem
along with the old one, so I suppose his criticism applies to the
new one too.

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeoeffnet mindestens haltbar bis: (siehe Rueckseite)




Attachments:
signature.asc (253.00 B)
OpenPGP digital signature

2006-10-02 03:54:55

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Jan Engelhardt wrote:
>> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
>> won't work on 64-bit.
>
> Should not that be fixed instead of restricting isdn to 32bit?

It hasn't been fixed in many years, and I don't see anyone stepping up
to the plate, even with my current trolling... :)


> Though this is probably the best temporary workaround until someone can
> fix up all the "tons".

I have a better workaround, I think.

Jeff



2006-10-02 09:26:43

by Karsten Keil

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

On Sun, Oct 01, 2006 at 11:21:16AM -0400, Jeff Garzik wrote:
>
> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
> won't work on 64-bit.
>

NACK.

This is no problem for ISDN only some old HW drivers may not work (for most
I do not have any hardware by myself).
I will compile all drivers again and if here are 64 bit issues I will
disable the drivers, not ISDN.

ISDN works on 64 bits, at least with following protocols:
- transparent (audio/voice)
- X.75
- HDLC
- sync PPP

Maybe some other protocols need some 64 bit cleanup, but last time I audited
the ISDN core code for 64 bit I did not saw any problems.
My main test machines are 64 bit (and SMP) and they are running ISDN servers
for sync PPP and voice data processing on different ISDN HW and drivers
(CAPI and Hisax) without any problem.

You will see still some warnings in the CAPI code, because the capi code
has dual 64/32 bit protocol format for data packets (has 2 different pointer
areas one for 64 bit one for32 bit), but this is OK.

> Signed-off-by: Jeff Garzik <[email protected]>
>
> diff --git a/drivers/isdn/Kconfig b/drivers/isdn/Kconfig
> index c90afee..608588f 100644
> --- a/drivers/isdn/Kconfig
> +++ b/drivers/isdn/Kconfig
> @@ -6,7 +6,7 @@ menu "ISDN subsystem"
>
> config ISDN
> tristate "ISDN support"
> - depends on NET
> + depends on NET && 32BIT
> ---help---
> ISDN ("Integrated Services Digital Networks", called RNIS in France)
> is a special type of fully digital telephone service; it's mostly

--
Karsten Keil
SuSE Labs
ISDN development

2006-10-02 09:31:15

by Karsten Keil

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Thanks.

On Mon, Oct 02, 2006 at 12:47:50AM +0100, Alan Cox wrote:
> Some suggestions on doing it better
>
> - Clean up warnings in drivers/isdn by using long not int for the values
> where we pass void * and cast to integer types. The code is ok (ok
> passing the stuff this way isn't pretty but the code is valid). In all
> the cases I checked out the right thing happens anyway but this removes
> all the warnings.
>
> Signed-off-by: Alan Cox <[email protected]>

Acked-by: [email protected]

>
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/config.c linux-2.6.18-mm2/drivers/isdn/hisax/config.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/config.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/config.c 2006-09-25 12:20:16.000000000 +0100
> @@ -1721,11 +1721,11 @@
> hisax_b_sched_event(bcs, B_RCVBUFREADY);
> break;
> case PH_DATA | CONFIRM:
> - bcs->tx_cnt -= (int) arg;
> + bcs->tx_cnt -= (long)arg;
> if (test_bit(FLG_LLI_L1WAKEUP,&bcs->st->lli.flag)) {
> u_long flags;
> spin_lock_irqsave(&bcs->aclock, flags);
> - bcs->ackcnt += (int) arg;
> + bcs->ackcnt += (long)arg;
> spin_unlock_irqrestore(&bcs->aclock, flags);
> schedule_event(bcs, B_ACKPENDING);
> }
> @@ -1789,7 +1789,7 @@
>
> switch (pr) {
> case PH_ACTIVATE | REQUEST:
> - B_L2L1(b_if, pr, (void *) st->l1.mode);
> + B_L2L1(b_if, pr, (void *)(unsigned long)st->l1.mode);
> break;
> case PH_DATA | REQUEST:
> case PH_PULL | INDICATION:
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc4s8s_l1.c 2006-09-25 12:20:25.000000000 +0100
> @@ -424,7 +424,7 @@
> struct hfc4s8s_btype *bch = ifc->priv;
> struct hfc4s8s_l1 *l1 = bch->l1p;
> struct sk_buff *skb = (struct sk_buff *) arg;
> - int mode = (int) arg;
> + long mode = (long) arg;
> u_long flags;
>
> switch (pr) {
> @@ -914,7 +914,7 @@
> struct sk_buff *skb;
> u_char f1, f2;
> u_char *cp;
> - int cnt;
> + long cnt;
>
> if (l1p->l1_state != 7)
> return;
> @@ -980,7 +980,8 @@
> struct sk_buff *skb;
> struct hfc4s8s_l1 *l1 = bch->l1p;
> u_char *cp;
> - int cnt, max, hdlc_num, ack_len = 0;
> + int cnt, max, hdlc_num;
> + long ack_len = 0;
>
> if (!l1->enabled || (bch->mode == L1_MODE_NULL))
> return;
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc_sx.c 2006-09-25 12:20:25.000000000 +0100
> @@ -970,7 +970,7 @@
> break;
> case (HW_TESTLOOP | REQUEST):
> spin_lock_irqsave(&cs->lock, flags);
> - switch ((int) arg) {
> + switch ((long) arg) {
> case (1):
> Write_hfc(cs, HFCSX_B1_SSL, 0x80); /* tx slot */
> Write_hfc(cs, HFCSX_B1_RSL, 0x80); /* rx slot */
> @@ -986,7 +986,7 @@
> default:
> spin_unlock_irqrestore(&cs->lock, flags);
> if (cs->debug & L1_DEB_WARN)
> - debugl1(cs, "hfcsx_l1hw loop invalid %4x", (int) arg);
> + debugl1(cs, "hfcsx_l1hw loop invalid %4lx", arg);
> return;
> }
> cs->hw.hfcsx.trm |= 0x80; /* enable IOM-loop */
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c linux-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/hfc_usb.c 2006-09-25 12:20:25.000000000 +0100
> @@ -696,7 +696,7 @@
> fifo->delete_flg = TRUE;
> fifo->hif->l1l2(fifo->hif,
> PH_DATA | CONFIRM,
> - (void *) fifo->skbuff->
> + (void *) (unsigned long) fifo->skbuff->
> truesize);
> if (fifo->skbuff && fifo->delete_flg) {
> dev_kfree_skb_any(fifo->skbuff);
> @@ -1144,7 +1144,7 @@
> set_hfcmode(hfc,
> (fifo->fifonum ==
> HFCUSB_B1_TX) ? 0 : 1,
> - (int) arg);
> + (long) arg);
> fifo->hif->l1l2(fifo->hif,
> PH_ACTIVATE | INDICATION,
> NULL);
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c linux-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/hisax_fcpcipnp.c 2006-09-25 12:20:25.000000000 +0100
> @@ -546,7 +546,7 @@
> }
> bcs->tx_cnt = 0;
> bcs->tx_skb = NULL;
> - B_L1L2(bcs, PH_DATA | CONFIRM, (void *) skb->truesize);
> + B_L1L2(bcs, PH_DATA | CONFIRM, (void *)(unsigned long)skb->truesize);
> dev_kfree_skb_irq(skb);
> }
>
> @@ -635,7 +635,7 @@
> hdlc_fill_fifo(bcs);
> break;
> case PH_ACTIVATE | REQUEST:
> - mode = (int) arg;
> + mode = (long) arg;
> DBG(4,"B%d,PH_ACTIVATE_REQUEST %d", bcs->channel + 1, mode);
> modehdlc(bcs, mode);
> B_L1L2(bcs, PH_ACTIVATE | INDICATION, NULL);
> @@ -998,18 +998,15 @@
>
> retval = pci_register_driver(&fcpci_driver);
> if (retval)
> - goto out;
> + return retval;
> #ifdef __ISAPNP__
> retval = pnp_register_driver(&fcpnp_driver);
> - if (retval < 0)
> - goto out_unregister_pci;
> + if (retval < 0) {
> + pci_unregister_driver(&fcpci_driver);
> + return retval;
> + }
> #endif
> return 0;
> -
> - out_unregister_pci:
> - pci_unregister_driver(&fcpci_driver);
> - out:
> - return retval;
> }
>
> static void __exit hisax_fcpcipnp_exit(void)
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c linux-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/st5481_b.c 2006-09-25 12:20:39.000000000 +0100
> @@ -86,7 +86,7 @@
> if (!skb->len) {
> // Frame sent
> b_out->tx_skb = NULL;
> - B_L1L2(bcs, PH_DATA | CONFIRM, (void *) skb->truesize);
> + B_L1L2(bcs, PH_DATA | CONFIRM, (void *)(unsigned long) skb->truesize);
> dev_kfree_skb_any(skb);
>
> /* if (!(bcs->tx_skb = skb_dequeue(&bcs->sq))) { */
> @@ -350,7 +350,7 @@
> {
> struct st5481_bcs *bcs = ifc->priv;
> struct sk_buff *skb = arg;
> - int mode;
> + long mode;
>
> DBG(4, "");
>
> @@ -360,8 +360,8 @@
> bcs->b_out.tx_skb = skb;
> break;
> case PH_ACTIVATE | REQUEST:
> - mode = (int) arg;
> - DBG(4,"B%d,PH_ACTIVATE_REQUEST %d", bcs->channel + 1, mode);
> + mode = (long) arg;
> + DBG(4,"B%d,PH_ACTIVATE_REQUEST %ld", bcs->channel + 1, mode);
> st5481B_mode(bcs, mode);
> B_L1L2(bcs, PH_ACTIVATE | INDICATION, NULL);
> break;
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c linux-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c
> --- linux.vanilla-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c 2006-09-20 04:42:06.000000000 +0100
> +++ linux-2.6.18-mm2/drivers/isdn/hisax/st5481_d.c 2006-09-25 12:20:39.000000000 +0100
> @@ -374,7 +374,7 @@
> {
> struct st5481_adapter *adapter = urb->context;
> struct st5481_d_out *d_out = &adapter->d_out;
> - int buf_nr;
> + long buf_nr;
>
> DBG(2, "");
>
> @@ -546,7 +546,7 @@
> static void dout_complete(struct FsmInst *fsm, int event, void *arg)
> {
> struct st5481_adapter *adapter = fsm->userdata;
> - int buf_nr = (int) arg;
> + long buf_nr = (long) arg;
>
> usb_d_out(adapter, buf_nr);
> }

--
Karsten Keil
SuSE Labs
ISDN development

2006-10-02 09:46:31

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Jeff Garzik <[email protected]> writes:

> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
> won't work on 64-bit.

Actually AFAIK all ISDN drivers work on x86-64.
However x86-64 doesn't support all of them because many depend
on CONFIG_ISA.

-Andi

2006-10-02 20:37:04

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

From: Jeff Garzik <[email protected]>
Date: Sun, 01 Oct 2006 23:54:42 -0400

> Jan Engelhardt wrote:
> >> Tons of ISDN drivers cast pointers to/from 32-bit values, which just
> >> won't work on 64-bit.
> >
> > Should not that be fixed instead of restricting isdn to 32bit?
>
> It hasn't been fixed in many years, and I don't see anyone stepping up
> to the plate, even with my current trolling... :)
>
> > Though this is probably the best temporary workaround until someone can
> > fix up all the "tons".
>
> I have a better workaround, I think.

I totally agree with Jeff. The ISDN layer is effectively unmaintained
and the vast majority of the ISDN work that does actually occur is
done out-of-tree.

If someone cares enough about the ISDN layer, they can make changes
after Jeff's goes in to reduce the protection to a per-driver basis.

But right now, blocking this whole unmaintained pile of poo on 64-bit
is the best starting point.

We're talking about effectively an 8 year old code base that hasn't
been touched much at all during that time. Let's be honest about the
situation. The only subsystem that is more unmaintained and gathering
dust is probably the ftape layer :-)


2006-10-02 21:39:34

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ISDN: mark as 32-bit only

Ar Llu, 2006-10-02 am 13:37 -0700, ysgrifennodd David Miller:
> I totally agree with Jeff. The ISDN layer is effectively unmaintained
> and the vast majority of the ISDN work that does actually occur is
> done out-of-tree.

There are big chunks this is not true for. Also it work son x86-64
correctly, Jeff is barking up the wrong tree altogether.

Alan