Re: [PATCH] omap3-iommu: reorganize

[Felipe Contreras <felipe.contreras@gmail.com>]

On Tue, May 19, 2009 at 9:03 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [PATCH] omap3-iommu: reorganize
> Date: Mon, 18 May 2009 18:43:00 +0200
>
>> No functional changes.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> How about this?
>
> What's the advantage of introducing a new structure here?

That instead of having 3 arrays with potentially different sizes:

iommu_base
iommu_irq
omap3_iommu_pdata

You have one:

devices

Also, think about the hypothetical situation where you need 2 more
iommu devices (maybe OMAP4?). What would you need to do? You'll need
to add OMAP3_MMU3_BASE and OMAP3_MMU4_BASE add them to iommu_base,
then add OMAP3_MMU3_IRQ and OMAP3_MMU4_IRQ and put them to iommu_irq,
finally add the device data to omap3_iommu_pdata.

In the process a simple mistake would be easy to overlook:

static unsigned long iommu_base[] __initdata = {
       OMAP3_MMU1_BASE,
       OMAP3_MMU2_BASE,
+       OMAP3_MMU3_BASE,
+       OMAP3_MMU2_BASE,
};

With the 'devices' array you just need to add two more elements and
that's it. It leaves less room for mistakes.

However 'struct iommu_device' is a local structure, it could very well
be called __foobar__, nobody outside omap3-iommu.c will see it anyway.

-- 
Felipe Contreras