linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Using uio_pdrv to create an platform device for an FPGA, mmap() fails
@ 2012-08-29 23:19 Worth, Kevin
  2012-08-30  3:27 ` Hans J. Koch
  0 siblings, 1 reply; 8+ messages in thread
From: Worth, Kevin @ 2012-08-29 23:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: Worth, Kevin

I have below what appears to be a mostly-functional device using the UIO Platform Driver. The /sys entries I'd expect appear,  /proc/iomem contains " d0000000-d0000fff : myfpga", and lsuio sees the properties that I've set. However an mmap() from userspace (either my test program below or lsuio) fails. So close! (at least it would seem)

Finding a good example for this did not come easily (which is why I'm hitting LKML, since I know it'll get a lot of eyeballs and be archived). Maybe there was something obvious I missed, but the only "official" documentation I could locate was http://www.kernel.org/doc/htmldocs/uio-howto.html#using_uio_pdrv , and most everything else was snippets from presentations, papers, and forums that lacked completeness.

Thanks for the help any might be able to offer.
Please CC me on replies as I'm not ready to drink from the fire hose that is an LKML subscription.

# lsuio -v -m
uio0: name=uio_myfpga, version=0.1, events=0
        map[0]: addr=0xD0000000, size=4096, mmap test: FAILED
        Device attributes:
        uevent=DRIVER=uio_pdrv
        modalias=platform:uio_pdrv

------Kernelspace portion-------

#include <linux/platform_device.h>
#include <linux/uio_driver.h>
#include <linux/module.h>

#define MYFPGA_BASE     0xd0000000 // 3G
#define MYFPGA_SIZE     0x00040000 // 256k

static struct resource myfpga_resources[] = {
        {
                .start  = MYFPGA_BASE,
                .end    = MYFPGA_BASE + MYFPGA_SIZE - 1,
                .name   = "myfpga",
                .flags  = IORESOURCE_MEM
        }
};

static struct uio_info myfpga_uio_info = {
   .name = "uio_myfpga",
   .version = "0.1",
   .irq = UIO_IRQ_CUSTOM,
   .mem = {
        {
            .name = "myfpga",
            .memtype = UIO_MEM_PHYS,
            .addr = MYFPGA_BASE,
            .size = MYFPGA_SIZE
        }
    }
};

static struct platform_device_info myfpga_uio_pdevinfo = {
    .name = "uio_pdrv",
    .id = -1,
    .res = myfpga_resources,
    .num_res = 1,
    .data = &myfpga_uio_info,
    .size_data = sizeof(struct uio_info)
};

static struct platform_device *myfpga_uio_pdev;

static int __init myfpga_init(void)
{
    myfpga_uio_pdev = platform_device_register_full(&myfpga_uio_pdevinfo);
    if (IS_ERR(myfpga_uio_pdev)) {
        return PTR_ERR(myfpga_uio_pdev);
    }

    return 0;
}

static void __exit myfpga_exit(void)
{
    platform_device_unregister(myfpga_uio_pdev);
}

module_init(myfpga_init);
module_exit(myfpga_exit);

------Userspace portion-------

#include <sys/types.h>
#include <sys/mman.h>
#include <sys/stat.h>

#include <dirent.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>

#define MYFPGA_BASE     0xd0000000 // 3G
#define MYFPGA_SIZE     0x00040000 // 256k
#define MYFPGA_UIO_NUM  0          // uio0

int main (int argc, char *argv[])
{
    int fd;
    void *iomem;
    fd = open("/dev/uio0", O_RDWR|O_SYNC);
    if (fd < 0) {
        printf("failed to open /dev/uio0, quitting\n");
        return -1;
    }
    /* Note offset has a special meaning with uio devices */
    iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
                 MYFPGA_UIO_NUM * getpagesize());
    if (iomem == MAP_FAILED) {
        printf("mmap failed, quitting\n");
        close(fd);
        return -2;
    }
    printf("mmap successful!\n");
    munmap(iomem, MYFPGA_SIZE);
    close(fd);
    return 0;
}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-29 23:19 Using uio_pdrv to create an platform device for an FPGA, mmap() fails Worth, Kevin
@ 2012-08-30  3:27 ` Hans J. Koch
  2012-08-30 18:36   ` Worth, Kevin
  0 siblings, 1 reply; 8+ messages in thread
From: Hans J. Koch @ 2012-08-30  3:27 UTC (permalink / raw)
  To: Worth, Kevin; +Cc: linux-kernel, u.kleine-koenig

[Added driver author to Cc:]

On Wed, Aug 29, 2012 at 11:19:00PM +0000, Worth, Kevin wrote:
> I have below what appears to be a mostly-functional device using the UIO Platform Driver. The /sys entries I'd expect appear,  /proc/iomem contains " d0000000-d0000fff : myfpga", and lsuio sees the properties that I've set. However an mmap() from userspace (either my test program below or lsuio) fails. So close! (at least it would seem)
> 
> Finding a good example for this did not come easily (which is why I'm hitting LKML, since I know it'll get a lot of eyeballs and be archived). Maybe there was something obvious I missed, but the only "official" documentation I could locate was http://www.kernel.org/doc/htmldocs/uio-howto.html#using_uio_pdrv , and most everything else was snippets from presentations, papers, and forums that lacked completeness.
> 
> Thanks for the help any might be able to offer.
> Please CC me on replies as I'm not ready to drink from the fire hose that is an LKML subscription.
> 
> # lsuio -v -m
> uio0: name=uio_myfpga, version=0.1, events=0
>         map[0]: addr=0xD0000000, size=4096, mmap test: FAILED
>         Device attributes:
>         uevent=DRIVER=uio_pdrv
>         modalias=platform:uio_pdrv
> 
> ------Kernelspace portion-------
> 
> #include <linux/platform_device.h>
> #include <linux/uio_driver.h>
> #include <linux/module.h>
> 
> #define MYFPGA_BASE     0xd0000000 // 3G
> #define MYFPGA_SIZE     0x00040000 // 256k
> 
> static struct resource myfpga_resources[] = {
>         {
>                 .start  = MYFPGA_BASE,
>                 .end    = MYFPGA_BASE + MYFPGA_SIZE - 1,
>                 .name   = "myfpga",
>                 .flags  = IORESOURCE_MEM
>         }
> };
> 
> static struct uio_info myfpga_uio_info = {
>    .name = "uio_myfpga",
>    .version = "0.1",
>    .irq = UIO_IRQ_CUSTOM,
>    .mem = {
>         {
>             .name = "myfpga",
>             .memtype = UIO_MEM_PHYS,
>             .addr = MYFPGA_BASE,
>             .size = MYFPGA_SIZE
>         }
>     }
> };
> 
> static struct platform_device_info myfpga_uio_pdevinfo = {
>     .name = "uio_pdrv",
>     .id = -1,
>     .res = myfpga_resources,
>     .num_res = 1,
>     .data = &myfpga_uio_info,
>     .size_data = sizeof(struct uio_info)
> };
> 
> static struct platform_device *myfpga_uio_pdev;
> 
> static int __init myfpga_init(void)
> {
>     myfpga_uio_pdev = platform_device_register_full(&myfpga_uio_pdevinfo);
>     if (IS_ERR(myfpga_uio_pdev)) {
>         return PTR_ERR(myfpga_uio_pdev);
>     }
> 
>     return 0;
> }
> 
> static void __exit myfpga_exit(void)
> {
>     platform_device_unregister(myfpga_uio_pdev);
> }
> 
> module_init(myfpga_init);
> module_exit(myfpga_exit);
> 
> ------Userspace portion-------
> 
> #include <sys/types.h>
> #include <sys/mman.h>
> #include <sys/stat.h>
> 
> #include <dirent.h>
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <fcntl.h>
> #include <unistd.h>
> 
> #define MYFPGA_BASE     0xd0000000 // 3G
> #define MYFPGA_SIZE     0x00040000 // 256k
> #define MYFPGA_UIO_NUM  0          // uio0

That's misleading regarding its use below. The factor you need for mmap
is the number of the mapping, not uio0, uio1...

> 
> int main (int argc, char *argv[])
> {
>     int fd;
>     void *iomem;
>     fd = open("/dev/uio0", O_RDWR|O_SYNC);

Does it work with O_RDWR ?

Thanks,
Hans

>     if (fd < 0) {
>         printf("failed to open /dev/uio0, quitting\n");
>         return -1;
>     }
>     /* Note offset has a special meaning with uio devices */
>     iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
>                  MYFPGA_UIO_NUM * getpagesize());
>     if (iomem == MAP_FAILED) {
>         printf("mmap failed, quitting\n");
>         close(fd);
>         return -2;
>     }
>     printf("mmap successful!\n");
>     munmap(iomem, MYFPGA_SIZE);
>     close(fd);
>     return 0;
> }
> --
> 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/
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30  3:27 ` Hans J. Koch
@ 2012-08-30 18:36   ` Worth, Kevin
  2012-08-30 19:00     ` Hans J. Koch
  0 siblings, 1 reply; 8+ messages in thread
From: Worth, Kevin @ 2012-08-30 18:36 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linux-kernel, u.kleine-koenig

Thanks for the reply, Hans. Your question about opening /dev/uio0 O_RDWR
prompted me to check out how I was creating /dev/uio0 ... my system
isn't using udev, and I was accidentally creating it with major/minor
number 254/0 instead of the correct 253/0 (found by looking at
/proc/devices). Fixed that and the mmap() call started working.

Verified that if /dev/uio0 has permissions 0644, root can open it O_RDWR
and mmap PROT_READ | PROT_WRITE using the below code and write to an
address within my memory map. Of course this contradicts the statement
"/dev/uioX is a read-only file" in the UIO howto.

Including my updated, tested code for completeness.
Note I also cleaned up the device registration a little by
using a different platform_device_register_ call and removing fields
in the struct uio_info that get filled in by uio_pdrv automatically.

-Kevin

# lsuio -m -v
uio0: name=uio_myfpga, version=0.1, events=0
        map[0]: addr=0xD0000000, size=262144, mmap test: OK
        Device attributes:
        uevent=DRIVER=uio_pdrv
        modalias=platform:uio_pdrv

------Kernelspace------
#include <linux/platform_device.h>
#include <linux/uio_driver.h>
#include <linux/module.h>

#define MYFPGA_BASE     0xd0000000 // 3G
#define MYFPGA_SIZE     0x00040000 // 256k

static struct resource myfpga_resources[] = {
    {
        .start = MYFPGA_BASE,
        .end   = MYFPGA_BASE + MYFPGA_SIZE - 1,
        .name  = "myfpga",
        .flags = IORESOURCE_MEM
    }
};

static struct uio_info myfpga_uio_info = {
   .name = "uio_myfpga",
   .version = "0.1",
};

static struct platform_device *myfpga_uio_pdev;

static int __init myfpga_init(void)
{
    myfpga_uio_pdev = platform_device_register_resndata (NULL,
                                                         "uio_pdrv",
                                                         -1,
                                                         myfpga_resources,
                                                         1,
                                                         &myfpga_uio_info,
                                                         sizeof(struct uio_info)
                                                        );
    if (IS_ERR(myfpga_uio_pdev)) {
        return PTR_ERR(myfpga_uio_pdev);
    }

    return 0;
}

static void __exit myfpga_exit(void)
{
    platform_device_unregister(myfpga_uio_pdev);
}

module_init(myfpga_init);
module_exit(myfpga_exit);

------Userspace-------
#include <sys/types.h>
#include <sys/mman.h>
#include <sys/stat.h>

#include <dirent.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdint.h>

#define MYFPGA_BASE     0xd0000000 // 3G
#define MYFPGA_SIZE     0x00040000 // 256k
#define MYFPGA_MAP_NUM  0 // First and only defined map

#define BIT32(n) (1 << (n))

/* Use mmap()'ped address "iomem", not physical MYFPGA address */
#define MYFPGA_REG(iomem) (volatile uint32_t*)(iomem + 0x8) // Third 32-bit reg

int main (int argc, char *argv[])
{
    int fd;
    void *iomem;
    fd = open("/dev/uio0", O_RDWR|O_SYNC);
    if (fd < 0) {
        printf("failed to open /dev/uio0, quitting\n");
        return -1;
    }
    /* Note offset has a special meaning with uio devices */
    iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
                 MYFPGA_MAP_NUM * getpagesize());
    if (iomem == MAP_FAILED) {
        printf("mmap failed, quitting\n");
        close(fd);
        return -2;
    }

    /* Set bit 5 of MYFPGA_REG register */
    *MYFPGA_REG(iomem) |= BIT32(5);

    munmap(iomem, MYFPGA_SIZE);
    close(fd);
    return 0;
}
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30 18:36   ` Worth, Kevin
@ 2012-08-30 19:00     ` Hans J. Koch
  2012-08-30 20:10       ` Worth, Kevin
  0 siblings, 1 reply; 8+ messages in thread
From: Hans J. Koch @ 2012-08-30 19:00 UTC (permalink / raw)
  To: Worth, Kevin; +Cc: Hans J. Koch, linux-kernel, u.kleine-koenig

On Thu, Aug 30, 2012 at 06:36:53PM +0000, Worth, Kevin wrote:
> Thanks for the reply, Hans. Your question about opening /dev/uio0 O_RDWR
> prompted me to check out how I was creating /dev/uio0 ... my system
> isn't using udev, and I was accidentally creating it with major/minor
> number 254/0 instead of the correct 253/0 (found by looking at
> /proc/devices). Fixed that and the mmap() call started working.

Good.

> 
> Verified that if /dev/uio0 has permissions 0644, root can open it O_RDWR
> and mmap PROT_READ | PROT_WRITE using the below code and write to an
> address within my memory map. Of course this contradicts the statement
> "/dev/uioX is a read-only file" in the UIO howto.

You're right. That wants to be fixed...

> 
> Including my updated, tested code for completeness.
> Note I also cleaned up the device registration a little by
> using a different platform_device_register_ call and removing fields
> in the struct uio_info that get filled in by uio_pdrv automatically.

If you want to have that included in the mainline, please choose a more
descriptive name than "myfpga" and send a proper patch.

Thanks,
Hans

> 
> -Kevin
> 
> # lsuio -m -v
> uio0: name=uio_myfpga, version=0.1, events=0
>         map[0]: addr=0xD0000000, size=262144, mmap test: OK
>         Device attributes:
>         uevent=DRIVER=uio_pdrv
>         modalias=platform:uio_pdrv
> 
> ------Kernelspace------
> #include <linux/platform_device.h>
> #include <linux/uio_driver.h>
> #include <linux/module.h>
> 
> #define MYFPGA_BASE     0xd0000000 // 3G
> #define MYFPGA_SIZE     0x00040000 // 256k
> 
> static struct resource myfpga_resources[] = {
>     {
>         .start = MYFPGA_BASE,
>         .end   = MYFPGA_BASE + MYFPGA_SIZE - 1,
>         .name  = "myfpga",
>         .flags = IORESOURCE_MEM
>     }
> };
> 
> static struct uio_info myfpga_uio_info = {
>    .name = "uio_myfpga",
>    .version = "0.1",
> };
> 
> static struct platform_device *myfpga_uio_pdev;
> 
> static int __init myfpga_init(void)
> {
>     myfpga_uio_pdev = platform_device_register_resndata (NULL,
>                                                          "uio_pdrv",
>                                                          -1,
>                                                          myfpga_resources,
>                                                          1,
>                                                          &myfpga_uio_info,
>                                                          sizeof(struct uio_info)
>                                                         );
>     if (IS_ERR(myfpga_uio_pdev)) {
>         return PTR_ERR(myfpga_uio_pdev);
>     }
> 
>     return 0;
> }
> 
> static void __exit myfpga_exit(void)
> {
>     platform_device_unregister(myfpga_uio_pdev);
> }
> 
> module_init(myfpga_init);
> module_exit(myfpga_exit);
> 
> ------Userspace-------
> #include <sys/types.h>
> #include <sys/mman.h>
> #include <sys/stat.h>
> 
> #include <dirent.h>
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <stdint.h>
> 
> #define MYFPGA_BASE     0xd0000000 // 3G
> #define MYFPGA_SIZE     0x00040000 // 256k
> #define MYFPGA_MAP_NUM  0 // First and only defined map
> 
> #define BIT32(n) (1 << (n))
> 
> /* Use mmap()'ped address "iomem", not physical MYFPGA address */
> #define MYFPGA_REG(iomem) (volatile uint32_t*)(iomem + 0x8) // Third 32-bit reg
> 
> int main (int argc, char *argv[])
> {
>     int fd;
>     void *iomem;
>     fd = open("/dev/uio0", O_RDWR|O_SYNC);
>     if (fd < 0) {
>         printf("failed to open /dev/uio0, quitting\n");
>         return -1;
>     }
>     /* Note offset has a special meaning with uio devices */
>     iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
>                  MYFPGA_MAP_NUM * getpagesize());
>     if (iomem == MAP_FAILED) {
>         printf("mmap failed, quitting\n");
>         close(fd);
>         return -2;
>     }
> 
>     /* Set bit 5 of MYFPGA_REG register */
>     *MYFPGA_REG(iomem) |= BIT32(5);
> 
>     munmap(iomem, MYFPGA_SIZE);
>     close(fd);
>     return 0;
> }
>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30 19:00     ` Hans J. Koch
@ 2012-08-30 20:10       ` Worth, Kevin
  2012-08-30 22:24         ` Hans J. Koch
  0 siblings, 1 reply; 8+ messages in thread
From: Worth, Kevin @ 2012-08-30 20:10 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linux-kernel, u.kleine-koenig

>> Thanks for the reply, Hans. Your question about opening /dev/uio0 O_RDWR
>> prompted me to check out how I was creating /dev/uio0 ... my system
>> isn't using udev, and I was accidentally creating it with major/minor
>> number 254/0 instead of the correct 253/0 (found by looking at
>> /proc/devices). Fixed that and the mmap() call started working.
>
>Good.
>
>> 
>> Verified that if /dev/uio0 has permissions 0644, root can open it O_RDWR
>> and mmap PROT_READ | PROT_WRITE using the below code and write to an
>> address within my memory map. Of course this contradicts the statement
>> "/dev/uioX is a read-only file" in the UIO howto.
>
>You're right. That wants to be fixed...
>
>> 
>> Including my updated, tested code for completeness.
>> Note I also cleaned up the device registration a little by
>> using a different platform_device_register_ call and removing fields
>> in the struct uio_info that get filled in by uio_pdrv automatically.
>
>If you want to have that included in the mainline, please choose a more
>descriptive name than "myfpga" and send a proper patch.

I wasn't sure about submitting as a patch since it's for a custom FPGA
that I don't expect the community will be using, but the code seems like
possibly useful sample/example code. Perhaps patching the HOWTO like
http://www.kernel.org/doc/htmldocs/uio-howto.html#uio_pci_generic_example
is the right approach?

>
>Thanks,
>Hans
>
>> 
>> -Kevin
>> 
>> # lsuio -m -v
>> uio0: name=uio_myfpga, version=0.1, events=0
>>         map[0]: addr=0xD0000000, size=262144, mmap test: OK
>>         Device attributes:
>>         uevent=DRIVER=uio_pdrv
>>         modalias=platform:uio_pdrv
>> 
>> ------Kernelspace------
>> #include <linux/platform_device.h>
>> #include <linux/uio_driver.h>
>> #include <linux/module.h>
>> 
>> #define MYFPGA_BASE     0xd0000000 // 3G
>> #define MYFPGA_SIZE     0x00040000 // 256k
>> 
>> static struct resource myfpga_resources[] = {
>>     {
>>         .start = MYFPGA_BASE,
>>         .end   = MYFPGA_BASE + MYFPGA_SIZE - 1,
>>         .name  = "myfpga",
>>         .flags = IORESOURCE_MEM
>>     }
>> };
>> 
>> static struct uio_info myfpga_uio_info = {
>>    .name = "uio_myfpga",
>>    .version = "0.1",
>> };
>> 
>> static struct platform_device *myfpga_uio_pdev;
>> 
>> static int __init myfpga_init(void)
>> {
>>     myfpga_uio_pdev = platform_device_register_resndata (NULL,
>>                                                          "uio_pdrv",
>>                                                          -1,
>>                                                          myfpga_resources,
>>                                                          1,
>>                                                          &myfpga_uio_info,
>>                                                          sizeof(struct uio_info)
>>                                                         );
>>     if (IS_ERR(myfpga_uio_pdev)) {
>>         return PTR_ERR(myfpga_uio_pdev);
>>     }
>> 
>>     return 0;
>> }
>> 
>> static void __exit myfpga_exit(void)
>> {
>>     platform_device_unregister(myfpga_uio_pdev);
>> }
>> 
>> module_init(myfpga_init);
>> module_exit(myfpga_exit);
>> 
>> ------Userspace-------
>> #include <sys/types.h>
>> #include <sys/mman.h>
>> #include <sys/stat.h>
>> 
>> #include <dirent.h>
>> #include <string.h>
>> #include <stdlib.h>
>> #include <stdio.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <stdint.h>
>> 
>> #define MYFPGA_BASE     0xd0000000 // 3G
>> #define MYFPGA_SIZE     0x00040000 // 256k
>> #define MYFPGA_MAP_NUM  0 // First and only defined map
>> 
>> #define BIT32(n) (1 << (n))
>> 
>> /* Use mmap()'ped address "iomem", not physical MYFPGA address */
>> #define MYFPGA_REG(iomem) (volatile uint32_t*)(iomem + 0x8) // Third 32-bit reg
>> 
>> int main (int argc, char *argv[])
>> {
>>     int fd;
>>     void *iomem;
>>     fd = open("/dev/uio0", O_RDWR|O_SYNC);
>>     if (fd < 0) {
>>         printf("failed to open /dev/uio0, quitting\n");
>>         return -1;
>>     }
>>     /* Note offset has a special meaning with uio devices */
>>     iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
>>                  MYFPGA_MAP_NUM * getpagesize());
>>     if (iomem == MAP_FAILED) {
>>         printf("mmap failed, quitting\n");
>>         close(fd);
>>         return -2;
>>     }
>> 
>>     /* Set bit 5 of MYFPGA_REG register */
>>     *MYFPGA_REG(iomem) |= BIT32(5);
>> 
>>     munmap(iomem, MYFPGA_SIZE);
>>     close(fd);
>>     return 0;
>> }
>>  
>>  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30 20:10       ` Worth, Kevin
@ 2012-08-30 22:24         ` Hans J. Koch
  2012-08-30 23:03           ` Worth, Kevin
  0 siblings, 1 reply; 8+ messages in thread
From: Hans J. Koch @ 2012-08-30 22:24 UTC (permalink / raw)
  To: Worth, Kevin
  Cc: Hans J. Koch, linux-kernel, u.kleine-koenig, Greg Kroah-Hartman

[Added Greg Kroah-Hartman to Cc:]

On Thu, Aug 30, 2012 at 08:10:11PM +0000, Worth, Kevin wrote:
> >> Thanks for the reply, Hans. Your question about opening /dev/uio0 O_RDWR
> >> prompted me to check out how I was creating /dev/uio0 ... my system
> >> isn't using udev, and I was accidentally creating it with major/minor
> >> number 254/0 instead of the correct 253/0 (found by looking at
> >> /proc/devices). Fixed that and the mmap() call started working.
> >
> >Good.
> >
> >> 
> >> Verified that if /dev/uio0 has permissions 0644, root can open it O_RDWR
> >> and mmap PROT_READ | PROT_WRITE using the below code and write to an
> >> address within my memory map. Of course this contradicts the statement
> >> "/dev/uioX is a read-only file" in the UIO howto.
> >
> >You're right. That wants to be fixed...
> >
> >> 
> >> Including my updated, tested code for completeness.
> >> Note I also cleaned up the device registration a little by
> >> using a different platform_device_register_ call and removing fields
> >> in the struct uio_info that get filled in by uio_pdrv automatically.
> >
> >If you want to have that included in the mainline, please choose a more
> >descriptive name than "myfpga" and send a proper patch.
> 
> I wasn't sure about submitting as a patch since it's for a custom FPGA
> that I don't expect the community will be using,

That doesn't matter. If it helps YOU that the code is maintained in mainline,
post it.

>  but the code seems like
> possibly useful sample/example code.

That is another good argument.

> Perhaps patching the HOWTO like
> http://www.kernel.org/doc/htmldocs/uio-howto.html#uio_pci_generic_example
> is the right approach?

Oh, if you could hack up a patch for the documentation, that would be great.
But please make it a second patch, don't mix it with your driver code.

Thanks,
Hans

> 
> >
> >Thanks,
> >Hans
> >
> >> 
> >> -Kevin
> >> 
> >> # lsuio -m -v
> >> uio0: name=uio_myfpga, version=0.1, events=0
> >>         map[0]: addr=0xD0000000, size=262144, mmap test: OK
> >>         Device attributes:
> >>         uevent=DRIVER=uio_pdrv
> >>         modalias=platform:uio_pdrv
> >> 
> >> ------Kernelspace------
> >> #include <linux/platform_device.h>
> >> #include <linux/uio_driver.h>
> >> #include <linux/module.h>
> >> 
> >> #define MYFPGA_BASE     0xd0000000 // 3G
> >> #define MYFPGA_SIZE     0x00040000 // 256k
> >> 
> >> static struct resource myfpga_resources[] = {
> >>     {
> >>         .start = MYFPGA_BASE,
> >>         .end   = MYFPGA_BASE + MYFPGA_SIZE - 1,
> >>         .name  = "myfpga",
> >>         .flags = IORESOURCE_MEM
> >>     }
> >> };
> >> 
> >> static struct uio_info myfpga_uio_info = {
> >>    .name = "uio_myfpga",
> >>    .version = "0.1",
> >> };
> >> 
> >> static struct platform_device *myfpga_uio_pdev;
> >> 
> >> static int __init myfpga_init(void)
> >> {
> >>     myfpga_uio_pdev = platform_device_register_resndata (NULL,
> >>                                                          "uio_pdrv",
> >>                                                          -1,
> >>                                                          myfpga_resources,
> >>                                                          1,
> >>                                                          &myfpga_uio_info,
> >>                                                          sizeof(struct uio_info)
> >>                                                         );
> >>     if (IS_ERR(myfpga_uio_pdev)) {
> >>         return PTR_ERR(myfpga_uio_pdev);
> >>     }
> >> 
> >>     return 0;
> >> }
> >> 
> >> static void __exit myfpga_exit(void)
> >> {
> >>     platform_device_unregister(myfpga_uio_pdev);
> >> }
> >> 
> >> module_init(myfpga_init);
> >> module_exit(myfpga_exit);
> >> 
> >> ------Userspace-------
> >> #include <sys/types.h>
> >> #include <sys/mman.h>
> >> #include <sys/stat.h>
> >> 
> >> #include <dirent.h>
> >> #include <string.h>
> >> #include <stdlib.h>
> >> #include <stdio.h>
> >> #include <fcntl.h>
> >> #include <unistd.h>
> >> #include <stdint.h>
> >> 
> >> #define MYFPGA_BASE     0xd0000000 // 3G
> >> #define MYFPGA_SIZE     0x00040000 // 256k
> >> #define MYFPGA_MAP_NUM  0 // First and only defined map
> >> 
> >> #define BIT32(n) (1 << (n))
> >> 
> >> /* Use mmap()'ped address "iomem", not physical MYFPGA address */
> >> #define MYFPGA_REG(iomem) (volatile uint32_t*)(iomem + 0x8) // Third 32-bit reg
> >> 
> >> int main (int argc, char *argv[])
> >> {
> >>     int fd;
> >>     void *iomem;
> >>     fd = open("/dev/uio0", O_RDWR|O_SYNC);
> >>     if (fd < 0) {
> >>         printf("failed to open /dev/uio0, quitting\n");
> >>         return -1;
> >>     }
> >>     /* Note offset has a special meaning with uio devices */
> >>     iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> >>                  MYFPGA_MAP_NUM * getpagesize());
> >>     if (iomem == MAP_FAILED) {
> >>         printf("mmap failed, quitting\n");
> >>         close(fd);
> >>         return -2;
> >>     }
> >> 
> >>     /* Set bit 5 of MYFPGA_REG register */
> >>     *MYFPGA_REG(iomem) |= BIT32(5);
> >> 
> >>     munmap(iomem, MYFPGA_SIZE);
> >>     close(fd);
> >>     return 0;
> >> }
> >>  
> >>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30 22:24         ` Hans J. Koch
@ 2012-08-30 23:03           ` Worth, Kevin
  2012-08-31  8:42             ` Hans J. Koch
  0 siblings, 1 reply; 8+ messages in thread
From: Worth, Kevin @ 2012-08-30 23:03 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linux-kernel, u.kleine-koenig, Greg Kroah-Hartman

 >[Added Greg Kroah-Hartman to Cc:]
>
>On Thu, Aug 30, 2012 at 08:10:11PM +0000, Worth, Kevin wrote:
>> >> Thanks for the reply, Hans. Your question about opening /dev/uio0 O_RDWR
>> >> prompted me to check out how I was creating /dev/uio0 ... my system
>> >> isn't using udev, and I was accidentally creating it with major/minor
>> >> number 254/0 instead of the correct 253/0 (found by looking at
>> >> /proc/devices). Fixed that and the mmap() call started working.
>> >
>> >Good.
>> >
>> >> 
>> >> Verified that if /dev/uio0 has permissions 0644, root can open it O_RDWR
>> >> and mmap PROT_READ | PROT_WRITE using the below code and write to an
>> >> address within my memory map. Of course this contradicts the statement
>> >> "/dev/uioX is a read-only file" in the UIO howto.
>> >
>> >You're right. That wants to be fixed...
>> >
>> >> 
>> >> Including my updated, tested code for completeness.
>> >> Note I also cleaned up the device registration a little by
>> >> using a different platform_device_register_ call and removing fields
>> >> in the struct uio_info that get filled in by uio_pdrv automatically.
>> >
>> >If you want to have that included in the mainline, please choose a more
>> >descriptive name than "myfpga" and send a proper patch.
>> 
>> I wasn't sure about submitting as a patch since it's for a custom FPGA
>> that I don't expect the community will be using,
>
>That doesn't matter. If it helps YOU that the code is maintained in mainline,
>post it.
>
>>  but the code seems like
>> possibly useful sample/example code.
>
>That is another good argument.

Perhaps this could be genericized to be a generic "Memory Map Userspace
IO Device" that takes a base address and a length in config (since those
are really the only things that are particular to my device/usage).
Could be enhanced to allow for additional maps, etc. or just serve as a
working example. Docs could then also simply refer to this as an example
of a device that uses the uio_pdrv driver.

>
>> Perhaps patching the HOWTO like
>> http://www.kernel.org/doc/htmldocs/uio-howto.html#uio_pci_generic_example
>> is the right approach?
>
>Oh, if you could hack up a patch for the documentation, that would be great.
>But please make it a second patch, don't mix it with your driver code.

Certainly these would belong in separate patches.

>
>Thanks,
>Hans
>
>> 
>> >
>> >Thanks,
>> >Hans
>> >
>> >> 
>> >> -Kevin
>> >> 
>> >> # lsuio -m -v
>> >> uio0: name=uio_myfpga, version=0.1, events=0
>> >>         map[0]: addr=0xD0000000, size=262144, mmap test: OK
>> >>         Device attributes:
>> >>         uevent=DRIVER=uio_pdrv
>> >>         modalias=platform:uio_pdrv
>> >> 
>> >> ------Kernelspace------
>> >> #include <linux/platform_device.h>
>> >> #include <linux/uio_driver.h>
>> >> #include <linux/module.h>
>> >> 
>> >> #define MYFPGA_BASE     0xd0000000 // 3G
>> >> #define MYFPGA_SIZE     0x00040000 // 256k
>> >> 
>> >> static struct resource myfpga_resources[] = {
>> >>     {
>> >>         .start = MYFPGA_BASE,
>> >>         .end   = MYFPGA_BASE + MYFPGA_SIZE - 1,
>> >>         .name  = "myfpga",
>> >>         .flags = IORESOURCE_MEM
>> >>     }
>> >> };
>> >> 
>> >> static struct uio_info myfpga_uio_info = {
>> >>    .name = "uio_myfpga",
>> >>    .version = "0.1",
>> >> };
>> >> 
>> >> static struct platform_device *myfpga_uio_pdev;
>> >> 
>> >> static int __init myfpga_init(void)
>> >> {
>> >>     myfpga_uio_pdev = platform_device_register_resndata (NULL,
>> >>                                                          "uio_pdrv",
>> >>                                                          -1,
>> >>                                                          myfpga_resources,
>> >>                                                          1,
>> >>                                                          &myfpga_uio_info,
>> >>                                                          sizeof(struct uio_info)
>> >>                                                         );
>> >>     if (IS_ERR(myfpga_uio_pdev)) {
>> >>         return PTR_ERR(myfpga_uio_pdev);
>> >>     }
>> >> 
>> >>     return 0;
>> >> }
>> >> 
>> >> static void __exit myfpga_exit(void)
>> >> {
>> >>     platform_device_unregister(myfpga_uio_pdev);
>> >> }
>> >> 
>> >> module_init(myfpga_init);
>> >> module_exit(myfpga_exit);
>> >> 
>> >> ------Userspace-------
>> >> #include <sys/types.h>
>> >> #include <sys/mman.h>
>> >> #include <sys/stat.h>
>> >> 
>> >> #include <dirent.h>
>> >> #include <string.h>
>> >> #include <stdlib.h>
>> >> #include <stdio.h>
>> >> #include <fcntl.h>
>> >> #include <unistd.h>
>> >> #include <stdint.h>
>> >> 
>> >> #define MYFPGA_BASE     0xd0000000 // 3G
>> >> #define MYFPGA_SIZE     0x00040000 // 256k
>> >> #define MYFPGA_MAP_NUM  0 // First and only defined map
>> >> 
>> >> #define BIT32(n) (1 << (n))
>> >> 
>> >> /* Use mmap()'ped address "iomem", not physical MYFPGA address */
>> >> #define MYFPGA_REG(iomem) (volatile uint32_t*)(iomem + 0x8) // Third 32-bit reg
>> >> 
>> >> int main (int argc, char *argv[])
>> >> {
>> >>     int fd;
>> >>     void *iomem;
>> >>     fd = open("/dev/uio0", O_RDWR|O_SYNC);
>> >>     if (fd < 0) {
>> >>         printf("failed to open /dev/uio0, quitting\n");
>> >>         return -1;
>> >>     }
>> >>     /* Note offset has a special meaning with uio devices */
>> >>     iomem = mmap(NULL, MYFPGA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
>> >>                  MYFPGA_MAP_NUM * getpagesize());
>> >>     if (iomem == MAP_FAILED) {
>> >>         printf("mmap failed, quitting\n");
>> >>         close(fd);
>> >>         return -2;
>> >>     }
>> >> 
>> >>     /* Set bit 5 of MYFPGA_REG register */
>> >>     *MYFPGA_REG(iomem) |= BIT32(5);
>> >> 
>> >>     munmap(iomem, MYFPGA_SIZE);
>> >>     close(fd);
>> >>     return 0;
>> >> }
>> >>  
>> >>  
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Using uio_pdrv to create an platform device for an FPGA, mmap() fails
  2012-08-30 23:03           ` Worth, Kevin
@ 2012-08-31  8:42             ` Hans J. Koch
  0 siblings, 0 replies; 8+ messages in thread
From: Hans J. Koch @ 2012-08-31  8:42 UTC (permalink / raw)
  To: Worth, Kevin
  Cc: Hans J. Koch, linux-kernel, u.kleine-koenig, Greg Kroah-Hartman

On Thu, Aug 30, 2012 at 11:03:04PM +0000, Worth, Kevin wrote:
> >
> >>  but the code seems like
> >> possibly useful sample/example code.
> >
> >That is another good argument.
> 
> Perhaps this could be genericized to be a generic "Memory Map Userspace
> IO Device" that takes a base address and a length in config (since those
> are really the only things that are particular to my device/usage).

I don't see a point in making it more generic than it already is. Other
people's devices generate interrupts or have several mappings. Since these
drivers are so small, especially when using uio_pdrv, it doesn't make sense
to make it more generic.

> Could be enhanced to allow for additional maps, etc. or just serve as a
> working example. Docs could then also simply refer to this as an example
> of a device that uses the uio_pdrv driver.

Just make your driver clean and simple, then refer to it in the docs as one
possible example.

Thanks,
Hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-08-31  8:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-29 23:19 Using uio_pdrv to create an platform device for an FPGA, mmap() fails Worth, Kevin
2012-08-30  3:27 ` Hans J. Koch
2012-08-30 18:36   ` Worth, Kevin
2012-08-30 19:00     ` Hans J. Koch
2012-08-30 20:10       ` Worth, Kevin
2012-08-30 22:24         ` Hans J. Koch
2012-08-30 23:03           ` Worth, Kevin
2012-08-31  8:42             ` Hans J. Koch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).