Re: [PATCH 3/6] omap iommu: omap3 iommu device registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 8, 2009 at 8:21 PM, Aguirre Rodriguez, Sergio Alberto
<saaguirre@xxxxxx> wrote:
> Hi,
>
> Just one comment below:
>
>> > Does the following make sense?
>> >
>> > +#define NR_IOMMU_RES 2
>> >
>> > ....
>> >
>> > +               err = platform_device_add_resources(pdev,
>> > +                                   omap3_iommu_res +  i * NR_IOMMU_RES,
>> NR_IOMMU_RES);
>>
>> Yeap, also:
>>
>> > +               err = platform_device_add_resources(pdev,
>> omap3_iommu_res +  i * 2, 2);
>
> IMHO, I don't think it's a good idea to add magical numbers to any code in the kernel. Why is it better to NOT use a define?

I agree, but even a define is not good enough. For example you can
update the array without updating the define.

A better solution is to do what I did on my follow up patch series:

+               struct resource res[] = {
+                       { .flags = IORESOURCE_MEM },
+                       { .flags = IORESOURCE_IRQ },
+               };

+               err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));

No need for a define, and as soon as the array is updated so will the
second argument sent to platform_device_add_resources.

-- 
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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux