Basic driver devel questions ?

Basic driver devel questions ?

[Guillaume Dargaud]

Hello all,
is it OK if I ask basic driver development questions here ?
Could you recommend a better forum for that maybe ?
Thanks
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Basic driver devel questions ?

[Michael Ellerman]

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

On Wed, 2010-12-01 at 11:15 +0100, Guillaume Dargaud wrote:
> Hello all,
> is it OK if I ask basic driver development questions here ?
> Could you recommend a better forum for that maybe ?

Hi Guillaume,

I guess it depends how basic they are :)

If they're basic _powerpc_ driver questions then this is probably the
right place.

But I'd say just ask and maybe someone will be able to help, or maybe
they'll point you somewhere else.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

> I guess it depends how basic they are :)
> 
> If they're basic _powerpc_ driver questions then this is probably the
> right place.
> 
> But I'd say just ask and maybe someone will be able to help, or maybe
> they'll point you somewhere else.

OK, here goes then: how do I get the IRQ number so that I can install an 
interrupt handler on it ?

In my dts file I have:
		xps_acqui_data_0: xps-acqui-data@c9800000 {
			compatible = "xlnx,xps-acqui-data-3.00.a";
			interrupt-parent = <&xps_intc_0>;
			interrupts = < 0 2 >;
			reg = < 0xc9800000 0x10000 >;
			xlnx,family = "virtex4";
			xlnx,include-dphase-timer = <0x1>;
			xlnx,mplb-awidth = <0x20>;
			xlnx,mplb-clk-period-ps = <0x2710>;
			xlnx,mplb-dwidth = <0x40>;
			xlnx,mplb-native-dwidth = <0x40>;
			xlnx,mplb-p2p = <0x0>;
			xlnx,mplb-smallest-slave = <0x20>;
		} ;

In my minimal driver init, I have:
  first = MKDEV (my_major, my_minor);
  register_chrdev_region(first, count, NAME);
  cdev_init(my_cdev, &fops);
  cdev_add (my_cdev, first, count);
So far so good.

Now how do I connect the dots between the hardware definitions from the dts and 
my driver ?
I have:
# ll /proc/device-tree/plb@0/xps-acqui-data@c9800000/
-r--r--r--    1 root     root          27 Dec  1 16:26 compatible
-r--r--r--    1 root     root           4 Dec  1 16:26 interrupt-parent
-r--r--r--    1 root     root           8 Dec  1 16:26 interrupts
-r--r--r--    1 root     root          15 Dec  1 16:26 name
-r--r--r--    1 root     root           8 Dec  1 16:26 reg
-r--r--r--    1 root     root           8 Dec  1 16:26 xlnx,family
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,include-dphase-
timer
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-awidth
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-clk-period-ps
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-dwidth
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-native-dwidth
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-p2p
-r--r--r--    1 root     root           4 Dec  1 16:26 xlnx,mplb-smallest-
slave

But first I'm not sure where to find the IRQ in there, and also I'm not sure if 
reading the filesystem from a module is allowed.

How do I know if this interrupt is shared or not (is it important ?)

Thanks
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Philipp Ittershagen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/01/2010 05:35 PM, Guillaume Dargaud wrote:
> Now how do I connect the dots between the hardware definitions from the dts and 
> my driver ?

You can get the interrupt number from the dt by calling
irq_of_parse_and_map(). Be sure to pass the node of your device to this
function.

Then you have to request the interrupt by calling request_irq. This is
where you specify the interrupt handler.

> But first I'm not sure where to find the IRQ in there, and also I'm not sure if 
> reading the filesystem from a module is allowed.

Why do you want to read the file system?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkz2lCEACgkQCG4q0RxCsY4GpgCgiQFRhiF7jjhUdZcUBc4Y5ScJ
E0AAn0VxcCaVexepjrah64ZSS+Xhbed8
=h97e
-----END PGP SIGNATURE-----

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Scott Wood]

On Wed, 1 Dec 2010 17:35:58 +0100
Guillaume Dargaud <dargaud@lpsc.in2p3.fr> wrote:

> OK, here goes then: how do I get the IRQ number so that I can install an 
> interrupt handler on it ?
> 
> In my dts file I have:
> 		xps_acqui_data_0: xps-acqui-data@c9800000 {
> 			compatible = "xlnx,xps-acqui-data-3.00.a";
> 			interrupt-parent = <&xps_intc_0>;
> 			interrupts = < 0 2 >;
> 			reg = < 0xc9800000 0x10000 >;
> 			xlnx,family = "virtex4";
> 			xlnx,include-dphase-timer = <0x1>;
> 			xlnx,mplb-awidth = <0x20>;
> 			xlnx,mplb-clk-period-ps = <0x2710>;
> 			xlnx,mplb-dwidth = <0x40>;
> 			xlnx,mplb-native-dwidth = <0x40>;
> 			xlnx,mplb-p2p = <0x0>;
> 			xlnx,mplb-smallest-slave = <0x20>;
> 		} ;
> 
> In my minimal driver init, I have:
>   first = MKDEV (my_major, my_minor);
>   register_chrdev_region(first, count, NAME);
>   cdev_init(my_cdev, &fops);
>   cdev_add (my_cdev, first, count);
> So far so good.
> 
> Now how do I connect the dots between the hardware definitions from the dts and 
> my driver ?

How was your driver probed?  If you can get a pointer to the device
node, use irq_of_parse_and_map() to get a virtual irq that you can pass
to request_irq().

> But first I'm not sure where to find the IRQ in there, and also I'm not sure if 
> reading the filesystem from a module is allowed.

There's no need; there are much easier ways to access the device tree
from within the kernel.

> How do I know if this interrupt is shared or not (is it important ?)

Can your driver tolerate it being shared?  If so, request it as shared.

-Scott

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

> How was your driver probed?  If you can get a pointer to the device
> node, use irq_of_parse_and_map() to get a virtual irq that you can pass
> to request_irq().

Sounds like what I need... But I don't find irq_of_parse_and_map() in LDD 3rd 
ed and very few info in google. Are there some examples of use somewhere ?

> Can your driver tolerate it being shared?  If so, request it as shared.
I think so (not sure).
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Timur Tabi]

On Thu, Dec 2, 2010 at 9:36 AM, Guillaume Dargaud <dargaud@lpsc.in2p3.fr> wrote:

> Sounds like what I need... But I don't find irq_of_parse_and_map() in LDD 3rd
> ed and very few info in google. Are there some examples of use somewhere ?

     grep -r irq_of_parse_and_map *

will give you plenty of examples.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

>      grep -r irq_of_parse_and_map *
> 
> will give you plenty of examples.

OK, learning by example then.

Another basic question: 
what's the difference between of_register_platform_driver() and 
platform_driver_register() ?
I assume I must use the first one if I want to use irq_of_parse_and_map...
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Benjamin Herrenschmidt]

On Thu, 2010-12-02 at 18:47 +0100, Guillaume Dargaud wrote:
> 
> OK, learning by example then.
> 
> Another basic question: 
> what's the difference between of_register_platform_driver() and 
> platform_driver_register() ?
> I assume I must use the first one if I want to use
> irq_of_parse_and_map... 

No. of_platform_drivers are more/less obsolete. Normal platform drivers
can now be associated with a device-tree node just fine.

Cheers,
Ben.

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

> No. of_platform_drivers are more/less obsolete. Normal platform drivers
> can now be associated with a device-tree node just fine.

OK.

If my dts definition is thus:
		xps_acqui_data_0: xps-acqui-data@c9800000 {
			compatible = "xlnx,xps-acqui-data-3.00.a";
			interrupt-parent = <&xps_intc_0>;
			interrupts = < 0 2 >;
			reg = < 0xc9800000 0x10000 >;
			xlnx,family = "virtex4";
			xlnx,include-dphase-timer = <0x1>;
			xlnx,mplb-awidth = <0x20>;
			xlnx,mplb-clk-period-ps = <0x2710>;
			xlnx,mplb-dwidth = <0x40>;
			xlnx,mplb-native-dwidth = <0x40>;
			xlnx,mplb-p2p = <0x0>;
			xlnx,mplb-smallest-slave = <0x20>;
		} ;

What are the names I need to pass to platform_driver_register and 
platform_device_register_simple ? xps_acqui_data_0, xps-acqui-data or xps-
acqui-data-3.00.a ? None seem to call any init functions...

Why is there not a word about the functions platform_*_register in my various 
driver books ? LDD 3rd ed (O'Reilly), Writing LDD (Cooperstein) or LKD 
(Love)... Is it something specific to powerpc and the books are oriented x86 ? 
What's a good source, besides grepping the kernel to no end ?

-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Martyn Welch]

On 03/12/10 14:58, Guillaume Dargaud wrote:
> Why is there not a word about the functions platform_*_register in my various 
> driver books ? LDD 3rd ed (O'Reilly), Writing LDD (Cooperstein) or LKD 
> (Love)... Is it something specific to powerpc and the books are oriented x86 ? 
> What's a good source, besides grepping the kernel to no end ?
> 

I have yet to find a Linux Device Driver book that isn't heavily skewed
towards x86. Some of this stuff is either specific to powerpc or
relevant a subset of non-x86 platforms.

I think Google, the linuxppc-dev mailing list, related IRC channel and
lxr.linux.no have been the sources I've mostly used learning about Linux
on powerpc.


-- 
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square,
Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Fri, 2010-12-03 at 15:58 +0100, Guillaume Dargaud wrote:
> > No. of_platform_drivers are more/less obsolete. Normal platform drivers
> > can now be associated with a device-tree node just fine.
> 
> OK.
> 
> If my dts definition is thus:
> 		xps_acqui_data_0: xps-acqui-data@c9800000 {
> 			compatible = "xlnx,xps-acqui-data-3.00.a";
> 			interrupt-parent = <&xps_intc_0>;
> 			interrupts = < 0 2 >;
> 			reg = < 0xc9800000 0x10000 >;
> 			xlnx,family = "virtex4";
> 			xlnx,include-dphase-timer = <0x1>;
> 			xlnx,mplb-awidth = <0x20>;
> 			xlnx,mplb-clk-period-ps = <0x2710>;
> 			xlnx,mplb-dwidth = <0x40>;
> 			xlnx,mplb-native-dwidth = <0x40>;
> 			xlnx,mplb-p2p = <0x0>;
> 			xlnx,mplb-smallest-slave = <0x20>;
> 		} ;
> 
> What are the names I need to pass to platform_driver_register and 
> platform_device_register_simple ? xps_acqui_data_0, xps-acqui-data or xps-
> acqui-data-3.00.a ?

None of the above :)  xps_acqui_data_0 is a label, it is only relevant
within the dts source, ie. other nodes may refer to this node by that
label. But the label string is not useful in Linux. The node name
"xps-acqui-data" is visible in Linux, but is generally not used for
discovery, it's just a name. What you do want to use is the full
compatible value, ie. "xlnx,xps-acqui-data-3.00.a".

> Why is there not a word about the functions platform_*_register in my various 
> driver books ? LDD 3rd ed (O'Reilly), Writing LDD (Cooperstein) or LKD 
> (Love)... Is it something specific to powerpc and the books are oriented x86 ? 

Because they're old dead-tree references, that aren't up to date with
the kernel. The LDD website says "LDD3 is current as of the 2.6.10
kernel", that's 25 kernel versions ago! I'm also not aware of any book
that has information on anything other than x86 drivers.

> What's a good source, besides grepping the kernel to no end ?

Nothing really I'm afraid.


So you have a device, it appears in your device tree, and you want to
write a driver for it.

The first issue is that someone needs to create a device for your node,
it doesn't necessarily happen by default. For many platforms this will
all happen "automagically" as long as the device node that describes
your device appears in the right place, but I'll describe it in full
anyway.

In most cases your device will appear below a node that represents a
bus, eg:

	foobus@xyz {
		compatible = "vendor,foobus-v1.m.n", "simple-bus";
		...
		yournode@abc {
			...
		}
	}

If that isn't the case you need to arrange for it somehow [1]. 

Somewhere there needs to be code to probe that bus and create devices on
it. That is usually done in platform code with of_platform_bus_probe(). 

If you don't know what platform you're on, you can look at a boot log
for a line saying "Using <blah> machine description", it will be very
early in the log. "blah" is then the name of the platform you're on, and
you should be able to grep for it in arch/powerpc/platforms.

Once you've found the .c file for your platform, there should be a call
somewhere to of_platform_bus_probe(), with a list of ids, and one of
those ids should match the compatible property of your bus node. In a
lot of cases that is just "simple-bus".

To check a device is being created for your device node, you can look
in /sys/devices. The device names don't match 100% with what's in the
device tree, but the name should be there, so in your case:

# find /sys/devices -name '*xps-acqui-data*'


Assuming that is working you can start on the driver side of things. A
rough template is:

static int foo_driver_probe(struct platform_device *device,
			    const struct of_device_id *device_id)
{
	struct device_node *dn = device->dev.of_node;
	struct foo_device *foo;

	pr_devel("%s: probing %s\n", __func__, dn->full_name);
	
	/* Check you really want to probe this device and everything is OK */

	foo = kzalloc(sizeof(*foo));
	if (!foo)
		return -ENOMEM;

	foo->regs = of_iomap(dn, 0);
	if (!foo->regs)
		goto out;

	foo->irq = irq_of_parse_and_map(dn, 0);
	if (foo->irq == NO_IRQ)
		goto out;

	etc.

	return 0;
}

static const struct of_device_id foo_device_id[] =
{                       
        { .compatible     = "vendor,foo-device" },
        {}
};

static struct platform_driver foo_driver = {
        .probe          = foo_driver_probe,
        .driver = {
                .name = "foo-driver",
                .owner = THIS_MODULE,
                .of_match_table = foo_device_id,
        },          
};

static int __init foo_driver_init(void)
{
        return platform_driver_register(&foo_driver);
}
module_init(foo_driver_init);


I probably missed something there, I haven't compiled it (obviously),
but it's a start. Some of this stuff is in flux, but I think everything
I have above is OK at least for now. Grant might correct me :)

cheers


[1]: One option is to put your device under an existing bus, but only if
that (fairly) accurately represents the hardware topology. You might
need to create a new node representing the bus your device appears on.
You can also manually create the device in C, but that is generally not
the right approach unless the device tree is immutable - ie. comes from
firmware that you don't control.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Jeremy Kerr]

Hi Michael,

> So you have a device, it appears in your device tree, and you want to
> write a driver for it.

Nice write up, mind if I steal this for the devicetree.org wiki?

Cheers,


Jeremy

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Mon, 2010-12-06 at 14:35 +0800, Jeremy Kerr wrote:
> Hi Michael,
> 
> > So you have a device, it appears in your device tree, and you want to
> > write a driver for it.
> 
> Nice write up, mind if I steal this for the devicetree.org wiki?

Sure thing, feel free to correct any mistakes too ;D

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

Thank you for the detailed answer. I'm trying to digest it...

> > What's a good source, besides grepping the kernel to no end ?
> 
> Nothing really I'm afraid.
8-|

> So you have a device, it appears in your device tree, and you want to
> write a driver for it.
That's the idea. Communication between usermode and the driver is limited to 
simple ioctl calls and the driver receives an interrupt when certain data has 
been placed in memory blocks by the hardware (like a DMA). Then the user prog 
figures out where that latest data buffer is (mmap) and saves it. 

> The first issue is that someone needs to create a device for your node,
> it doesn't necessarily happen by default. For many platforms this will
> all happen "automagically" as long as the device node that describes
> your device appears in the right place, but I'll describe it in full
> anyway.

I'm on a minimalist embedded system (buildroot+busybox+uclibc), so I'm pretty 
sure I have to do the whole thing. No udev/HAL here.

> In most cases your device will appear below a node that represents a
> bus, eg:
> 
> 	foobus@xyz {
> 		compatible = "vendor,foobus-v1.m.n", "simple-bus";
> 		...
> 		yournode@abc {
> 			...
> 		}
> 	}
> 
> If that isn't the case you need to arrange for it somehow [1].

Commenting on your [1] here. So should I just add "simple-bus" or does the 
VHDL needs to be modified so as to generate this additional bus info ?
I indeed have:

/dts-v1/;
/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "xlnx,virtex405", "xlnx,virtex";
	model = "testing";
	...
	plb: plb@0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "xlnx,plb-v46-1.05.a", "xlnx,plb-v46-1.00.a", "simple-
bus";
		ranges ;
		...
		xps_acqui_data_0: xps-acqui-data@c9800000 {
			compatible = "xlnx,xps-acqui-data-3.00.a";
			...
		}
		...
	}
	...
}

> Somewhere there needs to be code to probe that bus and create devices on
> it. That is usually done in platform code with of_platform_bus_probe().

Isn't the of_* code outdated (just been told that on a previous message) ?
Or was it just for of_register_platform_driver ?

> If you don't know what platform you're on, you can look at a boot log
> for a line saying "Using <blah> machine description", it will be very
> early in the log. "blah" is then the name of the platform you're on, and
> you should be able to grep for it in arch/powerpc/platforms.

"Using Xilinx Virtex machine description"
 
> Once you've found the .c file for your platform, there should be a call
> somewhere to of_platform_bus_probe(), with a list of ids, and one of
> those ids should match the compatible property of your bus node. In a
> lot of cases that is just "simple-bus".

That'd be:
arch/powerpc/platforms/40x/virtex.c:51: .name = "Xilinx Virtex",
and
arch/powerpc/platforms/40x/virtex.c:21: { .compatible = "simple-bus", },

No match for simple-bus of acqui-data in dmesg.

> To check a device is being created for your device node, you can look
> in /sys/devices. The device names don't match 100% with what's in the
> device tree, but the name should be there, so in your case:
> 
> # find /sys/devices -name '*xps-acqui-data*'

Indeed: 
# ll /sys/devices/plb.0/c9800000.xps-acqui-data
-r--r--r--    1 root     root        4.0K Dec  6 09:47 devspec
-r--r--r--    1 root     root        4.0K Dec  6 09:47 modalias
-r--r--r--    1 root     root        4.0K Dec  6 09:47 name
lrwxrwxrwx    1 root     root           0 Dec  6 09:47 subsystem -> 
../../../bus/of_platform/
-rw-r--r--    1 root     root        4.0K Dec  6 09:47 uevent

So that's created on boot, right ?

> [...code snipped...]

I still need a platform_device_register() after your sample init, right ?
I'll get on to adapting your sample code now. Thanks a lot.
-- 
Guillaume Dargaud
http://www.gdargaud.net/Antarctica/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Mon, 2010-12-06 at 10:58 +0100, Guillaume Dargaud wrote:
> Thank you for the detailed answer. I'm trying to digest it...
> 
> > > What's a good source, besides grepping the kernel to no end ?
> > 
> > Nothing really I'm afraid.
> 8-|
> 
> > So you have a device, it appears in your device tree, and you want to
> > write a driver for it.

> That's the idea. Communication between usermode and the driver is limited to 
> simple ioctl calls and the driver receives an interrupt when certain data has 
> been placed in memory blocks by the hardware (like a DMA). Then the user prog 
> figures out where that latest data buffer is (mmap) and saves it. 

OK, that should be all pretty straight forward, and covered by the
material in LDD and similar references. You just need to get your device
probed.

> > The first issue is that someone needs to create a device for your node,
> > it doesn't necessarily happen by default. For many platforms this will
> > all happen "automagically" as long as the device node that describes
> > your device appears in the right place, but I'll describe it in full
> > anyway.
> 
> I'm on a minimalist embedded system (buildroot+busybox+uclibc), so I'm pretty 
> sure I have to do the whole thing. No udev/HAL here.

That's another problem. The device I'm talking about here is just the
minimal struct in the kernel that you need in order for your driver to
probe against it. Once your driver has matched against a device then you
(might) need to worry about exposing a device to userspace in /dev.

> > In most cases your device will appear below a node that represents a
> > bus, eg:
> > 
> > 	foobus@xyz {
> > 		compatible = "vendor,foobus-v1.m.n", "simple-bus";
> > 		...
> > 		yournode@abc {
> > 			...
> > 		}
> > 	}
> > 
> > If that isn't the case you need to arrange for it somehow [1].
> 
> Commenting on your [1] here. So should I just add "simple-bus" or does the 
> VHDL needs to be modified so as to generate this additional bus info ?

Doesn't look like it:

> I indeed have:
> 
> /dts-v1/;
> / {
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 	compatible = "xlnx,virtex405", "xlnx,virtex";
> 	model = "testing";
> 	...
> 	plb: plb@0 {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		compatible = "xlnx,plb-v46-1.05.a", "xlnx,plb-v46-1.00.a", "simple-bus";
> 		ranges ;
> 		...
> 		xps_acqui_data_0: xps-acqui-data@c9800000 {
> 			compatible = "xlnx,xps-acqui-data-3.00.a";
> 			...
> 		}
> 		...
> 	}
> 	...
> }

That looks good.

> > Somewhere there needs to be code to probe that bus and create devices on
> > it. That is usually done in platform code with of_platform_bus_probe().
> 
> Isn't the of_* code outdated (just been told that on a previous message) ?
> Or was it just for of_register_platform_driver ?

No it's just that platform_drivers are now able to do all the things an
of_platform_driver can do, so new code should just use platform_driver.

I'm not sure if of_platform_bus_probe() will go away eventually, but for
now it is the only way to achieve what you need.

> > If you don't know what platform you're on, you can look at a boot log
> > for a line saying "Using <blah> machine description", it will be very
> > early in the log. "blah" is then the name of the platform you're on, and
> > you should be able to grep for it in arch/powerpc/platforms.
> 
> "Using Xilinx Virtex machine description"

Cool.

> > Once you've found the .c file for your platform, there should be a call
> > somewhere to of_platform_bus_probe(), with a list of ids, and one of
> > those ids should match the compatible property of your bus node. In a
> > lot of cases that is just "simple-bus".
> 
> That'd be:
> arch/powerpc/platforms/40x/virtex.c:51: .name = "Xilinx Virtex",
> and
> arch/powerpc/platforms/40x/virtex.c:21: { .compatible = "simple-bus", },

Perfect.

> No match for simple-bus of acqui-data in dmesg.

That's normal, if the kernel printed out every device it found you'd
drown in messages. If you want to you can put a printk in
of_platform_device_create() and you should see it.

> > To check a device is being created for your device node, you can look
> > in /sys/devices. The device names don't match 100% with what's in the
> > device tree, but the name should be there, so in your case:
> > 
> > # find /sys/devices -name '*xps-acqui-data*'
> 
> Indeed: 
> # ll /sys/devices/plb.0/c9800000.xps-acqui-data
> -r--r--r--    1 root     root        4.0K Dec  6 09:47 devspec
> -r--r--r--    1 root     root        4.0K Dec  6 09:47 modalias
> -r--r--r--    1 root     root        4.0K Dec  6 09:47 name
> lrwxrwxrwx    1 root     root           0 Dec  6 09:47 subsystem -> ../../../bus/of_platform/
> -rw-r--r--    1 root     root        4.0K Dec  6 09:47 uevent
> 
> So that's created on boot, right ?

Yep, so that is all working fine. You just need to write a driver that
will match against that device.

> > [...code snipped...]
> 
> I still need a platform_device_register() after your sample init, right ?

I had one in there already, in foo_driver_init(), didn't I?

> I'll get on to adapting your sample code now. Thanks a lot.

No worries.

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

Hello all,

> OK, that should be all pretty straight forward, and covered by the
> material in LDD and similar references. You just need to get your device
> probed.

I'm not sure what you mean with that term: simply identifying that the device 
works using device specific code ?
I've looked at several *_probe functions from other drivers and they are all 
completely different with no common code...

Or do you mean calling of_platform_bus_probe() ? What does that function do ?

> No it's just that platform_drivers are now able to do all the things an
> of_platform_driver can do, so new code should just use platform_driver.
> 
> I'm not sure if of_platform_bus_probe() will go away eventually, but for
> now it is the only way to achieve what you need.

I assume the kernel needs to be compiled with CONFIG_OF.
...hmm I had to "git pull" in order for this to compile your snippet. That's 
really recent! Unfortunately i need to reflash my device with the new kernel 
before i can begin testing my module.

When I try to compile your (adapted) snippet, I got this minor problems:

  .probe = foo_driver_probe,
^ warning: initialization from incompatible pointer type
Shouldn't foo_driver_probe be:
static int foo_driver_probe(struct platform_device *pdev)
instead of:
static int foo_driver_probe(struct platform_device *device,
                            const struct of_device_id *device_id)


Also:
       struct foo_device *foo;
That's where I put my own content, right ?
And needs to be kfreed in a foo_driver_remove() function, right ?

> > I still need a platform_device_register() after your sample init, right ?
> 
> I had one in there already, in foo_driver_init(), didn't I?

You had platform_driver_register(), not that I'm all clear yet on the 
distinction...


Another question: I just spent 10 minutes trying to find where "struct device" 
was defined. (ack-)grep was absolutely no use. Isn't there a way to cross-
reference my own kernel, the way I've compiled it ?
-- 
Guillaume Dargaud
http://www.gdargaud.net/Climbing/

RE: Getting the IRQ number (Was: Basic driver devel questions ?)

[David Laight]

=20
> Another question: I just spent 10 minutes trying to find
> where "struct device" was defined.

Dirty trick #4:

At the top of a source file (before the first include) add:
    struct device { int fubar; };
Then try to compile it.

The compiler will the tell where it is defined!

	David

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Mon, 2010-12-06 at 15:44 +0100, Guillaume Dargaud wrote: 
> Hello all,
> 
> > OK, that should be all pretty straight forward, and covered by the
> > material in LDD and similar references. You just need to get your device
> > probed.
> 
> I'm not sure what you mean with that term: simply identifying that the device 
> works using device specific code ?

I just mean connecting your driver up to a struct device that represents
your device.

> I've looked at several *_probe functions from other drivers and they are all 
> completely different with no common code...

Yes that's pretty normal, the probe routine usually finds device
specific info about the device and stores it in some sort of structure
for later use.

> Or do you mean calling of_platform_bus_probe() ? What does that function do ?

The platform code does that. That function create devices on the
platform bus by examining the device tree. It looks for nodes that are
compatible with the compatible strings you give it, and treats them as
busses, ie. creates devices for all child nodes of those nodes.

> > No it's just that platform_drivers are now able to do all the things an
> > of_platform_driver can do, so new code should just use platform_driver.
> > 
> > I'm not sure if of_platform_bus_probe() will go away eventually, but for
> > now it is the only way to achieve what you need.
> 
> I assume the kernel needs to be compiled with CONFIG_OF.

Yes absolutely.

> ...hmm I had to "git pull" in order for this to compile your snippet. That's 
> really recent! Unfortunately i need to reflash my device with the new kernel 
> before i can begin testing my module.

It shouldn't be that recent, what kernel version were you using?

> When I try to compile your (adapted) snippet, I got this minor problems:
> 
>   .probe = foo_driver_probe,
> ^ warning: initialization from incompatible pointer type
> Shouldn't foo_driver_probe be:
> static int foo_driver_probe(struct platform_device *pdev)
> instead of:
> static int foo_driver_probe(struct platform_device *device,
>                             const struct of_device_id *device_id)

Yes sorry, that's a cut & paste bug, between the old and new style code.

> Also:
>        struct foo_device *foo;
> That's where I put my own content, right ?

Yep.

> And needs to be kfreed in a foo_driver_remove() function, right ?

If you have a remove then yes.

> > > I still need a platform_device_register() after your sample init, right ?
> > 
> > I had one in there already, in foo_driver_init(), didn't I?
> 
> You had platform_driver_register(), not that I'm all clear yet on the 
> distinction...

Oh sorry, I always read those wrong.

platform_device_register() creates a device on the platform bus. You
then write a driver for that device, and register it with
platform_driver_register(), your driver then attaches to the device.

In this case though you don't need to call platform_device_register()
because the device has already been created, using the device tree (by
of_platform_bus_probe()).

In general on powerpc we don't use platform_device_register() much,
instead things are described in the device tree and devices are created
for them.

platform_device_register() tends to be for cases where you can't
discover or probe a device, but you "know" that it exists.

> Another question: I just spent 10 minutes trying to find where "struct device" 
> was defined. (ack-)grep was absolutely no use. Isn't there a way to cross-
> reference my own kernel, the way I've compiled it ?

Yes, "make tags", then use vim :)

If you're cross-compiling make sure to set ARCH=powerpc when you make
tags.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

Thanks again for your detailed answer,
I'm learning, but not as fast as my colleagues with a deadline want me to !

> The platform code does that. That function create devices on the
> platform bus by examining the device tree. It looks for nodes that are
> compatible with the compatible strings you give it, and treats them as
> busses, ie. creates devices for all child nodes of those nodes.

Is there a way to enable some debug output from it, to see what's going on ?

> > ...hmm I had to "git pull" in order for this to compile your snippet.
> > That's really recent! Unfortunately i need to reflash my device with the
> > new kernel before i can begin testing my module.
> 
> It shouldn't be that recent, what kernel version were you using?

I had to go from 2.6.34 to 2.6.35, xilinx git tree.

> Yes sorry, that's a cut & paste bug, between the old and new style code.

No worries, I knew it was some uncompiled example.

> platform_device_register() creates a device on the platform bus. You
> then write a driver for that device, and register it with
> platform_driver_register(), your driver then attaches to the device.
> 
> In this case though you don't need to call platform_device_register()
> because the device has already been created, using the device tree (by
> of_platform_bus_probe()).

OK, I'll have to remove it from my sample code below.

> In general on powerpc we don't use platform_device_register() much,
> instead things are described in the device tree and devices are created
> for them.
> 
> platform_device_register() tends to be for cases where you can't
> discover or probe a device, but you "know" that it exists.

When you see something in /sys/devices/plb.0/, it means that you don't need 
platform_device_register, right ?

> Yes, "make tags", then use vim :)

Great, that works.


OK, here's the relevant part of my code, ripped directly from your sample, 
with a few additions and different variable names. Why do you think 
xad_driver_probe() is not being called ?


#include <linux/kernel.h>
#include <linux/init.h>
#include <asm/device.h>
#include <linux/platform_device.h>
#include <linux/cdev.h>         // char device
#include <linux/fs.h>
#include <linux/of.h>
#include <linux/interrupt.h>

#define DEVNAME "xps-acqui-data"
#define NAME "xad"              // This is only used for printk

#define SD "{%s %d} "
#define FL , __func__, __LINE__

static dev_t first;
static unsigned int count = 1;
static int my_major = 241, my_minor = 0;
// You must run "mknod /dev/xad c 241 0" in a shell at least once

struct cdev *my_cdev=NULL;
struct platform_device *pdev=NULL;

typedef struct XadDevice {
  struct resource *hw_region;
  struct device *dev;
  int irq;
} tXadDevice;
tXadDevice Xad;

// There should be something in:
// ll /sys/devices/plb.0/c9800000.xps-acqui-data
static const struct of_device_id xad_device_id[] = {                       
        { .compatible     = "xlnx,xps-acqui-data-3.00.a" },     // Must match 
the DTS
        {}
};

  
static irqreturn_t XadIsr(int irq, void *dev_id) {
  printk(KERN_INFO SD "IRQ:%d\n" FL, irq);
  return IRQ_HANDLED;
}

///////////////////////////////////////////////////////////////////////////////
// Platform Bus Support
///////////////////////////////////////////////////////////////////////////////

static int  xad_driver_probe(struct platform_device *device /*,
                            const struct of_device_id *device_id*/ ) {
  struct device_node *dn = device->dev.of_node;
  int rc;

  pr_devel("Probing %s\n", dn->full_name);
  
  Xad.irq = irq_of_parse_and_map(dn, 0);
  rc=request_irq(Xad.irq, XadIsr, IRQF_TRIGGER_RISING  | IRQF_DISABLED, 
"XadIsr", &Xad);
  if (rc) printk(KERN_INFO SD "Failled IRQ request: %d\n" FL, rc);
  
  return 0;
}

static int __devexit xad_driver_remove(struct platform_device *device) {
  printk(KERN_INFO SD "Removing...\n" FL);
  return 0;
}

static struct platform_driver xad_driver = {
  .probe  = xad_driver_probe,
  .remove = xad_driver_remove,
  .driver = {
    .owner = THIS_MODULE,
    .name = "xad-driver",
        .of_match_table = xad_device_id,
  },
};


///////////////////////////////////////////////////////////////////////////////
// This section deals with the /dev/xad device
///////////////////////////////////////////////////////////////////////////////
static int xad_open(struct inode *node, struct file *filep) {
  printk (KERN_INFO SD "OPENING device: %s\n" FL, NAME);
  return 0;
}

static int xad_release(struct inode *node, struct file *filep) {
  printk (KERN_INFO SD "RELEASING device: %s\n" FL, NAME);
  return 0;
}

static int  xad_ioctl(struct inode *node, struct file *filep, unsigned int cmd, 
unsigned long arg) {
  printk (KERN_INFO SD "IOCTL on device: %s, cmd:%d, arg:%lu\n" FL, NAME, cmd, 
arg);
  return 0;
}

static struct file_operations fops = {
  .owner   = THIS_MODULE,
  .open    = xad_open,
  .release = xad_release,
  .ioctl   = xad_ioctl,
};


///////////////////////////////////////////////////////////////////////////////
// Called on insmod
static int __init xad_init(void) {
  int rc=0;
  printk(KERN_INFO SD "Module %s: loading...\n" FL, NAME);
  
  // Deal with the device
  first = MKDEV (my_major, my_minor);
  register_chrdev_region(first, count, DEVNAME);
  my_cdev = cdev_alloc ();
  if (NULL==my_cdev) goto Err;
  
  cdev_init(my_cdev, &fops);
  cdev_add (my_cdev, first, count);

  printk(KERN_INFO SD "Module %s: Major=%d, Minor=%d, Count=%d\n" FL, NAME, 
my_major, my_minor, count);

  // Driver
  rc = platform_driver_register(&xad_driver);
//  rc = platform_driver_probe(&xad_driver, xad_driver_probe);
  if (rc) goto err_plat;

  // Device
  pdev=platform_device_register_simple("xps-acqui-data", -1, NULL, 0);
  if (IS_ERR(pdev)) {
          rc = PTR_ERR(pdev);
          platform_driver_unregister(&xad_driver);
          goto err_plat;
  }


  return 0;

err_plat:
  unregister_chrdev_region(first, count);
Err:
  printk(KERN_ERR SD "Module %s: Failed loading rc=%d\n" FL, NAME, rc);
  return rc;
}

///////////////////////////////////////////////////////////////////////////////
// Called on rmmod
static void xad_exit(void) {
  platform_device_unregister(pdev); pdev=NULL;
  platform_driver_unregister(&xad_driver);
  
  cdev_del (my_cdev); my_cdev=NULL;
  unregister_chrdev_region (first, count);
  printk(KERN_INFO SD "Module %s unloaded\n" FL, NAME);
}

module_init(xad_init);
module_exit(xad_exit);

MODULE_AUTHOR("Guillaume Dargaud");
MODULE_LICENSE("GPL");





And here's the result:

# tail -f /var/log/messages &
# insmod xad.ko
Dec  7 14:42:02 gandalf user.info kernel: [15897.152109] {xad_init 162} Module 
xad: loading...
Dec  7 14:42:02 gandalf user.info kernel: [15897.156665] {xad_init 173} Module 
xad: Major=241, Minor=0, Count=1

# ./xad_test	# A simple prog that opens, does an iocl and closes the device
./xad_test Initialisation
./xad_test Ioctl
./xad_test Closing

Dec  7 14:42:16 gandalf user.info kernel: [15911.160343] {xad_open 130} 
OPENING device: xad
Dec  7 14:42:16 gandalf user.info kernel: [15911.164521] {xad_ioctl 142} IOCTL 
on device: xad, cmd:0, arg:0
Dec  7 14:42:16 gandalf user.info kernel: [15911.173035] {xad_release 136} 
RELEASING device: xad

# rmmod xad
Dec  7 14:42:23 gandalf user.info kernel: [15918.753661] {xad_exit 206} Module 
xad unloaded


And in the dts:

/dts-v1/;
/ {
	...
	plb: plb@0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "xlnx,plb-v46-1.05.a", "xlnx,plb-v46-1.00.a", "simple-
bus";
		ranges ;
		...
		xps_acqui_data_0: xps-acqui-data@c9800000 {
			compatible = "xlnx,xps-acqui-data-3.00.a";
			interrupt-parent = <&xps_intc_0>;
			interrupts = < 0 2 >;
			reg = < 0xc9800000 0x10000 >;
			xlnx,family = "virtex4";
			xlnx,include-dphase-timer = <0x1>;
			xlnx,mplb-awidth = <0x20>;
			xlnx,mplb-clk-period-ps = <0x2710>;
			xlnx,mplb-dwidth = <0x40>;
			xlnx,mplb-native-dwidth = <0x40>;
			xlnx,mplb-p2p = <0x0>;
			xlnx,mplb-smallest-slave = <0x20>;
		} ;
		...
	} ;
	...
}  ;


# ll /sys/devices/plb.0/c9800000.xps-acqui-data/
-r--r--r--    1 root     root        4.0K Dec  7 12:55 devspec
-r--r--r--    1 root     root        4.0K Dec  7 12:55 modalias
-r--r--r--    1 root     root        4.0K Dec  7 12:55 name
lrwxrwxrwx    1 root     root           0 Dec  7 12:55 subsystem -> 
../../../bus/of_platform/
-rw-r--r--    1 root     root        4.0K Dec  7 12:55 uevent

-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Wed, 2010-12-08 at 11:18 +0100, Guillaume Dargaud wrote:
> Thanks again for your detailed answer,
> I'm learning, but not as fast as my colleagues with a deadline want me to !

:)

> > The platform code does that. That function create devices on the
> > platform bus by examining the device tree. It looks for nodes that are
> > compatible with the compatible strings you give it, and treats them as
> > busses, ie. creates devices for all child nodes of those nodes.
> 
> Is there a way to enable some debug output from it, to see what's going on ?

Not really, but there probably should be.

> > > ...hmm I had to "git pull" in order for this to compile your snippet.
> > > That's really recent! Unfortunately i need to reflash my device with the
> > > new kernel before i can begin testing my module.
> > 
> > It shouldn't be that recent, what kernel version were you using?
> 
> I had to go from 2.6.34 to 2.6.35, xilinx git tree.

Ah that is the problem I think.

Sorry I assumed you were working on mainline. In 2.6.35 you still need
to use an of_platform_driver, I'll describe below.

> > platform_device_register() tends to be for cases where you can't
> > discover or probe a device, but you "know" that it exists.
> 
> When you see something in /sys/devices/plb.0/, it means that you don't need 
> platform_device_register, right ?

That's right.

> > Yes, "make tags", then use vim :)
> 
> Great, that works.

Cool.

> OK, here's the relevant part of my code, ripped directly from your sample, 
> with a few additions and different variable names. Why do you think 
> xad_driver_probe() is not being called ?

As I said above in 2.6.35 platform drivers and of_platform drivers were
still separate. So your device is on the of_platform_bus (ie. was
discovered using the device tree), but your driver is on the
platform_bus. Yes this is very confusing.

So basically you need to change all occurrences of platform_driver to
of_platform_driver.

> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <asm/device.h>
> #include <linux/platform_device.h>
> #include <linux/cdev.h>         // char device
> #include <linux/fs.h>
> #include <linux/of.h>
> #include <linux/interrupt.h>
> 
> #define DEVNAME "xps-acqui-data"
> #define NAME "xad"              // This is only used for printk
> 
> #define SD "{%s %d} "
> #define FL , __func__, __LINE__
> 
> static dev_t first;
> static unsigned int count = 1;
> static int my_major = 241, my_minor = 0;
> // You must run "mknod /dev/xad c 241 0" in a shell at least once
> 
> struct cdev *my_cdev=NULL;
> struct platform_device *pdev=NULL;
> 
> typedef struct XadDevice {
>   struct resource *hw_region;
>   struct device *dev;
>   int irq;
> } tXadDevice;
> tXadDevice Xad;
> 
> // There should be something in:
> // ll /sys/devices/plb.0/c9800000.xps-acqui-data
> static const struct of_device_id xad_device_id[] = {                       
>         { .compatible     = "xlnx,xps-acqui-data-3.00.a" },     // Must match 
> the DTS
>         {}
> };
> 
>   
> static irqreturn_t XadIsr(int irq, void *dev_id) {
>   printk(KERN_INFO SD "IRQ:%d\n" FL, irq);
>   return IRQ_HANDLED;
> }
> 
> ///////////////////////////////////////////////////////////////////////////////
> // Platform Bus Support
> ///////////////////////////////////////////////////////////////////////////////
> 
> static int  xad_driver_probe(struct platform_device *device /*,
>                             const struct of_device_id *device_id*/ ) {

So you need to switch the prototype here to:

static int xad_driver_probe(struct of_platform_device *ofdev,
                            const struct of_device_id *device_id) {

>   struct device_node *dn = device->dev.of_node;
>   int rc;
> 
>   pr_devel("Probing %s\n", dn->full_name);
>   
>   Xad.irq = irq_of_parse_and_map(dn, 0);
>   rc=request_irq(Xad.irq, XadIsr, IRQF_TRIGGER_RISING  | IRQF_DISABLED, 
> "XadIsr", &Xad);
>   if (rc) printk(KERN_INFO SD "Failled IRQ request: %d\n" FL, rc);
>   
>   return 0;
> }
> 
> static int __devexit xad_driver_remove(struct platform_device *device) {
>   printk(KERN_INFO SD "Removing...\n" FL);
>   return 0;
> }
> 
> static struct platform_driver xad_driver = {

Becomes of_platform_driver

>   .probe  = xad_driver_probe,
>   .remove = xad_driver_remove,
>   .driver = {
>     .owner = THIS_MODULE,
>     .name = "xad-driver",
>         .of_match_table = xad_device_id,
>   },
> };
> 
> 
> ///////////////////////////////////////////////////////////////////////////////
> // This section deals with the /dev/xad device
> ///////////////////////////////////////////////////////////////////////////////
> static int xad_open(struct inode *node, struct file *filep) {
>   printk (KERN_INFO SD "OPENING device: %s\n" FL, NAME);
>   return 0;
> }
> 
> static int xad_release(struct inode *node, struct file *filep) {
>   printk (KERN_INFO SD "RELEASING device: %s\n" FL, NAME);
>   return 0;
> }
> 
> static int  xad_ioctl(struct inode *node, struct file *filep, unsigned int cmd, 
> unsigned long arg) {
>   printk (KERN_INFO SD "IOCTL on device: %s, cmd:%d, arg:%lu\n" FL, NAME, cmd, 
> arg);
>   return 0;
> }
> 
> static struct file_operations fops = {
>   .owner   = THIS_MODULE,
>   .open    = xad_open,
>   .release = xad_release,
>   .ioctl   = xad_ioctl,
> };
> 
> 
> ///////////////////////////////////////////////////////////////////////////////
> // Called on insmod
> static int __init xad_init(void) {
>   int rc=0;
>   printk(KERN_INFO SD "Module %s: loading...\n" FL, NAME);
>   
>   // Deal with the device
>   first = MKDEV (my_major, my_minor);
>   register_chrdev_region(first, count, DEVNAME);
>   my_cdev = cdev_alloc ();
>   if (NULL==my_cdev) goto Err;
>   
>   cdev_init(my_cdev, &fops);
>   cdev_add (my_cdev, first, count);
> 
>   printk(KERN_INFO SD "Module %s: Major=%d, Minor=%d, Count=%d\n" FL, NAME, 
> my_major, my_minor, count);
> 
>   // Driver
>   rc = platform_driver_register(&xad_driver);

Should be of_register_platform_driver()

> //  rc = platform_driver_probe(&xad_driver, xad_driver_probe);
>   if (rc) goto err_plat;
> 
>   // Device
>   pdev=platform_device_register_simple("xps-acqui-data", -1, NULL, 0);
>   if (IS_ERR(pdev)) {
>           rc = PTR_ERR(pdev);
>           platform_driver_unregister(&xad_driver);
>           goto err_plat;
>   }
> 
> 
>   return 0;
> 
> err_plat:
>   unregister_chrdev_region(first, count);
> Err:
>   printk(KERN_ERR SD "Module %s: Failed loading rc=%d\n" FL, NAME, rc);
>   return rc;
> }
> 
> ///////////////////////////////////////////////////////////////////////////////
> // Called on rmmod
> static void xad_exit(void) {
>   platform_device_unregister(pdev); pdev=NULL;
>   platform_driver_unregister(&xad_driver);
>   
>   cdev_del (my_cdev); my_cdev=NULL;
>   unregister_chrdev_region (first, count);
>   printk(KERN_INFO SD "Module %s unloaded\n" FL, NAME);
> }
> 
> module_init(xad_init);
> module_exit(xad_exit);
> 
> MODULE_AUTHOR("Guillaume Dargaud");
> MODULE_LICENSE("GPL");
> 
> 

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

> Sorry I assumed you were working on mainline. In 2.6.35 you still need
> to use an of_platform_driver, I'll describe below.

> So basically you need to change all occurrences of platform_driver to
> of_platform_driver.

OK, thanks, I did that, compiled and ran the driver and its test prog... no 
changes: xad_driver_probe() doesn't get called.
...?

-- 
Guillaume Dargaud
http://www.gdargaud.net/Antarctica/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Wed, 2010-12-08 at 16:52 +0100, Guillaume Dargaud wrote:
> > Sorry I assumed you were working on mainline. In 2.6.35 you still need
> > to use an of_platform_driver, I'll describe below.
> 
> > So basically you need to change all occurrences of platform_driver to
> > of_platform_driver.
> 
> OK, thanks, I did that, compiled and ran the driver and its test prog... no 
> changes: xad_driver_probe() doesn't get called.
> ...?

Er. Not sure sorry. I can't see anything obviously wrong. Maybe post
your driver code again.

Also turn on CONFIG_DEBUG_DRIVER and see if that gives you anything
interesting.

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

Hello all,

> Are you sure that you want to have the chrdev registration here (the
> following code)?

It was commented out in my lastest attempts after reading Michael's 
explainations.

> Such stuff typically goes into the probe() function. The modules's
> init() just registers the driver. Furthermore your global variables
> prohibit having more than one device instance using the driver.

Only one such device will ever be build, so don't expect a mainline kernel 
patch from me anytime soon !   C;-)

> Also turn on CONFIG_DEBUG_DRIVER and see if that gives you anything
> interesting.

I see an unset CONFIG_DEBUG_KERNEL but no CONFIG_DEBUG_DRIVER in the .config of 
my current kernel. 
Does it need to be changed in the .config with a full kernel recompilation, or 
can I still benefit from it in my module only by doing a #define at the begining 
of my code ? Some #defines like DEBUG seem to work locally on things like 
dev_dbg but I doubt the CONFIG_* work the same...

> Er. Not sure sorry. I can't see anything obviously wrong. Maybe post
> your driver code again.

Err... I ran it again this morning and it worked farther. I now get into the 
probe function and can now register my interrupt, yeah!, but I don't see the 
ISR being called. I'm currently checking if it can be a hardware problem 
before coming back here for more questions !

BTW, is errno/strerror used within the kernel ?

Thanks all.
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Fri, 2010-12-10 at 17:21 +0100, Guillaume Dargaud wrote:
> Hello all,

> 
> > Also turn on CONFIG_DEBUG_DRIVER and see if that gives you anything
> > interesting.
> 
> I see an unset CONFIG_DEBUG_KERNEL but no CONFIG_DEBUG_DRIVER in the .config of 
> my current kernel. 

Right you'd need to turn on CONFIG_DEBUG_KERNEL first, and then
CONFIG_DEBUG_DRIVER will appear.

> Does it need to be changed in the .config with a full kernel recompilation, or 
> can I still benefit from it in my module only by doing a #define at the begining 
> of my code ? Some #defines like DEBUG seem to work locally on things like 
> dev_dbg but I doubt the CONFIG_* work the same...

No you'd need to rebuild your kernel. DEBUG is the only #define I can
think of that has an effect locally. Using different values for a
CONFIG_ in the kernel vs modules will not work.

> > Er. Not sure sorry. I can't see anything obviously wrong. Maybe post
> > your driver code again.
> 
> Err... I ran it again this morning and it worked farther. I now get into the 
> probe function and can now register my interrupt, yeah!, but I don't see the 
> ISR being called. I'm currently checking if it can be a hardware problem 
> before coming back here for more questions !

Great!

> BTW, is errno/strerror used within the kernel ?

Nope.

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Joachim Förster]

Hi Guillaume,

Michael Ellerman wrote:
> On Wed, 2010-12-08 at 11:18 +0100, Guillaume Dargaud wrote:
>> ///////////////////////////////////////////////////////////////////////////////
>> // Called on insmod
>> static int __init xad_init(void) {
>>   int rc=0;
>>   printk(KERN_INFO SD "Module %s: loading...\n" FL, NAME);
>>   

Are you sure that you want to have the chrdev registration here (the
following code)?

Such stuff typically goes into the probe() function. The modules's
init() just registers the driver. Furthermore your global variables
prohibit having more than one device instance using the driver.

>>   // Deal with the device
>>   first = MKDEV (my_major, my_minor);
>>   register_chrdev_region(first, count, DEVNAME);
>>   my_cdev = cdev_alloc ();
>>   if (NULL==my_cdev) goto Err;
>>   
>>   cdev_init(my_cdev, &fops);
>>   cdev_add (my_cdev, first, count);
>>
>>   printk(KERN_INFO SD "Module %s: Major=%d, Minor=%d, Count=%d\n" FL, NAME, 
>> my_major, my_minor, count);
>>
>>   // Driver
>>   rc = platform_driver_register(&xad_driver);
> 
> Should be of_register_platform_driver()
> 
>> //  rc = platform_driver_probe(&xad_driver, xad_driver_probe);
>>   if (rc) goto err_plat;
>>

I think the following function call to platform_device_register_simple()
and if() does not belong here.

As was said before, "devices" are registered by the (platform) bus. Your
driver module, needs to just register, well, the "driver". You are doing
this above - and that's it: (of_)platform_driver_register().

>>   // Device
>>   pdev=platform_device_register_simple("xps-acqui-data", -1, NULL, 0);
>>   if (IS_ERR(pdev)) {
>>           rc = PTR_ERR(pdev);
>>           platform_driver_unregister(&xad_driver);
>>           goto err_plat;
>>   }
>>
>>
>>   return 0;
>>
>> err_plat:
>>   unregister_chrdev_region(first, count);
>> Err:
>>   printk(KERN_ERR SD "Module %s: Failed loading rc=%d\n" FL, NAME, rc);
>>   return rc;
>> }

 Joachim

RE: Getting the IRQ number (Was: Basic driver devel questions ?)

[David Laight]

=20

> That's the idea. Communication between usermode and the driver is
limited to=20
> simple ioctl calls and the driver receives an interrupt when certain
data has=20
> been placed in memory blocks by the hardware (like a DMA).=20
> Then the user prog figures out where that latest data buffer is (mmap)
and saves it.=20

I've used pread() and pwrite() to directly copy data to/from a devices
PCI memory space.
The driver just verifies the offset and calls copy_to/from_user() [1].
High offsets can be used to access driver specific data.
The userspace code for pread/write is less painful than using ioctl()
for everything.
This also lets you dump the device memory space with hexdump.

I also made the device interrupt generate POLLIN (this needs a bit of
care since the interrupt can arrive before poll() is called).

	David

[1] I actually had to use the PCIe dma controller to get adequate
throughput.

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Guillaume Dargaud]

Michael, 
in your example, when is foo_driver_probe() actually called ?
It's not being called when I do an insmod.
-- 
Guillaume Dargaud
http://www.gdargaud.net/

Re: Getting the IRQ number (Was: Basic driver devel questions ?)

[Michael Ellerman]

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

On Tue, 2010-12-07 at 13:46 +0100, Guillaume Dargaud wrote:
> Michael, 
> in your example, when is foo_driver_probe() actually called ?
> It's not being called when I do an insmod.

It should be called when the driver core finds a device that matches
your match table.

When you register your driver the driver core will iterate over all
devices on the platform bus and if they match then it will call your
probe routine.

I assume you've update the compatible property in the match table, I'm
not sure what else could be causing it to not be called.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]