On Mon, May 18, 2009 at 3:07 PM, Russell King - ARM Linux
<li...@arm.linux.org.uk> wrote:
> On Sat, May 16, 2009 at 01:05:48PM +0300, Felipe Contreras wrote:
>> +struct iommu_device {
>> + resource_size_t base;
>> + resource_size_t irq;
>> + struct iommu_platform_data pdata;
>> + struct resource res[2];
>> };
>
> The data which is needed per device is:
>
> - base address
> - IRQ (no need for this to be resource_size_t - int will do)
> - platform data
>
> There's no need for that res[2] being there.
Right, I was using 'res' but not any more; I forgot to remove it.
So resource_size_t is ok for 'base'?
>> @@ -63,23 +51,35 @@ 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[] = {
>> + { .flags = IORESOURCE_MEM },
>> + { .flags = IORESOURCE_IRQ },
>> + };
>
> This initialization doesn't buy you anything, in fact quite the opposite.
> The compiler actually interprets this as:
>
> static struct resource __initial_res[] = {
> { .flags = IORESOURCE_MEM },
> { .flags = IORESOURCE_IRQ },
> };
> ...
> for () {
> struct resource res[...];
> memcpy(res, __initial_res, sizeof(__initial_res));
>
>> +
>> + res[0].start = d->base;
>> + res[0].end = d->base + MMU_REG_SIZE - 1;
>> +
>> + res[1].start = d->irq;
>
> It would be far better to initialize the flags element here for both.
> And please also set res[1].end as I did in my patch.
I think I saw that res initialization somewhere and I found it much
easier to read.
Ok. Will do.
>> +
>> + 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));
>
> This will fail checkpatch.
I'll make sure it passes.
Cheers.
--
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
