| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100830162435.GB2566@local>
Date: Mon, 30 Aug 2010 18:24:35 +0200
From: "Hans J. Koch" <hjk@...utronix.de>
To: Armin Steinhoff <armin@...inhoff.de>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"Hans J. Koch" <hjk@...utronix.de>
Subject: Re: UIO and Fedora 13 (kernel 33.6)
On Mon, Aug 30, 2010 at 12:49:10PM +0200, Armin Steinhoff wrote:
> Hi,
>
> I'm writing an UIO driver for a plain PC/104 board (ISA bus).
> After insmod <my_driver_mod> I don't see an entry of uio0 in /dev
> and also no entries in /sys/class. There are no error messages at
> module load.
Are you sure your probe() function is called? After a successfull
uio_register_device() there is both a /dev/uioX and a directory
/sys/class/uio/uioX/.
>
> The same happens after loading the module uio.ko and uio_pdrv.ko ...
> no entries at all, no error messages.
uio_pdrv needs platform data set up somewhere, did you do that?
See docs in Documentation/DocBook/ for more details.
>
> In the attachment is the kernel part of the driver. What could be
> the problem?
Some comments below.
>
> --Armin
>
> a-steinhoff-at-web-de
> /*
> * UIO CAN L2
> *
> * (C) Armin Steinhoff <as@...inhoff-automation.com>
> * (C) 2007 Hans J. Koch <hjk@...utronix.de>
> * Original code (C) 2005 Benedikt Spranger <b.spranger@...utronix.de>
> *
> * Licensed under GPL version 2 only.
> *
> */
>
> #include <linux/device.h>
> #include <linux/module.h>
> #include <linux/uio_driver.h>
> #include <linux/platform_device.h>
> #include <linux/moduleparam.h>
> #include <asm/io.h>
#include <linux/io.h>, please.
>
> #define DEBUG 1
What's that used for?
> #define DRIVER_MAJOR 240
not needed.
> #define INT_QUEUE_SIZE 64
What's that used for?
>
> static struct uio_info *info;
Are you sure you can have only one instance of this driver?
> static unsigned char int_x[2], * int_q[2];
No space between * and variable name. There are more cases below.
Please run your patch through checkpatch.pl and fix the issues.
> static void __iomem *isr[2];
>
> static unsigned int irq = 11;
> module_param (irq, uint, 11);
> MODULE_PARM_DESC (irq, "IRQ (default 11)");
>
> static unsigned long base_addr = 0xd00000;
> module_param (base_addr, ulong, 0xd00000);
> MODULE_PARM_DESC (base_addr, "Base address (default 0xD0000)");
>
> static unsigned long base_len;
> module_param (base_len, ulong, 0x1);
> MODULE_PARM_DESC (base_len, "Base length (default 0x1)");
>
> static struct platform_device * uio_jand_device;
> static int jand_probe(struct device *dev);
> static int jand_remove(struct device *dev);
These declarations are not neeeded...
>
> static struct device_driver uio_jand_driver =
> {
> .name = "uio_jand",
> .bus = &platform_bus_type,
> .probe = jand_probe,
> .remove = jand_remove,
> };
...if you move this struct to the end of the file.
>
> static irqreturn_t int_handler(int irq, struct uio_info *dev_info)
> {
> int irq_flag = 0;
> unsigned char ISRstat;
>
> ISRstat = readb(isr[0]);
> if(ISRstat & 0x02)
> {
> int_q[0][int_x[0]] = ISRstat;
> int_x[0] = (int_x[0] + 1) & 0xF ; // modulo 16
>
> irq_flag = 1;
> }
>
> ISRstat = readb(isr[1]);
> if(ISRstat & 0x02)
> {
> int_q[1][int_x[1]] = ISRstat;
> int_x[1] = (int_x[1] + 1) & 0xF ; // modulo 16
> irq_flag = 1;
> }
>
> if(irq_flag)
> return(IRQ_HANDLED);
> else
> return(IRQ_NONE);
> }
>
> static int jand_probe(struct device *dev)
> {
> info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
info should be a local variable. If you probe the driver twice
you'll overwrite the previous value of info and cause a memory leak.
> if (!info)
> {
> return -ENOMEM;
> }
>
> info->mem[0].addr = base_addr;
> info->mem[0].size = base_len;
> info->mem[0].memtype = UIO_MEM_PHYS;
>
> info->mem[0].internal_addr = ioremap(info->mem[0].addr,info->mem[0].size);
> if (!info->mem[0].internal_addr)
> goto out_release;
>
> /* interrupt queue */
What does this comment mean?
> info->mem[1].addr = (unsigned long)kmalloc(64, GFP_KERNEL);
> if (!info->mem[1].addr)
> goto out_release1;
>
> int_q[0] = (unsigned char * )info->mem[1].addr;
> int_q[1] = (unsigned char * )info->mem[1].addr +32;
>
> memset(&int_q[0], 0x00, 16);
> int_x[0] = 0;
> memset(&int_q[1], 0x00, 16);
> int_x[1] = 0;
>
> info->mem[1].memtype = UIO_MEM_LOGICAL;
> info->mem[1].size = 64;
>
> isr[0] = info->mem[0].internal_addr + 3; /* interrupt reg channel 1 */
> isr[1] = info->mem[0].internal_addr + 3 + 0x200; /* interrupt reg channel 2 */
>
> info->name = "uio_jand";
> info->version = "0.0.1";
> info->irq = irq;
> info->irq_flags = 0;
> info->handler = int_handler;
>
> if (uio_register_device(dev, info))
> goto out_release2;
> printk("uio_jand started!\n");
please use dev_info instead of printk
> return 0;
>
>
> out_release2:
> kfree((void *)info->mem[1].addr);
> printk("uio_register_device failed!\n");
dev_err please.
> out_release1:
> free_irq( irq, "uio_jand" );
> iounmap(info->mem[0].internal_addr);
> release_mem_region( base_addr, base_len);
> out_release:
> kfree (info);
> printk("Error exit ENODEV! \n");
dev_err and correct indentation, please.
> return -ENODEV;
> }
>
> static int jand_remove(struct device *dev)
> {
> uio_unregister_device(info);
> iounmap(info->mem[0].internal_addr);
> release_mem_region( base_addr, base_len);
> free_irq( irq, "uio_jand" );
> kfree((void *)info->mem[1].addr);
> kfree (info);
> return 0;
> }
>
>
> static int __init uio_jand_init(void)
> {
> uio_jand_device = platform_device_register_simple("uio_jand", -1,NULL, 0);
> return driver_register(&uio_jand_driver);
Please check the return value of both *_register calls.
> }
>
> static void __exit uio_jand_exit(void)
> {
> platform_device_unregister(uio_jand_device);
> driver_unregister(&uio_jand_driver);
> }
>
> module_init( uio_jand_init );
> module_exit( uio_jand_exit );
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("A. Steinhoff");
> MODULE_DESCRIPTION("UIO Janus-D CAN driver");
If "CAN" means "Controller Area Network" it should probably be a
socketcan driver instead of UIO...
Thanks,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists