Re: [PATCH] omap3-iommu: reorganize
On Sun, Sep 13, 2009 at 11:19 PM, Felipe Contreras wrote: > This way it's more object oriented and easier to see what is happening. > No functional changes. @@ -16,7 +16,7 @@ Sorry, I forgot to address some comments from Russell from a previous version. I'll resend. struct iommu_device { resource_size_t base; - resource_size_t irq; + int irq; struct iommu_platform_data pdata; struct resource res[2]; }; @@ -67,7 +67,7 @@ static int __init omap3_iommu_init(void) res[0].start = d->base; res[0].end = d->base + MMU_REG_SIZE - 1; - res[1].start = d->irq; + res[1].start = res[1].end = d->irq; err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res)); -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3-iommu: reorganize
On Tue, May 19, 2009 at 9:03 AM, Hiroshi DOYU wrote: > From: ext Felipe Contreras > Subject: [PATCH] omap3-iommu: reorganize > Date: Mon, 18 May 2009 18:43:00 +0200 > >> No functional changes. >> >> Signed-off-by: Felipe Contreras >> --- >> >> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3-iommu: reorganize
From: ext Felipe Contreras Subject: [PATCH] omap3-iommu: reorganize Date: Mon, 18 May 2009 18:43:00 +0200 > No functional changes. > > Signed-off-by: Felipe Contreras > --- > > How about this? What's the advantage of introducing a new structure here? > > arch/arm/mach-omap2/omap3-iommu.c | 53 > ++--- > 1 files changed, 26 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap3-iommu.c > b/arch/arm/mach-omap2/omap3-iommu.c > index 91ee38a..50bd445 100644 > --- a/arch/arm/mach-omap2/omap3-iommu.c > +++ b/arch/arm/mach-omap2/omap3-iommu.c > @@ -14,32 +14,30 @@ > > #include > > -#define OMAP3_MMU1_BASE 0x480bd400 > -#define OMAP3_MMU2_BASE 0x5d00 > -#define OMAP3_MMU1_IRQ 24 > -#define OMAP3_MMU2_IRQ 28 > - > - > -static unsigned long iommu_base[] __initdata = { > - OMAP3_MMU1_BASE, > - OMAP3_MMU2_BASE, > -}; > - > -static int iommu_irq[] __initdata = { > - OMAP3_MMU1_IRQ, > - OMAP3_MMU2_IRQ, > +struct iommu_device { > + resource_size_t base; > + int irq; > + struct iommu_platform_data pdata; > }; > > -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = { > +static struct iommu_device devices[] __initdata = { > { > - .name = "isp", > - .nr_tlb_entries = 8, > - .clk_name = "cam_ick", > + .base = 0x480bd400, > + .irq = 24, > + .pdata = { > + .name = "isp", > + .nr_tlb_entries = 8, > + .clk_name = "cam_ick", > + }, > }, > { > - .name = "iva2", > - .nr_tlb_entries = 32, > - .clk_name = "iva2_ck", > + .base = 0x5d00, > + .irq = 28, > + .pdata = { > + .name = "iva2", > + .nr_tlb_entries = 32, > + .clk_name = "iva2_ck", > + }, > }, > }; > #define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata) > @@ -52,6 +50,7 @@ static int __init omap3_iommu_init(void) > > for (i = 0; i < NR_IOMMU_DEVICES; i++) { > struct platform_device *pdev; > + const struct iommu_device *d = &devices[i]; > struct resource res[2]; > > pdev = platform_device_alloc("omap-iommu", i); > @@ -60,19 +59,19 @@ static int __init omap3_iommu_init(void) > goto err_out; > } > > - memset(res, 0, sizeof(res)); > - res[0].start = iommu_base[i]; > - res[0].end = iommu_base[i] + MMU_REG_SIZE - 1; > + memset(res, 0, sizeof(res)); > + res[0].start = d->base; > + res[0].end = d->base + MMU_REG_SIZE - 1; > res[0].flags = IORESOURCE_MEM; > - res[1].start = res[1].end = iommu_irq[i]; > + res[1].start = res[1].end = d->irq; > res[1].flags = IORESOURCE_IRQ; > > err = platform_device_add_resources(pdev, res, > ARRAY_SIZE(res)); > if (err) > goto err_out; > - err = platform_device_add_data(pdev, &omap3_iommu_pdata[i], > -sizeof(omap3_iommu_pdata[0])); > + err = platform_device_add_data(pdev, &d->pdata, > +sizeof(d->pdata)); > if (err) > goto err_out; > err = platform_device_add(pdev); > -- > 1.6.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] omap3-iommu: reorganize
No functional changes. Signed-off-by: Felipe Contreras --- How about this? arch/arm/mach-omap2/omap3-iommu.c | 53 ++--- 1 files changed, 26 insertions(+), 27 deletions(-) diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c index 91ee38a..50bd445 100644 --- a/arch/arm/mach-omap2/omap3-iommu.c +++ b/arch/arm/mach-omap2/omap3-iommu.c @@ -14,32 +14,30 @@ #include -#define OMAP3_MMU1_BASE0x480bd400 -#define OMAP3_MMU2_BASE0x5d00 -#define OMAP3_MMU1_IRQ 24 -#define OMAP3_MMU2_IRQ 28 - - -static unsigned long iommu_base[] __initdata = { - OMAP3_MMU1_BASE, - OMAP3_MMU2_BASE, -}; - -static int iommu_irq[] __initdata = { - OMAP3_MMU1_IRQ, - OMAP3_MMU2_IRQ, +struct iommu_device { + resource_size_t base; + int irq; + struct iommu_platform_data pdata; }; -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = { +static struct iommu_device devices[] __initdata = { { - .name = "isp", - .nr_tlb_entries = 8, - .clk_name = "cam_ick", + .base = 0x480bd400, + .irq = 24, + .pdata = { + .name = "isp", + .nr_tlb_entries = 8, + .clk_name = "cam_ick", + }, }, { - .name = "iva2", - .nr_tlb_entries = 32, - .clk_name = "iva2_ck", + .base = 0x5d00, + .irq = 28, + .pdata = { + .name = "iva2", + .nr_tlb_entries = 32, + .clk_name = "iva2_ck", + }, }, }; #define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata) @@ -52,6 +50,7 @@ static int __init omap3_iommu_init(void) for (i = 0; i < NR_IOMMU_DEVICES; i++) { struct platform_device *pdev; + const struct iommu_device *d = &devices[i]; struct resource res[2]; pdev = platform_device_alloc("omap-iommu", i); @@ -60,19 +59,19 @@ static int __init omap3_iommu_init(void) goto err_out; } - memset(res, 0, sizeof(res)); - res[0].start = iommu_base[i]; - res[0].end = iommu_base[i] + MMU_REG_SIZE - 1; + memset(res, 0, sizeof(res)); + res[0].start = d->base; + res[0].end = d->base + MMU_REG_SIZE - 1; res[0].flags = IORESOURCE_MEM; - res[1].start = res[1].end = iommu_irq[i]; + res[1].start = res[1].end = d->irq; res[1].flags = IORESOURCE_IRQ; err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res)); if (err) goto err_out; - err = platform_device_add_data(pdev, &omap3_iommu_pdata[i], - sizeof(omap3_iommu_pdata[0])); + err = platform_device_add_data(pdev, &d->pdata, + sizeof(d->pdata)); if (err) goto err_out; err = platform_device_add(pdev); -- 1.6.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html