Re: [PATCH] ACP Modem (Mwave)

Re: [PATCH] ACP Modem (Mwave)

[Paul Schroeder]

Okay..  I cleaned things up...  I was more careful about the globals this
time...

Also...  The driver now builds as mwave.o instead of mwavedd.o...  The
driver now registers it's UART as a serial device (Thomas Hood)..  So there
is no need to run setserial anymore...

Cheers...Paul...


---
Paul B Schroeder  <paulsch@us.ibm.com>
Software Engineer, Linux Technology Center
IBM Corporation, Austin, TX


Alan Cox <alan@lxorguk.ukuu.org.uk> on 07/12/2001 02:55:00 PM

To:   Paul Schroeder/Austin/IBM@IBMUS
cc:   linux-kernel@vger.kernel.org, Mike Sullivan/Austin/IBM@IBMUS,
      alan@lxorguk.ukuu.org.uk (Alan Cox)
Subject:  Re: [PATCH] ACP Modem (Mwave)



> The patch has been updated...  The updates primarily consist of Alan's
> suggested changes below... (thank you)  It applies against the 2.4.6
> kernel...

A quick glance through it:

dsp3780I_WriteDStore still touches user space with a spinlock held
(also doesnt check the get_user return)

The ioctl handlers do not check copy_from_user/to_user returns

IOCTL_MW_UNREGISTER_IPC will oops if fed bogus info (ipcnum should be
unsigned)

The return should be -ENOTTY not -ENOIOCTLCMD  unless its internal code
that catches NOIOCTLCMD and changes it before user space sees it

mwave_Read should be -EINVAL not -ENOSYS (ENOSYS means the entire read
syscall
in the OS isnt there)

In debug mode mwave_write accesses user space directly and may crash
(buf[0])

Trivial item - coding style uses foo(void) not foo() to indicate functions
taking no arguments

Still have globals like "dspio" "uartio"  "ClaimResources" etc

whats wrong with tp3780_uart_io etc for globals ?

Otherwise it looks close to ready

Alan

Re: [PATCH] ACP Modem (Mwave)

[paulsch]

Oh yeah...

It can be gotten here:

http://oss.software.ibm.com/acpmodem/

Directly:
http://oss.software.ibm.com/pub/acpmodem/mwave_linux-2.4.6.patch-20010718


On Wed, 18 Jul 2001, Paul Schroeder wrote:

> Okay..  I cleaned things up...  I was more careful about the globals this
> time...
>
> Also...  The driver now builds as mwave.o instead of mwavedd.o...  The
> driver now registers it's UART as a serial device (Thomas Hood)..  So there
> is no need to run setserial anymore...
>
> Cheers...Paul...
>
>
> ---
> Paul B Schroeder  <paulsch@us.ibm.com>
> Software Engineer, Linux Technology Center
> IBM Corporation, Austin, TX
>
>
> Alan Cox <alan@lxorguk.ukuu.org.uk> on 07/12/2001 02:55:00 PM
>
> To:   Paul Schroeder/Austin/IBM@IBMUS
> cc:   linux-kernel@vger.kernel.org, Mike Sullivan/Austin/IBM@IBMUS,
>       alan@lxorguk.ukuu.org.uk (Alan Cox)
> Subject:  Re: [PATCH] ACP Modem (Mwave)
>
>
>
> > The patch has been updated...  The updates primarily consist of Alan's
> > suggested changes below... (thank you)  It applies against the 2.4.6
> > kernel...
>
> A quick glance through it:
>
> dsp3780I_WriteDStore still touches user space with a spinlock held
> (also doesnt check the get_user return)
>
> The ioctl handlers do not check copy_from_user/to_user returns
>
> IOCTL_MW_UNREGISTER_IPC will oops if fed bogus info (ipcnum should be
> unsigned)
>
> The return should be -ENOTTY not -ENOIOCTLCMD  unless its internal code
> that catches NOIOCTLCMD and changes it before user space sees it
>
> mwave_Read should be -EINVAL not -ENOSYS (ENOSYS means the entire read
> syscall
> in the OS isnt there)
>
> In debug mode mwave_write accesses user space directly and may crash
> (buf[0])
>
> Trivial item - coding style uses foo(void) not foo() to indicate functions
> taking no arguments
>
> Still have globals like "dspio" "uartio"  "ClaimResources" etc
>
> whats wrong with tp3780_uart_io etc for globals ?
>
> Otherwise it looks close to ready
>
> Alan
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

-- 

Paul B Schroeder
paulsch@haywired.net

Re: [PATCH] ACP Modem (Mwave)

[Thomas Hood]

FYI here's a data point.

I just tried this latest driver and it builds properly; it
correctly induces the serial driver to create /dev/tts/1
when mwave.o loads and to delete it when mwave.o unloads;
and it allows me to establish a connection.  No problems
encountered so far on:
   ThinkPad 600 51U (266 MHz Pentium II processor)
   linux-2.4.6-ac2 + Mwave driver patch 20010718

MWAVE DRIVER USERS!  Please note that the name of the module
has changed from mwavedd.o to mwave.o .  You may have to 
change your /etc/modules.conf.  E.g., I had to change the
line 
   alias char-major-10-219 mwavedd
to
   alias char-major-10-219 mwave

Although the driver is called "mwave", it does not support
all functions of all Mwave DSPs.  Currently it only supports
the 3780i Mwave DSP and only for the purpose of Hayes-
compatible modem implementation.  The 3780i DSP is found on
certain IBM ThinkPad laptop models, including the ThinkPad 600.

I would like to take this opportunity to thank Paul Schroeder
Mike Sullivan, and IBM for contributing this GPLed driver to
Linux.

--
Thomas Hood

paulsch@haywired.net wrote:
> 
> Oh yeah...
> 
> It can be gotten here:
> 
> http://oss.software.ibm.com/acpmodem/
> 
> Directly:
> http://oss.software.ibm.com/pub/acpmodem/mwave_linux-2.4.6.patch-20010718
> 
> On Wed, 18 Jul 2001, Paul Schroeder wrote:
> 
> > Okay..  I cleaned things up...  I was more careful about the globals this
> > time...
> >
> > Also...  The driver now builds as mwave.o instead of mwavedd.o...  The
> > driver now registers it's UART as a serial device (Thomas Hood)..  So there
> > is no need to run setserial anymore...
> >
> > Cheers...Paul...

Re: [PATCH] ACP Modem (Mwave)

[Robert Love]

I thought I would add
one more datapoint ...
kernel 2.4.6-ac5 +
mwave-patch-20010718 on
IBM ThinkPad 600 41U:
works fine thus far.

The userspace tools,
init script, etc also
work flawless on the
current RedHat Rawhide
(beta for 7.2).

[12:16:52]rml@icbm:~#
cat /proc/version 
Linux version 2.4.6-ac5
(rml@icbm) (gcc version
2.96 20000731 (Red Hat
Linux 7.1 2.96-94)) #1
Tue Jul 17 16:16:14 EDT
2001

I don't use modules (I
enabled them
specifically for this),
so I would like to see
this able to compile
into the kernel.  To
that effect, I think
this should be submitted
for inclusion into the
kernel proper.  It is
GPL now, and it is
apparently stable.
Perhaps Alan can pick it
up for the -ac tree?

Once its in the kernel
RedHat and others need
to be encouraged to
include packages in
their distribution :) --
i see SuSE does already.

Good job IBM.

-- 
Robert M. Love
rml at ufl.edu
rml at tech9.net

[INFO] NNTP access to linux-kernel for Sympatico subscribers

[Thomas Hood]

FYI I just stumbled upon a newsgroup on Sympatico's news server
(news1.sympatico.ca, but you should use the server assigned to
you) that is subscribed to linux-kernel.  (Sympatico is a major
Canadian ISP.)  The newsgroup is called "linux.kernel".

Thomas Hood