Need some guidance on i2c-ocores driver

DISCUSSION York Sun 4 participants 15 messages Apr 17, 2015 – May 8, 2015 I2C subsystem host drivers

lore

Share & tools

lore

York Sun

author

2015-04-17 00:02 UTC #0 lore raw

Julia and other experts,

I am seeking for help on my device driver.

I am working on a module driver for a FPGA design with open core I2C controller memory-mapped to BAR2. I have searched up and down and found only one driver (drivers/mfd/timberdale.c) with similar implementation. Following timberdale driver, I am able to load the driver, but blocked by devm_ioremap_resource(). It is always in conflict with my BAR2. I wonder if I did something wrong. Here is the flow I tracked so far. (By the way, I am using an older kernel 3.12. The new 4.0 kernel crashes when booting on my platform. I plan to move to newer kernel later).

mfd_add_devices()
|
|--mfd_add_device()

    |
    |--platform_device_add()
        |
        |--insert_resource()  /* this passed */
        |   |
        |   |--insert_resource_conflict()
        |
        |--device_add()
            |
            |--bus_probe_device()
                |
                |--devm_ioremap_resource()
                    |
                    |--devm_request_mem_region()
                        |
                        |--__request_region() /* this always conflicts */
                            |
                            |--__request_resource()

My driver is called RedStone DMA. Here is my debug output

Quoted 7 lines

root@p1022ds:~# insmod redstone_mfd.ko 
RedStone DMA 0002:05:00.0: pci_enable_device() successful
RedStone DMA 0002:05:00.0: MSI-X init successful
York kernel: Calling devm_ioremap_resource()
York kernel kernel/resource.c __request_region 977: conflict=[??? 0xc00080000-0xc00087fff flags 0x80000000]
ocores-i2c ocores-i2c: can't request region for resource [mem 0xc00086000-0xc0008601f]
ocores-i2c: probe of ocores-i2c failed with error -16

Quoted 8 lines

RedStone DMA 0002:05:00.0: BAR[0] 0x0000000c00000000-0x0000000c0007ffff flags 0x0014220c, length 524288
RedStone DMA 0002:05:00.0: BAR[1] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[2] 0x0000000c00080000-0x0000000c00087fff flags 0x00040200, length 32768
RedStone DMA 0002:05:00.0: BAR[3] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[4] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[5] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: Version 1.4, built on 4-16-15, id 0
root@p1022ds:~#

Can you shed some light on this?

Thanks.

York

York Sun

2015-04-17 15:47 UTC #1 lore raw

Resend to LKML

Lee,

This question is actually more about MFD. Can you point me to the possible causes for my failure below?

Thanks.

York

Quoted 59 lines · York Sun

Julia and other experts,
I am seeking for help on my device driver.
I am working on a module driver for a FPGA design with open core I2C controller memory-mapped to BAR2. I have searched up and down and found only one driver (drivers/mfd/timberdale.c) with similar implementation. Following timberdale driver, I am able to load the driver, but blocked by devm_ioremap_resource(). It is always in conflict with my BAR2. I wonder if I did something wrong. Here is the flow I tracked so far. (By the way, I am using an older kernel 3.12. The new 4.0 kernel crashes when booting on my platform. I plan to move to newer kernel later).
mfd_add_devices()
|
|--mfd_add_device()
    |
    |--platform_device_add()
        |
        |--insert_resource()  /* this passed */
        |   |
        |   |--insert_resource_conflict()
        |
        |--device_add()
            |
            |--bus_probe_device()
                |
                |--devm_ioremap_resource()
                    |
                    |--devm_request_mem_region()
                        |
                        |--__request_region() /* this always conflicts */
                            |
                            |--__request_resource()
My driver is called RedStone DMA. Here is my debug output
Quoted 7 lines
root@p1022ds:~# insmod redstone_mfd.ko 
RedStone DMA 0002:05:00.0: pci_enable_device() successful
RedStone DMA 0002:05:00.0: MSI-X init successful
York kernel: Calling devm_ioremap_resource()
York kernel kernel/resource.c __request_region 977: conflict=[??? 0xc00080000-0xc00087fff flags 0x80000000]
ocores-i2c ocores-i2c: can't request region for resource [mem 0xc00086000-0xc0008601f]
ocores-i2c: probe of ocores-i2c failed with error -16
Quoted 8 lines
RedStone DMA 0002:05:00.0: BAR[0] 0x0000000c00000000-0x0000000c0007ffff flags 0x0014220c, length 524288
RedStone DMA 0002:05:00.0: BAR[1] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[2] 0x0000000c00080000-0x0000000c00087fff flags 0x00040200, length 32768
RedStone DMA 0002:05:00.0: BAR[3] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[4] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[5] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: Version 1.4, built on 4-16-15, id 0
root@p1022ds:~# 
Can you shed some light on this?
Thanks.
York

Lee Jones

2015-04-20 06:42 UTC #2 lore raw

Quoted 6 lines · York Sun

Resend to LKML
Lee,
This question is actually more about MFD. Can you point me to the possible causes for my failure below?

It's hard to tell exactly without code, but it looks like you're trying to allocate overlapping memory regions. Double check all of your addresses. For DT you need to take a look at your 'reg' properties, for traditional platform data it's best to grep for IORESOURCE_MEM.

Quoted 65 lines

Thanks.
York
Quoted 59 lines · York Sun
Julia and other experts,
I am seeking for help on my device driver.
I am working on a module driver for a FPGA design with open core I2C controller memory-mapped to BAR2. I have searched up and down and found only one driver (drivers/mfd/timberdale.c) with similar implementation. Following timberdale driver, I am able to load the driver, but blocked by devm_ioremap_resource(). It is always in conflict with my BAR2. I wonder if I did something wrong. Here is the flow I tracked so far. (By the way, I am using an older kernel 3.12. The new 4.0 kernel crashes when booting on my platform. I plan to move to newer kernel later).
mfd_add_devices()
|
|--mfd_add_device()
    |
    |--platform_device_add()
        |
        |--insert_resource()  /* this passed */
        |   |
        |   |--insert_resource_conflict()
        |
        |--device_add()
            |
            |--bus_probe_device()
                |
                |--devm_ioremap_resource()
                    |
                    |--devm_request_mem_region()
                        |
                        |--__request_region() /* this always conflicts */
                            |
                            |--__request_resource()
My driver is called RedStone DMA. Here is my debug output
Quoted 7 lines
root@p1022ds:~# insmod redstone_mfd.ko 
RedStone DMA 0002:05:00.0: pci_enable_device() successful
RedStone DMA 0002:05:00.0: MSI-X init successful
York kernel: Calling devm_ioremap_resource()
York kernel kernel/resource.c __request_region 977: conflict=[??? 0xc00080000-0xc00087fff flags 0x80000000]
ocores-i2c ocores-i2c: can't request region for resource [mem 0xc00086000-0xc0008601f]
ocores-i2c: probe of ocores-i2c failed with error -16
Quoted 8 lines
RedStone DMA 0002:05:00.0: BAR[0] 0x0000000c00000000-0x0000000c0007ffff flags 0x0014220c, length 524288
RedStone DMA 0002:05:00.0: BAR[1] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[2] 0x0000000c00080000-0x0000000c00087fff flags 0x00040200, length 32768
RedStone DMA 0002:05:00.0: BAR[3] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[4] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: BAR[5] 0x0000000000000000-0x0000000000000000 flags 0x00000000, length 0
RedStone DMA 0002:05:00.0: Version 1.4, built on 4-16-15, id 0
root@p1022ds:~# 
Can you shed some light on this?
Thanks.
York

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

York Sun

2015-04-20 16:24 UTC #3 lore raw

Quoted 6 lines · Lee Jones

Quoted 4 lines · York Sun
Resend to LKML
Lee,

Quoted 9 lines

Quoted 2 lines
This question is actually more about MFD. Can you point me to the possible causes for my failure below?
It's hard to tell exactly without code, but it looks like you're trying to allocate overlapping memory regions. Double check all of your addresses. For DT you need to take a look at your 'reg' properties, for traditional platform data it's best to grep for IORESOURCE_MEM.

Lee,

It _is_ overlapping. How could it not be? The resource for the I2C is mapped to BAR2. So the resource is overlapping with BAR2. It is alway the case, isn't it? What I don't understand is how MFD works with the resources if it is guaranteed overlapping. Did I get something wrong?

Look at the reference code I took, drivers/mfd/timberdale.c, when mfd_add_devices() is called, it uses &dev->resource as the base. So the BAR will be the parent. Check the code in mfd-core.c, mfd_add_device(),

if ((cell->resources[r].flags & IORESOURCE_MEM) && mem_base) {
	res[r].parent = mem_base;
	res[r].start = mem_base->start + cell->resources[r].start;
	res[r].end = mem_base->start + cell->resources[r].end;
}

So the MFD resource is within its parent. When later the device driver request a region, will it get conflict with the parent?

York

Lee Jones

2015-04-20 18:16 UTC #4 lore raw

Quoted 29 lines · York Sun

Quoted 6 lines · Lee Jones
Quoted 4 lines · York Sun
Resend to LKML
Lee,
Quoted 9 lines
Quoted 2 lines
This question is actually more about MFD. Can you point me to the possible causes for my failure below?
It's hard to tell exactly without code, but it looks like you're trying to allocate overlapping memory regions. Double check all of your addresses. For DT you need to take a look at your 'reg' properties, for traditional platform data it's best to grep for IORESOURCE_MEM.
Lee,
It _is_ overlapping. How could it not be? The resource for the I2C is mapped to BAR2. So the resource is overlapping with BAR2. It is alway the case, isn't it? What I don't understand is how MFD works with the resources if it is guaranteed overlapping. Did I get something wrong?
Look at the reference code I took, drivers/mfd/timberdale.c, when mfd_add_devices() is called, it uses &dev->resource as the base. So the BAR will be the parent. Check the code in mfd-core.c, mfd_add_device(),

Quoted 8 lines

if ((cell->resources[r].flags & IORESOURCE_MEM) && mem_base) {
	res[r].parent = mem_base;
	res[r].start = mem_base->start + cell->resources[r].start;
	res[r].end = mem_base->start + cell->resources[r].end;
}
So the MFD resource is within its parent. When later the device driver request a region, will it get conflict with the parent?

I doubt you'll want to map the same memory area in both the parent and the child device drivers. Only map the registers you plan to use in the driver you plan to use them. If you need multiple devices to access the same registers then you need to create an API, complete with locking, in the MFD parent device.

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

York Sun

2015-04-20 18:31 UTC #5 lore raw

Quoted 37 lines · Lee Jones

Quoted 35 lines · York Sun
Quoted 6 lines · Lee Jones
Quoted 4 lines · York Sun
Resend to LKML
Lee,
Quoted 9 lines
Quoted 2 lines
This question is actually more about MFD. Can you point me to the possible causes for my failure below?
It's hard to tell exactly without code, but it looks like you're trying to allocate overlapping memory regions. Double check all of your addresses. For DT you need to take a look at your 'reg' properties, for traditional platform data it's best to grep for IORESOURCE_MEM.
Lee,
It _is_ overlapping. How could it not be? The resource for the I2C is mapped to BAR2. So the resource is overlapping with BAR2. It is alway the case, isn't it? What I don't understand is how MFD works with the resources if it is guaranteed overlapping. Did I get something wrong?
Look at the reference code I took, drivers/mfd/timberdale.c, when mfd_add_devices() is called, it uses &dev->resource as the base. So the BAR will be the parent. Check the code in mfd-core.c, mfd_add_device(),
if ((cell->resources[r].flags & IORESOURCE_MEM) && mem_base) {
	res[r].parent = mem_base;
	res[r].start = mem_base->start + cell->resources[r].start;
	res[r].end = mem_base->start + cell->resources[r].end;
}

Quoted 9 lines

Quoted 2 lines
So the MFD resource is within its parent. When later the device driver request a region, will it get conflict with the parent?
I doubt you'll want to map the same memory area in both the parent and the child device drivers. Only map the registers you plan to use in the driver you plan to use them. If you need multiple devices to access the same registers then you need to create an API, complete with locking, in the MFD parent device.

Thanks for pointing out. I thought at first the conflict was due to my pci_ioremap_bar(). I went ahead to remove the mapping but still not working. Your email inspired me to take a deeper look at my code and I found my error. I have called pci_request_regions() which reserves all BARs. I think that's my root cause. Thanks for helping me.

York

Lee Jones

2015-04-21 07:35 UTC #6 lore raw

Quoted 18 lines

Quoted 9 lines
Quoted 9 lines
mfd_add_devices() is called, it uses &dev->resource as the base. So the BAR will be the parent. Check the code in mfd-core.c, mfd_add_device(),
if ((cell->resources[r].flags & IORESOURCE_MEM) && mem_base) {
	res[r].parent = mem_base;
	res[r].start = mem_base->start + cell->resources[r].start;
	res[r].end = mem_base->start + cell->resources[r].end;
}
Quoted 8 lines
Quoted 2 lines
So the MFD resource is within its parent. When later the device driver request a region, will it get conflict with the parent?
I doubt you'll want to map the same memory area in both the parent and the child device drivers. Only map the registers you plan to use in the driver you plan to use them. If you need multiple devices to access the same registers then you need to create an API, complete with locking, in the MFD parent device.

Quoted 5 lines

Thanks for pointing out. I thought at first the conflict was due to my pci_ioremap_bar(). I went ahead to remove the mapping but still not working. Your email inspired me to take a deeper look at my code and I found my error. I have called pci_request_regions() which reserves all BARs. I think that's my root cause. Thanks for helping me.

You are welcome.

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

York Sun

2015-05-06 21:41 UTC #7 lore raw

Why isn't IRQ shared for i2c-ocore

Lee,

I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?

I understand the performance concern. If there is any other reason, I want to know about it before I go too far on my driver.

York

Lee Jones

2015-05-07 07:01 UTC #8 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 2 lines

I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?

Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.

Quoted 2 lines

I understand the performance concern. If there is any other reason, I want to know about it before I go too far on my driver.

This sort of stuff is trivial to fix and shouldn't stand in the way of you writing and submitting v1 of your driver.

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

York Sun

2015-05-07 14:46 UTC #9 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 7 lines · Lee Jones

Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.

I am working on a driver for multifunction card with open core I2C controller. The interrupt is shared between I2C and other controller.

Quoted 7 lines

Quoted 3 lines
I understand the performance concern. If there is any other reason, I want to know about it before I go too far on my driver.
This sort of stuff is trivial to fix and shouldn't stand in the way of you writing and submitting v1 of your driver.

It is trivial to fix. I just want to know if there is any reason the interrupt wasn't shared. Seems not.

York

Geert Uytterhoeven

2015-05-08 06:47 UTC #10 lore raw

Re: Why isn't IRQ shared for i2c-ocore

In reply to Lee Jones

Quoted 7 lines · Lee Jones

Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.

However, that's something the driver can't know.
Sharing interrupts is an integration property. The same IP core may share its interrupt on one SoC, and not on another.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Lee Jones

2015-05-08 06:57 UTC #11 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 4 lines · Geert Uytterhoeven

Quoted 3 lines · Lee Jones
Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?

Quoted 8 lines

Quoted 4 lines
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.
However, that's something the driver can't know.
Sharing interrupts is an integration property. The same IP core may share its interrupt on one SoC, and not on another.

I guess that would depend on the IP. If this is part of an MFD, you'd know if you only hand a single interrupt line coming into the chip or not. If the IP can be moved around (copy & pasted) into different chips, then yes, that might change.

How does one share an interrupt with other drivers if all them don't know the IRQ is shared thought?

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Geert Uytterhoeven

2015-05-08 07:07 UTC #12 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 15 lines · Lee Jones

Quoted 4 lines · Geert Uytterhoeven
Quoted 3 lines · Lee Jones
Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?
Quoted 8 lines
Quoted 4 lines
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.
However, that's something the driver can't know.
Sharing interrupts is an integration property. The same IP core may share its interrupt on one SoC, and not on another.

Quoted 7 lines

I guess that would depend on the IP. If this is part of an MFD, you'd know if you only hand a single interrupt line coming into the chip or not. If the IP can be moved around (copy & pasted) into different chips, then yes, that might change.
How does one share an interrupt with other drivers if all them don't know the IRQ is shared thought?

All drivers sharing the same interrupt must pass IRQF_SHARED, cfr. the checks in __setup_irq().

Traditionally, PC (ISA) drivers didn't share interrupts, as the ISA bus prohibited interrupt sharing.
Amiga drivers did.

New drivers should support IRQ sharing.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Lee Jones

2015-05-08 07:27 UTC #13 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 24 lines · Geert Uytterhoeven

Quoted 15 lines · Lee Jones
Quoted 4 lines · Geert Uytterhoeven
Quoted 3 lines · Lee Jones
Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?
Quoted 8 lines
Quoted 4 lines
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.
However, that's something the driver can't know.
Sharing interrupts is an integration property. The same IP core may share its interrupt on one SoC, and not on another.
Quoted 7 lines
I guess that would depend on the IP. If this is part of an MFD, you'd know if you only hand a single interrupt line coming into the chip or not. If the IP can be moved around (copy & pasted) into different chips, then yes, that might change.
How does one share an interrupt with other drivers if all them don't know the IRQ is shared thought?

Quoted 8 lines

All drivers sharing the same interrupt must pass IRQF_SHARED, cfr. the checks in __setup_irq().
Traditionally, PC (ISA) drivers didn't share interrupts, as the ISA bus prohibited interrupt sharing.
Amiga drivers did.
New drivers should support IRQ sharing.

Precisely, which is why I'm confused by:

"However, that's something the driver can't know."

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Geert Uytterhoeven

2015-05-08 08:03 UTC #14 lore raw

Re: Why isn't IRQ shared for i2c-ocore

Quoted 31 lines · Lee Jones

Quoted 30 lines · Geert Uytterhoeven
Quoted 14 lines · Lee Jones
Quoted 4 lines · Geert Uytterhoeven
Quoted 3 lines · Lee Jones
Quoted 2 lines
I have a follow up question regarding interrupt. I see many I2C bus drivers request interrupt with flag = 0. Why not using IRQF_SHARED?
Quoted 8 lines
Quoted 4 lines
Probably because that particular IRQ is only used by the I2C Controller. I'm not exactly sure that you're getting at? Why do you think it should be shared? You should only flag it as shared if it is.
However, that's something the driver can't know.
Sharing interrupts is an integration property. The same IP core may share its interrupt on one SoC, and not on another.
Quoted 7 lines
I guess that would depend on the IP. If this is part of an MFD, you'd know if you only hand a single interrupt line coming into the chip or not. If the IP can be moved around (copy & pasted) into different chips, then yes, that might change.
How does one share an interrupt with other drivers if all them don't know the IRQ is shared thought?
All drivers sharing the same interrupt must pass IRQF_SHARED, cfr. the checks in __setup_irq().
Traditionally, PC (ISA) drivers didn't share interrupts, as the ISA bus prohibited interrupt sharing.
Amiga drivers did.

Quoted 5 lines

Quoted 1 line
New drivers should support IRQ sharing.
Precisely, which is why I'm confused by:
"However, that's something the driver can't know."

So better safe than sorry: always use IRQF_SHARED, unless you have a very good reason not to use it.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds