Re: [RFC/PATCH 1/3] omap3-iommu: reorganize

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

On Fri, May 8, 2009 at 12:14 AM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Thu, May 07, 2009 at 10:11:07PM +0200, ext Felipe Contreras wrote:
>> -static struct resource omap3_iommu_res[] = {
>> -     { /* Camera ISP MMU */
>> -             .start          = OMAP3_MMU1_BASE,
>> -             .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> -             .flags          = IORESOURCE_MEM,
>> -     },
>> -     {
>> -             .start          = OMAP3_MMU1_IRQ,
>> -             .flags          = IORESOURCE_IRQ,
>> -     },
>> -     { /* IVA2.2 MMU */
>> -             .start          = OMAP3_MMU2_BASE,
>> -             .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> -             .flags          = IORESOURCE_MEM,
>> -     },
>> -     {
>> -             .start          = OMAP3_MMU2_IRQ,
>> -             .flags          = IORESOURCE_IRQ,
>> -     },
>
> i find this one much more readable :-s

You find an array of 'struct resource' more readable than this?

+               .base = 0x480bd400,
+               .irq = 24,

+               .base = 0x5d000000,
+               .irq = 28,

+               res[0].start = d->base;
+               res[0].end = d->base + MMU_REG_SIZE - 1;
+
+               res[1].start = d->irq;

How would you calculate the number of resources per iommu device?

How about these?

-               err = platform_device_add_resources(pdev,
-                                   &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+               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;

Do you find the original version easier to read?

>> +struct iommu_device {
>> +     resource_size_t base;
>> +     resource_size_t irq;
>> +     struct iommu_platform_data pdata;
>
> generally, platform_data is a void *. Don't know if it matters here.

Yes but we need to set the actual contents for platform_device_add_data.

>> +     struct resource res[2];
>>  };
>> -#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>>
>> -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
>> +static struct iommu_device devices[] = {
>>       {
>> -             .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",
>
> still passing clk names ? what about clkdev ?

That's for iommu to fix. I'm simply interested on decoupling iommu and
a hypothetical tidspbridge that doesn't exists yet.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html