mailbox_mach dependency problem

mailbox_mach dependency problem

[Felipe Contreras]

Hi,

There seems to be a problem with 'mailbox' and 'mailbox_mach' modules.
When another module, like bridgedriver, uses mailbox stuff, only the
'mailbox' module is loaded which doesn't register the platform device.

The easiest solution would be to make mailbox_mach built-in when
mailbox is configured, however, mailbox_mach needs mailbox to be
loaded.

I tried to add a poke() function in mailbox_mach so that the module is
loaded when mailbox is loaded, but apparently linux doesn't like loop
dependencies:
WARNING: Loop detected: kernel/arch/arm/mach-omap2/mailbox_mach.ko
needs mailbox.ko which needs mailbox_mach.ko again!

Any ideas how to solve this issue?

-- 
Felipe Contreras

Re: mailbox_mach dependency problem

[Felipe Balbi]

Hi,

On Sat, May 01, 2010 at 05:47:57PM +0300, Felipe Contreras wrote:
> There seems to be a problem with 'mailbox' and 'mailbox_mach' modules.
> When another module, like bridgedriver, uses mailbox stuff, only the
> 'mailbox' module is loaded which doesn't register the platform device.
> 
> The easiest solution would be to make mailbox_mach built-in when
> mailbox is configured, however, mailbox_mach needs mailbox to be
> loaded.
> 
> I tried to add a poke() function in mailbox_mach so that the module is
> loaded when mailbox is loaded, but apparently linux doesn't like loop
> dependencies:
> WARNING: Loop detected: kernel/arch/arm/mach-omap2/mailbox_mach.ko
> needs mailbox.ko which needs mailbox_mach.ko again!
> 
> Any ideas how to solve this issue?

generally platform_device instantiation isn't compiled as a module. The
patch below should probably help but I didn't test it.

diff --git a/arch/arm/mach-omap1/Makefile b/arch/arm/mach-omap1/Makefile
index b6a537c..39a9e9e 100644
--- a/arch/arm/mach-omap1/Makefile
+++ b/arch/arm/mach-omap1/Makefile
@@ -15,8 +15,7 @@ obj-$(CONFIG_OMAP_32K_TIMER)	+= timer32k.o
 obj-$(CONFIG_PM) += pm.o sleep.o
 
 # DSP
-obj-$(CONFIG_OMAP_MBOX_FWK)	+= mailbox_mach.o
-mailbox_mach-objs		:= mailbox.o
+obj-$(CONFIG_OMAP_MBOX_OMAP1)	+= mailbox.o
 
 i2c-omap-$(CONFIG_I2C_OMAP)		:= i2c.o
 obj-y					+= $(i2c-omap-m) $(i2c-omap-y)
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 4b9fc57..6813a60 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -86,8 +86,7 @@ obj-$(CONFIG_ARCH_OMAP3)		+= omap_hwmod_3xxx_data.o
 # EMU peripherals
 obj-$(CONFIG_OMAP3_EMU)			+= emu.o
 
-obj-$(CONFIG_OMAP_MBOX_FWK)		+= mailbox_mach.o
-mailbox_mach-objs			:= mailbox.o
+obj-$(CONFIG_OMAP_MBOX_OMAP2)		+= mailbox.o
 
 iommu-y					+= iommu2.o
 iommu-$(CONFIG_ARCH_OMAP3)		+= omap3-iommu.o
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..1c826c4 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -102,10 +102,18 @@ config OMAP_MCBSP
 config OMAP_MBOX_FWK
 	tristate "Mailbox framework support"
 	depends on ARCH_OMAP
+	select OMAP_MBOX_OMAP2 if ARCH_OMAP2PLUS
+	select OMAP_MBOX_OMAP1 if ARCH_OMAP1
 	help
 	  Say Y here if you want to use OMAP Mailbox framework support for
 	  DSP, IVA1.0 and IVA2 in OMAP1/2/3.
 
+config OMAP_MBOX_OMAP1
+	bool
+
+config OMAP_MBOX_OMAP2
+	bool
+
 config OMAP_IOMMU
 	tristate
 

-- 
balbi

Re: mailbox_mach dependency problem

[Felipe Contreras]

On Sat, May 1, 2010 at 8:33 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> On Sat, May 01, 2010 at 05:47:57PM +0300, Felipe Contreras wrote:
>> The easiest solution would be to make mailbox_mach built-in when
>> mailbox is configured, however, mailbox_mach needs mailbox to be
>> loaded.
>
> generally platform_device instantiation isn't compiled as a module. The
> patch below should probably help but I didn't test it.

Yeah, but as I said above... that doesn't work:

arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_probe':
/data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:389:
undefined reference to `omap_mbox_register'
arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_remove':
/data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:439:
undefined reference to `omap_mbox_unregister'

-- 
Felipe Contreras

Re: mailbox_mach dependency problem

[Felipe Balbi]

On Sun, May 02, 2010 at 02:08:58AM +0300, Felipe Contreras wrote:
> On Sat, May 1, 2010 at 8:33 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> > On Sat, May 01, 2010 at 05:47:57PM +0300, Felipe Contreras wrote:
> >> The easiest solution would be to make mailbox_mach built-in when
> >> mailbox is configured, however, mailbox_mach needs mailbox to be
> >> loaded.
> >
> > generally platform_device instantiation isn't compiled as a module. The
> > patch below should probably help but I didn't test it.
> 
> Yeah, but as I said above... that doesn't work:
> 
> arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_probe':
> /data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:389:
> undefined reference to `omap_mbox_register'
> arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_remove':
> /data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:439:
> undefined reference to `omap_mbox_unregister'

now I see the problem. You shouldn't put that kind of stuff on the
driver. The driver can be built as a module but the device will probably
be built-in. You'll need a more intrusive change to fix that, I guess.

-- 
balbi

Re: mailbox_mach dependency problem

[Felipe Contreras]

On Sun, May 2, 2010 at 10:13 AM, Felipe Balbi <me@felipebalbi.com> wrote:
> On Sun, May 02, 2010 at 02:08:58AM +0300, Felipe Contreras wrote:
>> On Sat, May 1, 2010 at 8:33 PM, Felipe Balbi <me@felipebalbi.com> wrote:
>> > On Sat, May 01, 2010 at 05:47:57PM +0300, Felipe Contreras wrote:
>> >> The easiest solution would be to make mailbox_mach built-in when
>> >> mailbox is configured, however, mailbox_mach needs mailbox to be
>> >> loaded.
>> >
>> > generally platform_device instantiation isn't compiled as a module. The
>> > patch below should probably help but I didn't test it.
>>
>> Yeah, but as I said above... that doesn't work:
>>
>> arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_probe':
>> /data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:389:
>> undefined reference to `omap_mbox_register'
>> arch/arm/mach-omap2/built-in.o: In function `omap2_mbox_remove':
>> /data/public/dev/omap/linux-omap/arch/arm/mach-omap2/mailbox.c:439:
>> undefined reference to `omap_mbox_unregister'
>
> now I see the problem. You shouldn't put that kind of stuff on the
> driver. The driver can be built as a module but the device will probably
> be built-in. You'll need a more intrusive change to fix that, I guess.

Indeed, that was my understanding, but the mailbox doesn't seem to be
designed that way. Apparently the driver has an internal linked list
of mailboxes, and the platform device registers a bunch of them.
Effectively they need each other.

-- 
Felipe Contreras

Re: mailbox_mach dependency problem

[Felipe Balbi]

Hi,

On Sun, May 02, 2010 at 03:57:42PM +0300, Felipe Contreras wrote:
> Indeed, that was my understanding, but the mailbox doesn't seem to be
> designed that way. Apparently the driver has an internal linked list
> of mailboxes, and the platform device registers a bunch of them.
> Effectively they need each other.

you could make it the other way around. For each registered
platform_device you instantiate one mailbox and add it to the list on
the fly, you would probably like to use a list_head for that also
instead of having your own list implementation.

if you do that, you leave the mbox "instantiation" up to the
platform_device code. You would simply register several omap-mbox with a
different id for each mbox you need. Then the name could be sent using a
well-defined platform_data. Just a suggestion.

-- 
balbi

Re: mailbox_mach dependency problem

[Felipe Contreras]

On Sun, May 2, 2010 at 6:59 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> On Sun, May 02, 2010 at 03:57:42PM +0300, Felipe Contreras wrote:
>> Indeed, that was my understanding, but the mailbox doesn't seem to be
>> designed that way. Apparently the driver has an internal linked list
>> of mailboxes, and the platform device registers a bunch of them.
>> Effectively they need each other.
>
> you could make it the other way around. For each registered
> platform_device you instantiate one mailbox and add it to the list on
> the fly, you would probably like to use a list_head for that also
> instead of having your own list implementation.
>
> if you do that, you leave the mbox "instantiation" up to the
> platform_device code. You would simply register several omap-mbox with a
> different id for each mbox you need. Then the name could be sent using a
> well-defined platform_data. Just a suggestion.

First, this is not my code (sounds like you are assuming so).

Now, I thought on doing something like what you suggest, but since I
haven't done anything like it, I thought on asking for ideas just so I
don't end up doing something stupid.

Making multiple platform_device sounds appealing, but the problem is
that there's no 1-to-1 mapping from resources to mailboxes. For
example, all mailboxes seem to share the same memory, and some
mailboxes use the same irq. I don't know if multiple platform_devices
can share the same resources.

What I ended up doing is store all the mailboxes in the platform data.
I didn't know how to store an array of pointers in the pdata, so I
created my own structure for that:
http://article.gmane.org/gmane.linux.ports.arm.omap/34916

You can check the whole patch series here:
http://thread.gmane.org/gmane.linux.ports.arm.omap/34912

Thanks for the comments. Cheers.

-- 
Felipe Contreras