Adding new driver in Linux 2.6 - read fails with -1

Adding new driver in Linux 2.6 - read fails with -1

[Ramkumar J]

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

Hi All,

I m using the Linux 2.6(2.6.23-rc2) from Grants for ML-403 and I tried to
add a new driver for a hardware based stream. When I created the device node
and executed the application, I could see that the printk's in the driver
are getting executed for open, release and ioctl. For read, the function
doesnt seem calling the drivers read routine and exits with -1. However the
same is called when I do a cat < /dev/devnode. Am I missing something ?

--------------------------------------------------------------------------------------------------------------
<Code Snip - Driver>

static struct file_operations co_stream_fops = {
        .owner = THIS_MODULE,
        .open = co_stream_open,
        .read = co_stream_read,
        .write = co_stream_write,
//      .ioctl = co_stream_ioctl,
        .release = co_stream_release
};

static int __init co_stream_init(void)
{
    unsigned int *tmp;
    int i=0;
    dbprintk("Welcome !!!\n");

    if (!request_mem_region(STREAM_PHY_ADDR, STREAM_PHY_SIZE, DRIVER_NAME) )
    {
        printk("Failed to lock the memory...\n");
        goto out;
    }

    if (!(remapped_address = ioremap(STREAM_PHY_ADDR, STREAM_PHY_SIZE)) )
    {
        printk("Failed to remap...\n");
        goto out;
    }

    printk("Address remapped to 0x%08X.\n", remapped_address);

    Major = register_chrdev( 0, DEVICE_NAME, &co_stream_fops);    <=========
Registered fops

    if (Major < 0)
    {
        printk("Registration of the device failed.\n\n");
        goto out;
    }

}


static ssize_t co_stream_read(struct file *filp, char __user *buf, size_t
size, loff_t *l)
{
        printk("\n co_stream_read() is called.\n");
        int data,status;

        /* Wait while empty. */
        while (((status=readl( ((volatile unsigned char*) remapped_address)
+ 8)&3))==0)
...
--------------------------------------------------------------------------------------------------------------
<Code Snip - Application>


    printf("CPU listening for hello...\n\r");
    for ( i = 0; i < 10; i++ ) {
        cstream_read(hello_in, &hi, sizeof(int));
        printf("FPGA hardware says: %d\n\r", hi);
    }

int cstream_read(cstream stream, void *buffer, int size)

{
  int nbytes;
  /* TO ADD : Error handing calls here */

  printf("Value of stream = 0x%08X, stream->fd = %d. \n",
                                        stream, stream->fd);

  printf("Value of buffer = 0x%08X, size = %d. \n", buffer, size );

  if (stream) {
      nbytes = read(stream->fd, buffer, size);
      printf("read bytes from hardware = %d.\n", nbytes);
  }
  else
      printf("stream is NULL here.\n");

  return(0);
}

--------------------------------------------------------------------------------------------------------------
<From System.map>

c00de4d0 t sysrq_handle_reboot
c00de4fc T handle_sysrq
c00de524 t co_stream_write
c00de550 t co_stream_release
c00de598 t co_stream_open
c00de624 t co_stream_read
c00de760 t __uart_start

----------------------------------------
<From console output fops dump>

[    0.383898] Welcome !!!
[    0.384445] Address remapped to 0xC5000000.
[    0.384628]
[    0.384653]
[    0.384675]  Impulse Costream assigned with Major = 254.
[    0.384743] Memory dump of fops structure.
[    0.384803] Address : 0xC01CA350, 0x00000000.
[    0.384867] Address : 0xC01CA354, 0x00000000.
[    0.384932] Address : 0xC01CA358, 0xC00DE624.
[    0.384996] Address : 0xC01CA35C, 0xC00DE524.
[    0.385060] Address : 0xC01CA360, 0x00000000.
[    0.385122] Address : 0xC01CA364, 0x00000000.
[    0.385185] Address : 0xC01CA368, 0x00000000.
[    0.385248] Address : 0xC01CA36C, 0x00000000.
[    0.385311] Address : 0xC01CA370, 0x00000000.
[    0.385374] Address : 0xC01CA374, 0x00000000.
[    0.385437] Address : 0xC01CA378, 0x00000000.
[    0.385499] Address : 0xC01CA37C, 0x00000000.
[    0.385565] Address : 0xC01CA380, 0xC00DE598.
[    0.385627] Address : 0xC01CA384, 0x00000000.
[    0.385693] Address : 0xC01CA388, 0xC00DE550.
[    0.385756] Address : 0xC01CA38C, 0x00000000.
[    0.385818] Address : 0xC01CA390, 0x00000000.
[    0.385881] Address : 0xC01CA394, 0x00000000.
[    0.385944] Address : 0xC01CA398, 0x00000000.
[    0.386007] Address : 0xC01CA39C, 0x00000000.
[    0.386070] Address : 0xC01CA3A0, 0x00000000.
[    0.386133] Address : 0xC01CA3A4, 0x00000000.
[    0.386196] Address : 0xC01CA3A8, 0x00000000.
[    0.386259] Address : 0xC01CA3AC, 0x00000000.
Any help or pointers would be helpful.

Thanks and Regards,
Ramkumar.

[-- Attachment #2: Type: text/html, Size: 6784 bytes --]

RE: Adding new driver in Linux 2.6 - read fails with -1

[Jenkins, Clive]

> From: linuxppc-embedded-bounces+clive.jenkins=3Dxerox.com@ozlabs.org
> =
[mailto:linuxppc-embedded-bounces+clive.jenkins=3Dxerox.com@ozlabs.org]
> On Behalf Of Ramkumar J
> Sent: 18 January 2008 10:54
> To: linuxppc-embedded@ozlabs.org
> Subject: Adding new driver in Linux 2.6 - read fails with -1
>
> Hi All,
>
> I m using the Linux 2.6(2.6.23-rc2) from Grants for ML-403 and
> I tried to add a new driver for a hardware based stream.
> [...] For read, the function doesnt seem calling the drivers read
> routine and exits with -1. [...]

You are not giving enough information -- better to supply the complete
code of driver and application, and the complete kernel log, or put
them on a web/ftp site and supply pointers to them.
=20
When your application calls read(), I cannot see whether you are
passing a valid file descriptor, whether you have opened the device
etc.  You say read() returns -1, but you don't give the value of errno
or the error string corresponding to that value. An easy way to get
this is to use perror() in your app.

>         /* Wait while empty. */
>         while (((status=3Dreadl( ((volatile unsigned char*)
remapped_address)
>  + 8)&3))=3D=3D0)=20

One thing I can see is that you have a "busy wait" loop in your
driver's read method. This will waste CPU time and may hang
indefinitely. Better to use wait-event_interruptible, and check for
termination of the wait by unexpected events such as signals and
changes of power management state.

Clive

Re: Adding new driver in Linux 2.6 - read fails with -1

[Ramkumar J]

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

On Jan 18, 2008 7:22 PM, Jenkins, Clive <Clive.Jenkins@xerox.com> wrote:

> > From: linuxppc-embedded-bounces+clive.jenkins=xerox.com@ozlabs.org
> > [mailto:linuxppc-embedded-bounces+clive.jenkins=xerox.com@ozlabs.org]
> > On Behalf Of Ramkumar J
> > Sent: 18 January 2008 10:54
> > To: linuxppc-embedded@ozlabs.org
> > Subject: Adding new driver in Linux 2.6 - read fails with -1
> >
> > Hi All,
> >
> > I m using the Linux 2.6(2.6.23-rc2) from Grants for ML-403 and
> > I tried to add a new driver for a hardware based stream.
> > [...] For read, the function doesnt seem calling the drivers read
> > routine and exits with -1. [...]
>
> You are not giving enough information -- better to supply the complete
> code of driver and application, and the complete kernel log, or put
> them on a web/ftp site and supply pointers to them.
>
> When your application calls read(), I cannot see whether you are
> passing a valid file descriptor, whether you have opened the device
> etc.  You say read() returns -1, but you don't give the value of errno
> or the error string corresponding to that value. An easy way to get
> this is to use perror() in your app.
>
> >         /* Wait while empty. */
> >         while (((status=readl( ((volatile unsigned char*)
> remapped_address)
> >  + 8)&3))==0)
>
> One thing I can see is that you have a "busy wait" loop in your
> driver's read method. This will waste CPU time and may hang
> indefinitely. Better to use wait-event_interruptible, and check for
> termination of the wait by unexpected events such as signals and
> changes of power management state.
>
> Clive
>



Hi,

Thanks a lot for replying. I will try using the perror() function and modify
the busy-loop through wait-for functions. Also, the way I have accessed the
memory mapped IO, is to ioremap() the physical address and then use readl()
functions. Is this correct.

Following is the link to the driver file,
http://www.geocities.com/ramkumarj_2000/impulse-stream.c
http://www.geocities.com/ramkumarj_2000/1.txt as impulse-stream.h
This is intermediate layer and part of my application space,
http://www.geocities.com/ramkumarj_2000/co_stream_linux.c

The application is,
http://www.geocities.com/ramkumarj_2000/HelloWorld.c
http://www.geocities.com/ramkumarj_2000/HelloWorld_sw.c

Logs at,
http://www.geocities.com/ramkumarj_2000/logs.txt
http://www.geocities.com/ramkumarj_2000/kernellog.txt

My apologies for the dirty code. To add some background, it starts from the
main() in HelloWorld.c and it proceeds calling the hear_hello() in
HelloWorld_sw.c . The co_stream_* functions are defined in intermediate
layer co_stream_linux.c and co_stream_open is defined to nothing in one
header file. The open and ioctl call ( modified a little in the driver now
for debugging) map from co_stream_attach.
co_stream_read is read and co_stream_write is the write to driver.

Though the logs indicate the call of release function before the read, I m
not quite sure about this as the read not being called have failed to get
inside the kernel messages.

Thanks and Regards,
Ramkumar.

[-- Attachment #2: Type: text/html, Size: 4585 bytes --]

RE: Adding new driver in Linux 2.6 - read fails with -1

[Jenkins, Clive]

> From: Ramkumar J [mailto:ramkumarj2000@gmail.com]=20
> Sent: 18 January 2008 12:29
> To: Jenkins, Clive
> Cc: linuxppc-embedded@ozlabs.org
> Subject: Re: Adding new driver in Linux 2.6 - read fails with -1
> [...]
>=20
> Hi,
>=20
> Thanks a lot for replying. I will try using the perror() function and
> modify the  busy-loop through wait-for functions. Also, the way I have
> accessed the memory  mapped IO, is to ioremap() the physical address
> and then use readl() functions.  Is this correct.=20
>=20
> Following is the link to the driver file,
> http://www.geocities.com/ramkumarj_2000/impulse-stream.c
> http://www.geocities.com/ramkumarj_2000/1.txt as impulse-stream.h
>
> This is intermediate layer and part of my application space,
> http://www.geocities.com/ramkumarj_2000/co_stream_linux.c
>=20
> The application is,
> http://www.geocities.com/ramkumarj_2000/HelloWorld.c
> http://www.geocities.com/ramkumarj_2000/HelloWorld_sw.c
>=20
> Logs at,
> http://www.geocities.com/ramkumarj_2000/logs.txt
> http://www.geocities.com/ramkumarj_2000/kernellog.txt
>=20
> My apologies for the dirty code. To add some background, it starts
> from the main() in HelloWorld.c and it proceeds calling the
> hear_hello() in HelloWorld_sw.c . The co_stream_* functions are
> defined in intermediate layer co_stream_linux.c and co_stream_open
> is defined to nothing in one header file. The open and ioctl call
> ( modified a little in the driver now for debugging) map from
> co_stream_attach.=20
> co_stream_read is read and co_stream_write is the write to driver.
=20
> Though the logs indicate the call of release function before the
> read, I m not quite sure about this as the read not being called
> have failed to get inside the kernel messages.

I still can't see everything because the header files are not shown.

But anyway, it is getting too complex. I think you need to cut the code
down to the mininum that causes the error if you want help from the
list.

A couple of hints regarding the driver's read method:
1. If you really want to copy different data sizes into the same
variable
then use a union.
2. *(char*)buffer=3Dx puts x where buffer points to; I think you wanted
to put x into the variable buffer: *(char*)&buffer =3D x, but this is
not very clear -- better to use a union.
3. copy_{to,from)_user() return values that should be checked. Always
check function return values (except maybe printf and printk), and
display all error information.

Clive