i2c child devices: binding to uio device driver

i2c child devices: binding to uio device driver

[Thomas De Schampheleire]

Hi,

I am using the uio framework (userspace i/o) to handle devices in
userspace. This includes memory access and interrupt handling.
For i2c devices, I'm using the i2c-dev interface, to talk to these
devices from userspace. This already works fine.

One of my i2c devices also has an interrupt line connected to the
processor. To be able to handle this interrupt with uio, I need to
have a kernel driver matched against the i2c device. However, the i2c
device described in the device tree does not bind with the driver. Is
this behavior supported, and if so, what am I doing wrong? Do I have
to create a dummy bus (non-i2c) and add a shadow node for the
interrupt?

My device tree nodes look like this:

        soc@fe000000 {
                i2c@118100 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        cell-index = <1>;
                        compatible = "fsl-i2c";
                        reg = <0x118100 0x100>;
                        interrupts = <38 2 0 0>;
                        dfsrr;
                        mydevice@68 {
                                compatible = "mymanufacturer,mydevice";
                                reg = <0x68>;
                                interrupts = <7 1 0 0>; /* External
IRQ7, active-low level */
                        };
                };

The device driver then has:

static const struct of_device_id mydevice_ids[] = {
        {
                .compatible = "mymanufacturer,mydevice"
        },
        {},
};
static struct of_platform_driver mydevice_driver = {
        .owner          = THIS_MODULE,
        .name           = "mydevice",
        .match_table    = mydevice_ids,
        .probe          = mydevice_probe,
        .remove         = __devexit_p(mydevice_remove),
};

static int __init mydevice_init(void)
{
        printk(KERN_INFO "mydevice: driver init\n");
        return of_register_platform_driver(&mydevice_driver);
}

Note that the init function is called correctly, but the probe
function is never called.

All this is on a p4080ds-based system, with linux 2.6.34.6.

Best regards,
Thomas

Re: i2c child devices: binding to uio device driver

[Grant Likely]

On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
> Hi,
> 
> I am using the uio framework (userspace i/o) to handle devices in
> userspace. This includes memory access and interrupt handling.
> For i2c devices, I'm using the i2c-dev interface, to talk to these
> devices from userspace. This already works fine.
> 
> One of my i2c devices also has an interrupt line connected to the
> processor. To be able to handle this interrupt with uio, I need to
> have a kernel driver matched against the i2c device. However, the i2c
> device described in the device tree does not bind with the driver. Is
> this behavior supported, and if so, what am I doing wrong? Do I have
> to create a dummy bus (non-i2c) and add a shadow node for the
> interrupt?

You're wandering into new territory here.  It is definitely the right
thing to do to describe your i2c device in the device tree with the
interrupt fully specified.

What you probably need to do is create a uio helper driver for your
device that takes care of finding the device node, mapping the
interrupt, and proving the interrupt handling UIO hook to userspace.
The easiest way to work this out would be to make your uio helper
actually bind to the i2c device created by the device node (this is
probably the best way too), but you don't have to.  As long as you
arrange for you uio helper to get called at some point, you can get it
to find the appropriate node in the DT, map the interrupt, and export
it to userspace.

g.

> 
> My device tree nodes look like this:
> 
>         soc@fe000000 {
>                 i2c@118100 {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         cell-index = <1>;
>                         compatible = "fsl-i2c";
>                         reg = <0x118100 0x100>;
>                         interrupts = <38 2 0 0>;
>                         dfsrr;
>                         mydevice@68 {
>                                 compatible = "mymanufacturer,mydevice";
>                                 reg = <0x68>;
>                                 interrupts = <7 1 0 0>; /* External
> IRQ7, active-low level */
>                         };
>                 };

This looks to be correct.

> 
> The device driver then has:
> 
> static const struct of_device_id mydevice_ids[] = {
>         {
>                 .compatible = "mymanufacturer,mydevice"
>         },
>         {},
> };
> static struct of_platform_driver mydevice_driver = {

You don't want to do this.  This is an i2c device, not a platform
device.  If anything, this should be an i2c_driver and you should use
the normal creation of i2c devices from the DT.

BTW, of_platform_driver is deprecated and is just a wrapper around
plain platform_driver on recent kernels.  platform_driver should be
used directly now.

>         .owner          = THIS_MODULE,
>         .name           = "mydevice",
>         .match_table    = mydevice_ids,
>         .probe          = mydevice_probe,
>         .remove         = __devexit_p(mydevice_remove),
> };
> 
> static int __init mydevice_init(void)
> {
>         printk(KERN_INFO "mydevice: driver init\n");
>         return of_register_platform_driver(&mydevice_driver);
> }
> 
> Note that the init function is called correctly, but the probe
> function is never called.
> 
> All this is on a p4080ds-based system, with linux 2.6.34.6.
> 
> Best regards,
> Thomas
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: i2c child devices: binding to uio device driver

[Thomas De Schampheleire]

Hi Grant,

On Thu, May 19, 2011 at 5:58 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
>> Hi,
>>
>> I am using the uio framework (userspace i/o) to handle devices in
>> userspace. This includes memory access and interrupt handling.
>> For i2c devices, I'm using the i2c-dev interface, to talk to these
>> devices from userspace. This already works fine.
>>
>> One of my i2c devices also has an interrupt line connected to the
>> processor. To be able to handle this interrupt with uio, I need to
>> have a kernel driver matched against the i2c device. However, the i2c
>> device described in the device tree does not bind with the driver. Is
>> this behavior supported, and if so, what am I doing wrong? Do I have
>> to create a dummy bus (non-i2c) and add a shadow node for the
>> interrupt?
>
> You're wandering into new territory here.  It is definitely the right
> thing to do to describe your i2c device in the device tree with the
> interrupt fully specified.
>
> What you probably need to do is create a uio helper driver for your
> device that takes care of finding the device node, mapping the
> interrupt, and proving the interrupt handling UIO hook to userspace.
> The easiest way to work this out would be to make your uio helper
> actually bind to the i2c device created by the device node (this is
> probably the best way too), but you don't have to.  As long as you
> arrange for you uio helper to get called at some point, you can get it
> to find the appropriate node in the DT, map the interrupt, and export
> it to userspace.

Binding the helper driver with the i2c device in the device tree is
indeed what I am trying to achieve. Unfortunately, the binding does
not happen, the probe function of my driver never gets called.

For the other UIO devices in this system (non-I2C), the mechanism is as follows:
1. the devices are described in the device tree, as children under
their respective bus node (e.g. localbus)
2. the platform code (arch/powerpc/platforms/85xx/corenet_ds.c)
iterates over all buses it understands, among which 'simple-bus'.
3. for each child device on the bus, a bind operation with a driver is
attempted, based on the device 'compatible' string

This works perfectly.
However, the i2c bus does not have any of the matched bus types, like
simple-bus, and as such it is handled differently. The i2c bus driver
(drivers/i2c/busses/i2c-mpc.c) calls of_register_i2c_devices (in
drivers/of/of_i2c.c), which handles the i2c-specific part of the
device, but does not (as far as I understand and observe) treat the
device as a regular device with a match table.

Maybe I misunderstand something. Is this where I need to play with
modaliases? How does this work in the context of device trees?

>
> g.
>
>>
>> My device tree nodes look like this:
>>
>>         soc@fe000000 {
>>                 i2c@118100 {
>>                         #address-cells = <1>;
>>                         #size-cells = <0>;
>>                         cell-index = <1>;
>>                         compatible = "fsl-i2c";
>>                         reg = <0x118100 0x100>;
>>                         interrupts = <38 2 0 0>;
>>                         dfsrr;
>>                         mydevice@68 {
>>                                 compatible = "mymanufacturer,mydevice";
>>                                 reg = <0x68>;
>>                                 interrupts = <7 1 0 0>; /* External
>> IRQ7, active-low level */
>>                         };
>>                 };
>
> This looks to be correct.
>
>>
>> The device driver then has:
>>
>> static const struct of_device_id mydevice_ids[] = {
>>         {
>>                 .compatible = "mymanufacturer,mydevice"
>>         },
>>         {},
>> };
>> static struct of_platform_driver mydevice_driver = {
>
> You don't want to do this.  This is an i2c device, not a platform
> device.  If anything, this should be an i2c_driver and you should use
> the normal creation of i2c devices from the DT.

Ok, so this may be the reason why my driver does not get registered
against the device...

>
> BTW, of_platform_driver is deprecated and is just a wrapper around
> plain platform_driver on recent kernels.  platform_driver should be
> used directly now.

Yes, I heard about this change. But if I recall correctly, this change
is not yet in 2.6.34, right?

Thanks,
Thomas

Re: i2c child devices: binding to uio device driver

[Thomas De Schampheleire]

Hi,

On Mon, May 23, 2011 at 10:13 AM, Thomas De Schampheleire
<patrickdepinguin+devicetree-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Grant,
>
> On Thu, May 19, 2011 at 5:58 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
>>> Hi,
>>>
>>> I am using the uio framework (userspace i/o) to handle devices in
>>> userspace. This includes memory access and interrupt handling.
>>> For i2c devices, I'm using the i2c-dev interface, to talk to these
>>> devices from userspace. This already works fine.
>>>
>>> One of my i2c devices also has an interrupt line connected to the
>>> processor. To be able to handle this interrupt with uio, I need to
>>> have a kernel driver matched against the i2c device. However, the i2c
>>> device described in the device tree does not bind with the driver. Is
>>> this behavior supported, and if so, what am I doing wrong? Do I have
>>> to create a dummy bus (non-i2c) and add a shadow node for the
>>> interrupt?
>>
>> You're wandering into new territory here.  It is definitely the right
>> thing to do to describe your i2c device in the device tree with the
>> interrupt fully specified.
>>
>> What you probably need to do is create a uio helper driver for your
>> device that takes care of finding the device node, mapping the
>> interrupt, and proving the interrupt handling UIO hook to userspace.
>> The easiest way to work this out would be to make your uio helper
>> actually bind to the i2c device created by the device node (this is
>> probably the best way too), but you don't have to.  As long as you
>> arrange for you uio helper to get called at some point, you can get it
>> to find the appropriate node in the DT, map the interrupt, and export
>> it to userspace.
>
> Binding the helper driver with the i2c device in the device tree is
> indeed what I am trying to achieve. Unfortunately, the binding does
> not happen, the probe function of my driver never gets called.
>
> For the other UIO devices in this system (non-I2C), the mechanism is as follows:
> 1. the devices are described in the device tree, as children under
> their respective bus node (e.g. localbus)
> 2. the platform code (arch/powerpc/platforms/85xx/corenet_ds.c)
> iterates over all buses it understands, among which 'simple-bus'.
> 3. for each child device on the bus, a bind operation with a driver is
> attempted, based on the device 'compatible' string
>
> This works perfectly.
> However, the i2c bus does not have any of the matched bus types, like
> simple-bus, and as such it is handled differently. The i2c bus driver
> (drivers/i2c/busses/i2c-mpc.c) calls of_register_i2c_devices (in
> drivers/of/of_i2c.c), which handles the i2c-specific part of the
> device, but does not (as far as I understand and observe) treat the
> device as a regular device with a match table.
>
> Maybe I misunderstand something. Is this where I need to play with
> modaliases? How does this work in the context of device trees?
>
>>
>> g.
>>
>>>
>>> My device tree nodes look like this:
>>>
>>>         soc@fe000000 {
>>>                 i2c@118100 {
>>>                         #address-cells = <1>;
>>>                         #size-cells = <0>;
>>>                         cell-index = <1>;
>>>                         compatible = "fsl-i2c";
>>>                         reg = <0x118100 0x100>;
>>>                         interrupts = <38 2 0 0>;
>>>                         dfsrr;
>>>                         mydevice@68 {
>>>                                 compatible = "mymanufacturer,mydevice";
>>>                                 reg = <0x68>;
>>>                                 interrupts = <7 1 0 0>; /* External
>>> IRQ7, active-low level */
>>>                         };
>>>                 };
>>
>> This looks to be correct.
>>
>>>
>>> The device driver then has:
>>>
>>> static const struct of_device_id mydevice_ids[] = {
>>>         {
>>>                 .compatible = "mymanufacturer,mydevice"
>>>         },
>>>         {},
>>> };
>>> static struct of_platform_driver mydevice_driver = {
>>
>> You don't want to do this.  This is an i2c device, not a platform
>> device.  If anything, this should be an i2c_driver and you should use
>> the normal creation of i2c devices from the DT.
>
> Ok, so this may be the reason why my driver does not get registered
> against the device...


I have it working now. The bottom line was indeed, as you hinted, to
transform the platform driver into an i2c driver, and make sure that
it gets bound to the device. Here indeed, I need to specify the right
modalias in the compatible string.

For the record, here is what needed to be done:
* In the device tree, add a child node to the i2c controller node. The
model part of the first compatible string, should correspond to the
'type' string you will specify in the driver. In this node, you can
also specify any interrupt assignments.

* The driver should be a regular i2c driver, but the 'type' field in
struct i2c_device_id should match what you specified in the device
tree. For example, if the device tree has 'mymanufacturer,mydevice' as
compatible string, then:

static struct i2c_device_id mydevice_ids[] = {
        { "mydevice", 0 },
        { }
};

static struct i2c_driver mydevice_driver = {
        .driver = {
                .name = "mydevice",
        },

        .id_table       = mydevice_ids,
        .probe          = mydevice_probe,
        .remove         = __devexit_p(mydevice_remove),
};

* In the probe function, to get the interrupt information out of the
device tree, you first have to find the correct node, for example
based on compatible string:

        np = of_find_compatible_node(NULL, NULL, "mymanufacturer,mydevice");
        if (NULL == np) {
                dev_err(&client->dev, "couldn't find corresponding
node in device tree\n");
                goto err_find_node;
        }

        {
                int irq = of_irq_to_resource(np, 0, NULL);
                if (unlikely(irq == NO_IRQ)) {
                        dev_warn(&client->dev, "couldn't obtain
interrupt information from device tree\n");
                } else {
                        ... (handle the UIO part)
                }
        }
        of_node_put(np);


Best regards,
Thomas

Re: i2c child devices: binding to uio device driver

[Grant Likely]

On Wed, May 25, 2011 at 08:39:26AM +0200, Thomas De Schampheleire wrote:
> Hi,
> 
> On Mon, May 23, 2011 at 10:13 AM, Thomas De Schampheleire
> <patrickdepinguin+devicetree-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Hi Grant,
> >
> > On Thu, May 19, 2011 at 5:58 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
> >>> Hi,
> >>>
> >>> I am using the uio framework (userspace i/o) to handle devices in
> >>> userspace. This includes memory access and interrupt handling.
> >>> For i2c devices, I'm using the i2c-dev interface, to talk to these
> >>> devices from userspace. This already works fine.
> >>>
> >>> One of my i2c devices also has an interrupt line connected to the
> >>> processor. To be able to handle this interrupt with uio, I need to
> >>> have a kernel driver matched against the i2c device. However, the i2c
> >>> device described in the device tree does not bind with the driver. Is
> >>> this behavior supported, and if so, what am I doing wrong? Do I have
> >>> to create a dummy bus (non-i2c) and add a shadow node for the
> >>> interrupt?
> >>
> >> You're wandering into new territory here.  It is definitely the right
> >> thing to do to describe your i2c device in the device tree with the
> >> interrupt fully specified.
> >>
> >> What you probably need to do is create a uio helper driver for your
> >> device that takes care of finding the device node, mapping the
> >> interrupt, and proving the interrupt handling UIO hook to userspace.
> >> The easiest way to work this out would be to make your uio helper
> >> actually bind to the i2c device created by the device node (this is
> >> probably the best way too), but you don't have to.  As long as you
> >> arrange for you uio helper to get called at some point, you can get it
> >> to find the appropriate node in the DT, map the interrupt, and export
> >> it to userspace.
> >
> > Binding the helper driver with the i2c device in the device tree is
> > indeed what I am trying to achieve. Unfortunately, the binding does
> > not happen, the probe function of my driver never gets called.
> >
> > For the other UIO devices in this system (non-I2C), the mechanism is as follows:
> > 1. the devices are described in the device tree, as children under
> > their respective bus node (e.g. localbus)
> > 2. the platform code (arch/powerpc/platforms/85xx/corenet_ds.c)
> > iterates over all buses it understands, among which 'simple-bus'.
> > 3. for each child device on the bus, a bind operation with a driver is
> > attempted, based on the device 'compatible' string
> >
> > This works perfectly.
> > However, the i2c bus does not have any of the matched bus types, like
> > simple-bus, and as such it is handled differently. The i2c bus driver
> > (drivers/i2c/busses/i2c-mpc.c) calls of_register_i2c_devices (in
> > drivers/of/of_i2c.c), which handles the i2c-specific part of the
> > device, but does not (as far as I understand and observe) treat the
> > device as a regular device with a match table.
> >
> > Maybe I misunderstand something. Is this where I need to play with
> > modaliases? How does this work in the context of device trees?
> >
> >>
> >> g.
> >>
> >>>
> >>> My device tree nodes look like this:
> >>>
> >>>         soc@fe000000 {
> >>>                 i2c@118100 {
> >>>                         #address-cells = <1>;
> >>>                         #size-cells = <0>;
> >>>                         cell-index = <1>;
> >>>                         compatible = "fsl-i2c";
> >>>                         reg = <0x118100 0x100>;
> >>>                         interrupts = <38 2 0 0>;
> >>>                         dfsrr;
> >>>                         mydevice@68 {
> >>>                                 compatible = "mymanufacturer,mydevice";
> >>>                                 reg = <0x68>;
> >>>                                 interrupts = <7 1 0 0>; /* External
> >>> IRQ7, active-low level */
> >>>                         };
> >>>                 };
> >>
> >> This looks to be correct.
> >>
> >>>
> >>> The device driver then has:
> >>>
> >>> static const struct of_device_id mydevice_ids[] = {
> >>>         {
> >>>                 .compatible = "mymanufacturer,mydevice"
> >>>         },
> >>>         {},
> >>> };
> >>> static struct of_platform_driver mydevice_driver = {
> >>
> >> You don't want to do this.  This is an i2c device, not a platform
> >> device.  If anything, this should be an i2c_driver and you should use
> >> the normal creation of i2c devices from the DT.
> >
> > Ok, so this may be the reason why my driver does not get registered
> > against the device...
> 
> 
> I have it working now. The bottom line was indeed, as you hinted, to
> transform the platform driver into an i2c driver, and make sure that
> it gets bound to the device. Here indeed, I need to specify the right
> modalias in the compatible string.

Glad to hear.  Thanks for reporting back.  Comments below.

> 
> For the record, here is what needed to be done:
> * In the device tree, add a child node to the i2c controller node. The
> model part of the first compatible string, should correspond to the
> 'type' string you will specify in the driver. In this node, you can
> also specify any interrupt assignments.
> 
> * The driver should be a regular i2c driver, but the 'type' field in
> struct i2c_device_id should match what you specified in the device
> tree. For example, if the device tree has 'mymanufacturer,mydevice' as
> compatible string, then:
> 
> static struct i2c_device_id mydevice_ids[] = {
>         { "mydevice", 0 },
>         { }
> };
> 
> static struct i2c_driver mydevice_driver = {
>         .driver = {
>                 .name = "mydevice",
>         },
> 
>         .id_table       = mydevice_ids,
>         .probe          = mydevice_probe,
>         .remove         = __devexit_p(mydevice_remove),
> };

This is a heuristic for getting device tree nodes to work with
existing i2c drivers.  However, it is very important that you *do not*
lie about what the device is in the compatible property to get it to
bind to the device.  So, if the value that best describes the device
is not in the i2c_device_id list, then you should add a proper
i2c_driver->driver->of_match_table that does a correct match to a
device tree compatible property.

I may very well remove the heuristic at some point in the future and
require all device tree binding to use the of_match_table method.

> 
> * In the probe function, to get the interrupt information out of the
> device tree, you first have to find the correct node, for example
> based on compatible string:
> 
>         np = of_find_compatible_node(NULL, NULL, "mymanufacturer,mydevice");
>         if (NULL == np) {
>                 dev_err(&client->dev, "couldn't find corresponding
> node in device tree\n");
>                 goto err_find_node;
>         }

You don't need to jump through this hoop.  The node pointer is already
populated in client->dev->of_node.

> 
>         {
>                 int irq = of_irq_to_resource(np, 0, NULL);
>                 if (unlikely(irq == NO_IRQ)) {
>                         dev_warn(&client->dev, "couldn't obtain
> interrupt information from device tree\n");
>                 } else {
>                         ... (handle the UIO part)
>                 }
>         }

Similarly, the irq should already be populated in client->irq.

Otherwise, looks correct.

g.

Re: i2c child devices: binding to uio device driver

[Thomas De Schampheleire]

On Fri, May 27, 2011 at 9:17 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Wed, May 25, 2011 at 08:39:26AM +0200, Thomas De Schampheleire wrote:
>> Hi,
>>
>> On Mon, May 23, 2011 at 10:13 AM, Thomas De Schampheleire
>> <patrickdepinguin+devicetree-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > Hi Grant,
>> >
>> > On Thu, May 19, 2011 at 5:58 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> >> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
>> >>> Hi,
>> >>>
>> >>> I am using the uio framework (userspace i/o) to handle devices in
>> >>> userspace. This includes memory access and interrupt handling.
>> >>> For i2c devices, I'm using the i2c-dev interface, to talk to these
>> >>> devices from userspace. This already works fine.
>> >>>
>> >>> One of my i2c devices also has an interrupt line connected to the
>> >>> processor. To be able to handle this interrupt with uio, I need to
>> >>> have a kernel driver matched against the i2c device. However, the i2c
>> >>> device described in the device tree does not bind with the driver. Is
>> >>> this behavior supported, and if so, what am I doing wrong? Do I have
>> >>> to create a dummy bus (non-i2c) and add a shadow node for the
>> >>> interrupt?
>> >>
>> >> You're wandering into new territory here.  It is definitely the right
>> >> thing to do to describe your i2c device in the device tree with the
>> >> interrupt fully specified.
>> >>
>> >> What you probably need to do is create a uio helper driver for your
>> >> device that takes care of finding the device node, mapping the
>> >> interrupt, and proving the interrupt handling UIO hook to userspace.
>> >> The easiest way to work this out would be to make your uio helper
>> >> actually bind to the i2c device created by the device node (this is
>> >> probably the best way too), but you don't have to.  As long as you
>> >> arrange for you uio helper to get called at some point, you can get it
>> >> to find the appropriate node in the DT, map the interrupt, and export
>> >> it to userspace.
>> >
>> > Binding the helper driver with the i2c device in the device tree is
>> > indeed what I am trying to achieve. Unfortunately, the binding does
>> > not happen, the probe function of my driver never gets called.
>> >
>> > For the other UIO devices in this system (non-I2C), the mechanism is as follows:
>> > 1. the devices are described in the device tree, as children under
>> > their respective bus node (e.g. localbus)
>> > 2. the platform code (arch/powerpc/platforms/85xx/corenet_ds.c)
>> > iterates over all buses it understands, among which 'simple-bus'.
>> > 3. for each child device on the bus, a bind operation with a driver is
>> > attempted, based on the device 'compatible' string
>> >
>> > This works perfectly.
>> > However, the i2c bus does not have any of the matched bus types, like
>> > simple-bus, and as such it is handled differently. The i2c bus driver
>> > (drivers/i2c/busses/i2c-mpc.c) calls of_register_i2c_devices (in
>> > drivers/of/of_i2c.c), which handles the i2c-specific part of the
>> > device, but does not (as far as I understand and observe) treat the
>> > device as a regular device with a match table.
>> >
>> > Maybe I misunderstand something. Is this where I need to play with
>> > modaliases? How does this work in the context of device trees?
>> >
>> >>
>> >> g.
>> >>
>> >>>
>> >>> My device tree nodes look like this:
>> >>>
>> >>>         soc@fe000000 {
>> >>>                 i2c@118100 {
>> >>>                         #address-cells = <1>;
>> >>>                         #size-cells = <0>;
>> >>>                         cell-index = <1>;
>> >>>                         compatible = "fsl-i2c";
>> >>>                         reg = <0x118100 0x100>;
>> >>>                         interrupts = <38 2 0 0>;
>> >>>                         dfsrr;
>> >>>                         mydevice@68 {
>> >>>                                 compatible = "mymanufacturer,mydevice";
>> >>>                                 reg = <0x68>;
>> >>>                                 interrupts = <7 1 0 0>; /* External
>> >>> IRQ7, active-low level */
>> >>>                         };
>> >>>                 };
>> >>
>> >> This looks to be correct.
>> >>
>> >>>
>> >>> The device driver then has:
>> >>>
>> >>> static const struct of_device_id mydevice_ids[] = {
>> >>>         {
>> >>>                 .compatible = "mymanufacturer,mydevice"
>> >>>         },
>> >>>         {},
>> >>> };
>> >>> static struct of_platform_driver mydevice_driver = {
>> >>
>> >> You don't want to do this.  This is an i2c device, not a platform
>> >> device.  If anything, this should be an i2c_driver and you should use
>> >> the normal creation of i2c devices from the DT.
>> >
>> > Ok, so this may be the reason why my driver does not get registered
>> > against the device...
>>
>>
>> I have it working now. The bottom line was indeed, as you hinted, to
>> transform the platform driver into an i2c driver, and make sure that
>> it gets bound to the device. Here indeed, I need to specify the right
>> modalias in the compatible string.
>
> Glad to hear.  Thanks for reporting back.  Comments below.

Seems that I spoke too soon.
Although the binding of the device and the driver works correctly, I
now have problems with using the i2c device from userspace. After
having transformed my platform driver into an i2c driver, setting the
same slave address from userspace fails. This is not what I wanted.

How do I get around this? To get proper binding I need an I2C device,
but to be able to use i2c-dev I shouldn't have an I2C device.

It seems that Arnd's suggestion to make i2c-dev interrupt aware, comes
into play here. However, how will i2c-dev know which interrupts belong
to which device? To use i2c-dev without interrupts, all you need in
the device tree is the i2c controller nodes.

>
>>
>> For the record, here is what needed to be done:
>> * In the device tree, add a child node to the i2c controller node. The
>> model part of the first compatible string, should correspond to the
>> 'type' string you will specify in the driver. In this node, you can
>> also specify any interrupt assignments.
>>
>> * The driver should be a regular i2c driver, but the 'type' field in
>> struct i2c_device_id should match what you specified in the device
>> tree. For example, if the device tree has 'mymanufacturer,mydevice' as
>> compatible string, then:
>>
>> static struct i2c_device_id mydevice_ids[] = {
>>         { "mydevice", 0 },
>>         { }
>> };
>>
>> static struct i2c_driver mydevice_driver = {
>>         .driver = {
>>                 .name = "mydevice",
>>         },
>>
>>         .id_table       = mydevice_ids,
>>         .probe          = mydevice_probe,
>>         .remove         = __devexit_p(mydevice_remove),
>> };
>
> This is a heuristic for getting device tree nodes to work with
> existing i2c drivers.  However, it is very important that you *do not*
> lie about what the device is in the compatible property to get it to
> bind to the device.  So, if the value that best describes the device
> is not in the i2c_device_id list, then you should add a proper
> i2c_driver->driver->of_match_table that does a correct match to a
> device tree compatible property.
>
> I may very well remove the heuristic at some point in the future and
> require all device tree binding to use the of_match_table method.

Thanks for the hint. Unfortunately, I'm still at 2.6.34 and these
changes were made in later kernel versions.
I'll certainly keep that in mind once we update, though.

>
>>
>> * In the probe function, to get the interrupt information out of the
>> device tree, you first have to find the correct node, for example
>> based on compatible string:
>>
>>         np = of_find_compatible_node(NULL, NULL, "mymanufacturer,mydevice");
>>         if (NULL == np) {
>>                 dev_err(&client->dev, "couldn't find corresponding
>> node in device tree\n");
>>                 goto err_find_node;
>>         }
>
> You don't need to jump through this hoop.  The node pointer is already
> populated in client->dev->of_node.
>
>>
>>         {
>>                 int irq = of_irq_to_resource(np, 0, NULL);
>>                 if (unlikely(irq == NO_IRQ)) {
>>                         dev_warn(&client->dev, "couldn't obtain
>> interrupt information from device tree\n");
>>                 } else {
>>                         ... (handle the UIO part)
>>                 }
>>         }
>
> Similarly, the irq should already be populated in client->irq.

Again, this great feature was only introduced in later versions than
2.6.34. But thanks for mentioning this, I wasn't aware of it before.

Thomas

Re: i2c child devices: binding to uio device driver

[Grant Likely]

On Fri, May 27, 2011 at 12:33:18PM +0200, Thomas De Schampheleire wrote:
> On Fri, May 27, 2011 at 9:17 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Wed, May 25, 2011 at 08:39:26AM +0200, Thomas De Schampheleire wrote:
> >> Hi,
> >>
> >> On Mon, May 23, 2011 at 10:13 AM, Thomas De Schampheleire
> >> <patrickdepinguin+devicetree-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > Hi Grant,
> >> >
> >> > On Thu, May 19, 2011 at 5:58 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >> >> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
> >> >>> Hi,
> >> >>>
> >> >>> I am using the uio framework (userspace i/o) to handle devices in
> >> >>> userspace. This includes memory access and interrupt handling.
> >> >>> For i2c devices, I'm using the i2c-dev interface, to talk to these
> >> >>> devices from userspace. This already works fine.
> >> >>>
> >> >>> One of my i2c devices also has an interrupt line connected to the
> >> >>> processor. To be able to handle this interrupt with uio, I need to
> >> >>> have a kernel driver matched against the i2c device. However, the i2c
> >> >>> device described in the device tree does not bind with the driver. Is
> >> >>> this behavior supported, and if so, what am I doing wrong? Do I have
> >> >>> to create a dummy bus (non-i2c) and add a shadow node for the
> >> >>> interrupt?
> >> >>
> >> >> You're wandering into new territory here.  It is definitely the right
> >> >> thing to do to describe your i2c device in the device tree with the
> >> >> interrupt fully specified.
> >> >>
> >> >> What you probably need to do is create a uio helper driver for your
> >> >> device that takes care of finding the device node, mapping the
> >> >> interrupt, and proving the interrupt handling UIO hook to userspace.
> >> >> The easiest way to work this out would be to make your uio helper
> >> >> actually bind to the i2c device created by the device node (this is
> >> >> probably the best way too), but you don't have to.  As long as you
> >> >> arrange for you uio helper to get called at some point, you can get it
> >> >> to find the appropriate node in the DT, map the interrupt, and export
> >> >> it to userspace.
> >> >
> >> > Binding the helper driver with the i2c device in the device tree is
> >> > indeed what I am trying to achieve. Unfortunately, the binding does
> >> > not happen, the probe function of my driver never gets called.
> >> >
> >> > For the other UIO devices in this system (non-I2C), the mechanism is as follows:
> >> > 1. the devices are described in the device tree, as children under
> >> > their respective bus node (e.g. localbus)
> >> > 2. the platform code (arch/powerpc/platforms/85xx/corenet_ds.c)
> >> > iterates over all buses it understands, among which 'simple-bus'.
> >> > 3. for each child device on the bus, a bind operation with a driver is
> >> > attempted, based on the device 'compatible' string
> >> >
> >> > This works perfectly.
> >> > However, the i2c bus does not have any of the matched bus types, like
> >> > simple-bus, and as such it is handled differently. The i2c bus driver
> >> > (drivers/i2c/busses/i2c-mpc.c) calls of_register_i2c_devices (in
> >> > drivers/of/of_i2c.c), which handles the i2c-specific part of the
> >> > device, but does not (as far as I understand and observe) treat the
> >> > device as a regular device with a match table.
> >> >
> >> > Maybe I misunderstand something. Is this where I need to play with
> >> > modaliases? How does this work in the context of device trees?
> >> >
> >> >>
> >> >> g.
> >> >>
> >> >>>
> >> >>> My device tree nodes look like this:
> >> >>>
> >> >>>         soc@fe000000 {
> >> >>>                 i2c@118100 {
> >> >>>                         #address-cells = <1>;
> >> >>>                         #size-cells = <0>;
> >> >>>                         cell-index = <1>;
> >> >>>                         compatible = "fsl-i2c";
> >> >>>                         reg = <0x118100 0x100>;
> >> >>>                         interrupts = <38 2 0 0>;
> >> >>>                         dfsrr;
> >> >>>                         mydevice@68 {
> >> >>>                                 compatible = "mymanufacturer,mydevice";
> >> >>>                                 reg = <0x68>;
> >> >>>                                 interrupts = <7 1 0 0>; /* External
> >> >>> IRQ7, active-low level */
> >> >>>                         };
> >> >>>                 };
> >> >>
> >> >> This looks to be correct.
> >> >>
> >> >>>
> >> >>> The device driver then has:
> >> >>>
> >> >>> static const struct of_device_id mydevice_ids[] = {
> >> >>>         {
> >> >>>                 .compatible = "mymanufacturer,mydevice"
> >> >>>         },
> >> >>>         {},
> >> >>> };
> >> >>> static struct of_platform_driver mydevice_driver = {
> >> >>
> >> >> You don't want to do this.  This is an i2c device, not a platform
> >> >> device.  If anything, this should be an i2c_driver and you should use
> >> >> the normal creation of i2c devices from the DT.
> >> >
> >> > Ok, so this may be the reason why my driver does not get registered
> >> > against the device...
> >>
> >>
> >> I have it working now. The bottom line was indeed, as you hinted, to
> >> transform the platform driver into an i2c driver, and make sure that
> >> it gets bound to the device. Here indeed, I need to specify the right
> >> modalias in the compatible string.
> >
> > Glad to hear.  Thanks for reporting back.  Comments below.
> 
> Seems that I spoke too soon.
> Although the binding of the device and the driver works correctly, I
> now have problems with using the i2c device from userspace. After
> having transformed my platform driver into an i2c driver, setting the
> same slave address from userspace fails. This is not what I wanted.
> 
> How do I get around this? To get proper binding I need an I2C device,
> but to be able to use i2c-dev I shouldn't have an I2C device.

You're venturing into new territory here.  You'll either need to adapt
i2c-dev to coexist peacefully with a real i2c_client device, or you'll
need to modify i2c-dev to parse the device tree and decode the irq for
you.

> It seems that Arnd's suggestion to make i2c-dev interrupt aware, comes
> into play here. However, how will i2c-dev know which interrupts belong
> to which device? To use i2c-dev without interrupts, all you need in
> the device tree is the i2c controller nodes.

You'll need i2c-dev to find the associated node in the device tree,
possibly with an ioctl call.  I don't know the best way to proceed
here, sorry.  Jean might have better suggestions, but in general I
don't think i2c-dev is intended to provide any irq infrastructure.

g.

Re: i2c child devices: binding to uio device driver

[Arnd Bergmann]

On Thursday 19 May 2011, Grant Likely wrote:
> On Thu, May 19, 2011 at 05:32:32PM +0200, Thomas De Schampheleire wrote:
> > Hi,
> > 
> > I am using the uio framework (userspace i/o) to handle devices in
> > userspace. This includes memory access and interrupt handling.
> > For i2c devices, I'm using the i2c-dev interface, to talk to these
> > devices from userspace. This already works fine.

Ok.

> > One of my i2c devices also has an interrupt line connected to the
> > processor. To be able to handle this interrupt with uio, I need to
> > have a kernel driver matched against the i2c device. However, the i2c
> > device described in the device tree does not bind with the driver. Is
> > this behavior supported, and if so, what am I doing wrong? Do I have
> > to create a dummy bus (non-i2c) and add a shadow node for the
> > interrupt?
> 
> You're wandering into new territory here.  It is definitely the right
> thing to do to describe your i2c device in the device tree with the
> interrupt fully specified.

Agreed.

> What you probably need to do is create a uio helper driver for your
> device that takes care of finding the device node, mapping the
> interrupt, and proving the interrupt handling UIO hook to userspace.
> The easiest way to work this out would be to make your uio helper
> actually bind to the i2c device created by the device node (this is
> probably the best way too), but you don't have to.  As long as you
> arrange for you uio helper to get called at some point, you can get it
> to find the appropriate node in the DT, map the interrupt, and export
> it to userspace.

My feeling is that you should not be using a UIO device for this
case, but rather extend the i2c-dev interface to provide an interrupt
passthrough for devices that have an interrupt line. In that case,
you will not need any UIO bindings, and the same i2c-dev user space
driver will simply work without much extra work on any such device.

The best way to hook up the interrupt into i2c-dev is probably
to provide a poll() callback for waiting for an interrupt, which
signals POLLMSG, plus a new i2c-dev ioctl that is used to clear the
interrupt state.

	Arnd