On Fri, Dec 03, 2010 at 10:37:10AM +0900, Jassi Brar wrote:
> On Fri, Dec 3, 2010 at 2:39 AM, Sylwester Nawrocki
> <s.nawrocki@xxxxxxxxxxx> wrote:
> >
> > Hi Jamie,
> >
> > On 12/02/2010 06:15 PM, Jamie Iles wrote:
> >> Hi Sylwester,
> >>
> >> On Thu, Dec 02, 2010 at 05:37:41PM +0100, Sylwester Nawrocki wrote:
> >>> diff --git a/arch/arm/plat-s5p/dev-csis0.c b/arch/arm/plat-s5p/dev-csis0.c
> >>> new file mode 100644
> >>> index 0000000..2b1ba43
> >>> --- /dev/null
> >>> +++ b/arch/arm/plat-s5p/dev-csis0.c
> >>> @@ -0,0 +1,34 @@
> >>> +/*
> >>> + * Copyright (C) 2010 Samsung Electronics
> >>> + *
> >>> + * S5P series device definition for MIPI-CSI device 0
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or modify
> >>> + * it under the terms of the GNU General Public License version 2 as
> >>> + * published by the Free Software Foundation.
> >>> +*/
> >>> +
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/interrupt.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <mach/map.h>
> >>> +
> >>> +static struct resource s5p_csis_resource[] = {
> >>> + [0] = {
> >>> + .start = S5P_PA_CSIS0,
> >>> + .end = S5P_PA_CSIS0 + SZ_4K - 1,
> >>> + .flags = IORESOURCE_MEM,
> >>> + },
> >>> + [1] = {
> >>> + .start = IRQ_MIPICSI0,
> >>> + .end = IRQ_MIPICSI0,
> >>> + .flags = IORESOURCE_IRQ,
> >>> + }
> >>> +};
> >> Do you really need the [0] and [1] here? These are only needed if you are
> >> sparsely initialising an array.
> >
> > I agree explicit indices are not really needed. I just followed the style of
> > all other resource definitions already there.
> > I am not quite sure what is the preference of the maintainer, I guess they want
> > all devices to follow same style for consistence.
>
> And that style has its origin because explicit indices are as because
> they stress the fact that
> drivers expect a particular resource at a particular index (when we
> have more than 1
> resource of a type).
> IMO we should use some macro shared between platform devices and their drivers.
> Something like -
> static struct resource s5p_csis_resource[] = {
> [PA_CSIS] = {
> .start = S5P_PA_CSIS0,
> .end = S5P_PA_CSIS0 + SZ_4K - 1,
> .flags = IORESOURCE_MEM,
> },
> [IRQ_CSIS] = {
> .start = IRQ_MIPICSI0,
> .end = IRQ_MIPICSI0,
> .flags = IORESOURCE_IRQ,
> }
> };
Or you could use the .name field of struct resource and use
platform_get_resource_byname() in the driver. The disadvantage would be that
this takes up a few more bytes though.
Jamie
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
