On Tue, May 19, 2009 at 9:03 AM, Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx> wrote:
> From: ext Felipe Contreras <felipe.contreras@xxxxxxxxx>
> Subject: [PATCH] omap3-iommu: reorganize
> Date: Mon, 18 May 2009 18:43:00 +0200
>
>> No functional changes.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
>> ---
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
