linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
@ 2013-08-07 16:19 Laurent Pinchart
  2013-08-07 17:20 ` Sergei Shtylyov
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Laurent Pinchart @ 2013-08-07 16:19 UTC (permalink / raw)
  To: linux-sh

Add a function to register the DU device with board-specific platform
data. The DU device is named rcar-du-r8a7779, fix the clock entry
accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mach-shmobile/clock-r8a7779.c        |  2 +-
 arch/arm/mach-shmobile/include/mach/r8a7779.h |  2 ++
 arch/arm/mach-shmobile/setup-r8a7779.c        | 22 ++++++++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/clock-r8a7779.c b/arch/arm/mach-shmobile/clock-r8a7779.c
index 10340f5..21e6364 100644
--- a/arch/arm/mach-shmobile/clock-r8a7779.c
+++ b/arch/arm/mach-shmobile/clock-r8a7779.c
@@ -190,7 +190,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh_mobile_sdhi.1", &mstp_clks[MSTP322]), /* SDHI1 */
 	CLKDEV_DEV_ID("sh_mobile_sdhi.2", &mstp_clks[MSTP321]), /* SDHI2 */
 	CLKDEV_DEV_ID("sh_mobile_sdhi.3", &mstp_clks[MSTP320]), /* SDHI3 */
-	CLKDEV_DEV_ID("rcar-du.0", &mstp_clks[MSTP103]), /* DU */
+	CLKDEV_DEV_ID("rcar-du-r8a7779", &mstp_clks[MSTP103]), /* DU */
 };
 
 void __init r8a7779_clock_init(void)
diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h b/arch/arm/mach-shmobile/include/mach/r8a7779.h
index f08d6ec..83c47a2 100644
--- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
+++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
@@ -6,6 +6,7 @@
 #include <linux/sh_eth.h>
 
 struct platform_device;
+struct rcar_du_platform_data;
 
 struct r8a7779_pm_ch {
 	unsigned long chan_offs;
@@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
 extern void r8a7779_add_early_devices(void);
 extern void r8a7779_add_standard_devices(void);
 extern void r8a7779_add_standard_devices_dt(void);
+extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
 extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
 extern void r8a7779_init_late(void);
 extern void r8a7779_clock_init(void);
diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
index 6a99333..c587fc9 100644
--- a/arch/arm/mach-shmobile/setup-r8a7779.c
+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
@@ -27,6 +27,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_data/gpio-rcar.h>
 #include <linux/platform_data/irq-renesas-intc-irqpin.h>
+#include <linux/platform_data/rcar-du.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/input.h>
@@ -605,6 +606,12 @@ static struct resource ether_resources[] __initdata = {
 	},
 };
 
+/* DU */
+static struct resource du_resources[] = {
+	DEFINE_RES_MEM(0xfff80000, 0x40000),
+	DEFINE_RES_IRQ(gic_iid(0x3f)),
+};
+
 static struct platform_device *r8a7779_devices_dt[] __initdata = {
 	&scif0_device,
 	&scif1_device,
@@ -640,6 +647,21 @@ void __init r8a7779_add_standard_devices(void)
 			    ARRAY_SIZE(r8a7779_standard_devices));
 }
 
+void __init r8a7779_add_du_device(struct rcar_du_platform_data *pdata)
+{
+	struct platform_device_info info = {
+		.name = "rcar-du-r8a7779",
+		.id = -1,
+		.res = du_resources,
+		.num_res = ARRAY_SIZE(du_resources),
+		.data = pdata,
+		.size_data = sizeof(*pdata),
+		.dma_mask = DMA_BIT_MASK(32),
+	};
+
+	platform_device_register_full(&info);
+}
+
 void __init r8a7779_add_ether_device(struct sh_eth_plat_data *pdata)
 {
 	platform_device_register_resndata(&platform_bus, "r8a777x-ether", -1,
-- 
1.8.1.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
@ 2013-08-07 17:20 ` Sergei Shtylyov
  2013-08-07 22:19 ` Laurent Pinchart
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2013-08-07 17:20 UTC (permalink / raw)
  To: linux-sh

On 08/07/2013 08:19 PM, Laurent Pinchart wrote:

> Add a function to register the DU device with board-specific platform
> data. The DU device is named rcar-du-r8a7779, fix the clock entry
> accordingly.

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[...]

> diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> index f08d6ec..83c47a2 100644
> --- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> +++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> @@ -6,6 +6,7 @@
>   #include <linux/sh_eth.h>
>
>   struct platform_device;
> +struct rcar_du_platform_data;
>
>   struct r8a7779_pm_ch {
>   	unsigned long chan_offs;
> @@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
>   extern void r8a7779_add_early_devices(void);
>   extern void r8a7779_add_standard_devices(void);
>   extern void r8a7779_add_standard_devices_dt(void);
> +extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
>   extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
>   extern void r8a7779_init_late(void);
>   extern void r8a7779_clock_init(void);
> diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
> index 6a99333..c587fc9 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7779.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> @@ -27,6 +27,7 @@
>   #include <linux/of_platform.h>
>   #include <linux/platform_data/gpio-rcar.h>
>   #include <linux/platform_data/irq-renesas-intc-irqpin.h>
> +#include <linux/platform_data/rcar-du.h>

    Why not just include it in the r8a7779.h?

>   #include <linux/platform_device.h>
>   #include <linux/delay.h>
>   #include <linux/input.h>
> @@ -605,6 +606,12 @@ static struct resource ether_resources[] __initdata = {
>   	},
>   };
>
> +/* DU */
> +static struct resource du_resources[] = {

    Should be annotated with '__initdata'.

> +	DEFINE_RES_MEM(0xfff80000, 0x40000),
> +	DEFINE_RES_IRQ(gic_iid(0x3f)),
> +};
> +

WBR, Sergei



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
  2013-08-07 17:20 ` Sergei Shtylyov
@ 2013-08-07 22:19 ` Laurent Pinchart
  2013-08-07 22:42 ` Sergei Shtylyov
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2013-08-07 22:19 UTC (permalink / raw)
  To: linux-sh

Hi Sergei,

Thank you for the review.

On Wednesday 07 August 2013 21:20:54 Sergei Shtylyov wrote:
> On 08/07/2013 08:19 PM, Laurent Pinchart wrote:
> > Add a function to register the DU device with board-specific platform
> > data. The DU device is named rcar-du-r8a7779, fix the clock entry
> > accordingly.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> [...]
> 
> > diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > b/arch/arm/mach-shmobile/include/mach/r8a7779.h index f08d6ec..83c47a2
> > 100644
> > --- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > +++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > @@ -6,6 +6,7 @@
> >   #include <linux/sh_eth.h>
> >   
> >   struct platform_device;
> > +struct rcar_du_platform_data;
> > 
> >   struct r8a7779_pm_ch {
> >   	unsigned long chan_offs;
> > @@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
> >   extern void r8a7779_add_early_devices(void);
> >   extern void r8a7779_add_standard_devices(void);
> >   extern void r8a7779_add_standard_devices_dt(void);
> > +extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
> >   extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
> >   extern void r8a7779_init_late(void);
> >   extern void r8a7779_clock_init(void);
> > diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
> > b/arch/arm/mach-shmobile/setup-r8a7779.c index 6a99333..c587fc9 100644
> > --- a/arch/arm/mach-shmobile/setup-r8a7779.c
> > +++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> > @@ -27,6 +27,7 @@
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_data/gpio-rcar.h>
> >   #include <linux/platform_data/irq-renesas-intc-irqpin.h>
> > +#include <linux/platform_data/rcar-du.h>
> 
> Why not just include it in the r8a7779.h?

Because not all users of r8a7779.h needs the rcar-du.h header. Including the 
header only where it is needed speeds up compilation.

> >   #include <linux/platform_device.h>
> >   #include <linux/delay.h>
> >   #include <linux/input.h>
> > @@ -605,6 +606,12 @@ static struct resource ether_resources[] __initdata > > {> 
> >   	},
> >   };
> > 
> > +/* DU */
> > +static struct resource du_resources[] = {
> 
>     Should be annotated with '__initdata'.

Good point, I'll fix that and resubmit (same for the other patches in this 
series).

> > +	DEFINE_RES_MEM(0xfff80000, 0x40000),
> > +	DEFINE_RES_IRQ(gic_iid(0x3f)),
> > +};
> > +

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
  2013-08-07 17:20 ` Sergei Shtylyov
  2013-08-07 22:19 ` Laurent Pinchart
@ 2013-08-07 22:42 ` Sergei Shtylyov
  2013-08-08  0:12 ` Kuninori Morimoto
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2013-08-07 22:42 UTC (permalink / raw)
  To: linux-sh

On 08/08/2013 02:19 AM, Laurent Pinchart wrote:

>>> Add a function to register the DU device with board-specific platform
>>> data. The DU device is named rcar-du-r8a7779, fix the clock entry
>>> accordingly.

>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas@ideasonboard.com>

>> [...]

>>> diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h
>>> b/arch/arm/mach-shmobile/include/mach/r8a7779.h index f08d6ec..83c47a2
>>> 100644
>>> --- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
>>> +++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
>>> @@ -6,6 +6,7 @@
>>>    #include <linux/sh_eth.h>
>>>
>>>    struct platform_device;
>>> +struct rcar_du_platform_data;
>>>
>>>    struct r8a7779_pm_ch {
>>>    	unsigned long chan_offs;
>>> @@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
>>>    extern void r8a7779_add_early_devices(void);
>>>    extern void r8a7779_add_standard_devices(void);
>>>    extern void r8a7779_add_standard_devices_dt(void);
>>> +extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
>>>    extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
>>>    extern void r8a7779_init_late(void);
>>>    extern void r8a7779_clock_init(void);
>>> diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
>>> b/arch/arm/mach-shmobile/setup-r8a7779.c index 6a99333..c587fc9 100644
>>> --- a/arch/arm/mach-shmobile/setup-r8a7779.c
>>> +++ b/arch/arm/mach-shmobile/setup-r8a7779.c
>>> @@ -27,6 +27,7 @@
>>>    #include <linux/of_platform.h>
>>>    #include <linux/platform_data/gpio-rcar.h>
>>>    #include <linux/platform_data/irq-renesas-intc-irqpin.h>
>>> +#include <linux/platform_data/rcar-du.h>
>>
>> Why not just include it in the r8a7779.h?

> Because not all users of r8a7779.h needs the rcar-du.h header. Including the
> header only where it is needed speeds up compilation.

    I'm afraid the most important users (setup-* and board-*) need it. Anyway, 
Simon/Magnus will most probably ask you to remove the header change and rework 
the patch according to their taste.

WBR, Sergei


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
                   ` (2 preceding siblings ...)
  2013-08-07 22:42 ` Sergei Shtylyov
@ 2013-08-08  0:12 ` Kuninori Morimoto
  2013-08-08  0:24 ` Simon Horman
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2013-08-08  0:12 UTC (permalink / raw)
  To: linux-sh


Hi Laurent

> Add a function to register the DU device with board-specific platform
> data. The DU device is named rcar-du-r8a7779, fix the clock entry
> accordingly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
(snip)
> @@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
>  extern void r8a7779_add_early_devices(void);
>  extern void r8a7779_add_standard_devices(void);
>  extern void r8a7779_add_standard_devices_dt(void);
> +extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
>  extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
>  extern void r8a7779_init_late(void);
>  extern void r8a7779_clock_init(void);
(snip)
> +void __init r8a7779_add_du_device(struct rcar_du_platform_data *pdata)
> +{
> +	struct platform_device_info info = {
> +		.name = "rcar-du-r8a7779",
> +		.id = -1,
> +		.res = du_resources,
> +		.num_res = ARRAY_SIZE(du_resources),
> +		.data = pdata,
> +		.size_data = sizeof(*pdata),
> +		.dma_mask = DMA_BIT_MASK(32),
> +	};
> +
> +	platform_device_register_full(&info);
> +}

Thank you for your patch.
But, now, we don't use above style of driver registration
which goes back and forth between setup.c and board.c. 
(sh_eth is exceptional case now)

Please check below on SH-ARM ML
[PATCH 0/11] ARM: shmobile: setup cleanup / r8a7778 update

then, please call register function on board.c directly
if it needs platform data

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
                   ` (3 preceding siblings ...)
  2013-08-08  0:12 ` Kuninori Morimoto
@ 2013-08-08  0:24 ` Simon Horman
  2013-08-08  0:27 ` Laurent Pinchart
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2013-08-08  0:24 UTC (permalink / raw)
  To: linux-sh

On Thu, Aug 08, 2013 at 02:42:55AM +0400, Sergei Shtylyov wrote:
> On 08/08/2013 02:19 AM, Laurent Pinchart wrote:
> 
> >>>Add a function to register the DU device with board-specific platform
> >>>data. The DU device is named rcar-du-r8a7779, fix the clock entry
> >>>accordingly.
> 
> >>>Signed-off-by: Laurent Pinchart
> >>><laurent.pinchart+renesas@ideasonboard.com>
> 
> >>[...]
> 
> >>>diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> >>>b/arch/arm/mach-shmobile/include/mach/r8a7779.h index f08d6ec..83c47a2
> >>>100644
> >>>--- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> >>>+++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> >>>@@ -6,6 +6,7 @@
> >>>   #include <linux/sh_eth.h>
> >>>
> >>>   struct platform_device;
> >>>+struct rcar_du_platform_data;
> >>>
> >>>   struct r8a7779_pm_ch {
> >>>   	unsigned long chan_offs;
> >>>@@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
> >>>   extern void r8a7779_add_early_devices(void);
> >>>   extern void r8a7779_add_standard_devices(void);
> >>>   extern void r8a7779_add_standard_devices_dt(void);
> >>>+extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
> >>>   extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
> >>>   extern void r8a7779_init_late(void);
> >>>   extern void r8a7779_clock_init(void);
> >>>diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
> >>>b/arch/arm/mach-shmobile/setup-r8a7779.c index 6a99333..c587fc9 100644
> >>>--- a/arch/arm/mach-shmobile/setup-r8a7779.c
> >>>+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> >>>@@ -27,6 +27,7 @@
> >>>   #include <linux/of_platform.h>
> >>>   #include <linux/platform_data/gpio-rcar.h>
> >>>   #include <linux/platform_data/irq-renesas-intc-irqpin.h>
> >>>+#include <linux/platform_data/rcar-du.h>
> >>
> >>Why not just include it in the r8a7779.h?
> 
> >Because not all users of r8a7779.h needs the rcar-du.h header. Including the
> >header only where it is needed speeds up compilation.
> 
>    I'm afraid the most important users (setup-* and board-*) need
> it. Anyway, Simon/Magnus will most probably ask you to remove the
> header change and rework the patch according to their taste.

At the risk of being inconsistent with feedback for previous patches:
I have no strong opinion on this at this time. Though personally I would
have just added the #include in r8a7779.h.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
                   ` (4 preceding siblings ...)
  2013-08-08  0:24 ` Simon Horman
@ 2013-08-08  0:27 ` Laurent Pinchart
  2013-08-08  0:28 ` Laurent Pinchart
  2013-08-08  0:35 ` Simon Horman
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2013-08-08  0:27 UTC (permalink / raw)
  To: linux-sh

Hi Simon,

On Thursday 08 August 2013 09:24:19 Simon Horman wrote:
> On Thu, Aug 08, 2013 at 02:42:55AM +0400, Sergei Shtylyov wrote:
> > On 08/08/2013 02:19 AM, Laurent Pinchart wrote:
> > >>>Add a function to register the DU device with board-specific platform
> > >>>data. The DU device is named rcar-du-r8a7779, fix the clock entry
> > >>>accordingly.
> > >>>
> > >>>Signed-off-by: Laurent Pinchart
> > >>><laurent.pinchart+renesas@ideasonboard.com>
> > >>
> > >>[...]
> > >>
> > >>>diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > >>>b/arch/arm/mach-shmobile/include/mach/r8a7779.h index f08d6ec..83c47a2
> > >>>100644
> > >>>--- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > >>>+++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > >>>@@ -6,6 +6,7 @@
> > >>>
> > >>>   #include <linux/sh_eth.h>
> > >>>   
> > >>>   struct platform_device;
> > >>>
> > >>>+struct rcar_du_platform_data;
> > >>>
> > >>>   struct r8a7779_pm_ch {
> > >>>   
> > >>>   	unsigned long chan_offs;
> > >>>
> > >>>@@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
> > >>>
> > >>>   extern void r8a7779_add_early_devices(void);
> > >>>   extern void r8a7779_add_standard_devices(void);
> > >>>   extern void r8a7779_add_standard_devices_dt(void);
> > >>>
> > >>>+extern void r8a7779_add_du_device(struct rcar_du_platform_data
> > >>>*pdata);
> > >>>
> > >>>   extern void r8a7779_add_ether_device(struct sh_eth_plat_data
> > >>>   *pdata);
> > >>>   extern void r8a7779_init_late(void);
> > >>>   extern void r8a7779_clock_init(void);
> > >>>
> > >>>diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
> > >>>b/arch/arm/mach-shmobile/setup-r8a7779.c index 6a99333..c587fc9 100644
> > >>>--- a/arch/arm/mach-shmobile/setup-r8a7779.c
> > >>>+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> > >>>@@ -27,6 +27,7 @@
> > >>>
> > >>>   #include <linux/of_platform.h>
> > >>>   #include <linux/platform_data/gpio-rcar.h>
> > >>>   #include <linux/platform_data/irq-renesas-intc-irqpin.h>
> > >>>
> > >>>+#include <linux/platform_data/rcar-du.h>
> > >>
> > >>Why not just include it in the r8a7779.h?
> >>
> >> Because not all users of r8a7779.h needs the rcar-du.h header. Including
> >> the header only where it is needed speeds up compilation.
> >
> > I'm afraid the most important users (setup-* and board-*) need
> > it. Anyway, Simon/Magnus will most probably ask you to remove the
> > header change and rework the patch according to their taste.
> 
> At the risk of being inconsistent with feedback for previous patches:
> I have no strong opinion on this at this time. Though personally I would
> have just added the #include in r8a7779.h.

Magnus has asked me to move the setup-*.c code to the board files, I'll do so 
and will resubmit the patches.


-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
                   ` (5 preceding siblings ...)
  2013-08-08  0:27 ` Laurent Pinchart
@ 2013-08-08  0:28 ` Laurent Pinchart
  2013-08-08  0:35 ` Simon Horman
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2013-08-08  0:28 UTC (permalink / raw)
  To: linux-sh

Hi Morimoto-san,

On Wednesday 07 August 2013 17:12:01 Kuninori Morimoto wrote:
> Hi Laurent
> 
> > Add a function to register the DU device with board-specific platform
> > data. The DU device is named rcar-du-r8a7779, fix the clock entry
> > accordingly.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> 
> (snip)
> 
> > @@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
> > 
> >  extern void r8a7779_add_early_devices(void);
> >  extern void r8a7779_add_standard_devices(void);
> >  extern void r8a7779_add_standard_devices_dt(void);
> > 
> > +extern void r8a7779_add_du_device(struct rcar_du_platform_data *pdata);
> > 
> >  extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata);
> >  extern void r8a7779_init_late(void);
> >  extern void r8a7779_clock_init(void);
> 
> (snip)
> 
> > +void __init r8a7779_add_du_device(struct rcar_du_platform_data *pdata)
> > +{
> > +	struct platform_device_info info = {
> > +		.name = "rcar-du-r8a7779",
> > +		.id = -1,
> > +		.res = du_resources,
> > +		.num_res = ARRAY_SIZE(du_resources),
> > +		.data = pdata,
> > +		.size_data = sizeof(*pdata),
> > +		.dma_mask = DMA_BIT_MASK(32),
> > +	};
> > +
> > +	platform_device_register_full(&info);
> > +}
> 
> Thank you for your patch.
> But, now, we don't use above style of driver registration
> which goes back and forth between setup.c and board.c.
> (sh_eth is exceptional case now)
> 
> Please check below on SH-ARM ML
> [PATCH 0/11] ARM: shmobile: setup cleanup / r8a7778 update
> 
> then, please call register function on board.c directly
> if it needs platform data

Thank you for the review. I'll fix the code and submit a v3.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support
  2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
                   ` (6 preceding siblings ...)
  2013-08-08  0:28 ` Laurent Pinchart
@ 2013-08-08  0:35 ` Simon Horman
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2013-08-08  0:35 UTC (permalink / raw)
  To: linux-sh

On Thu, Aug 08, 2013 at 02:27:34AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Thursday 08 August 2013 09:24:19 Simon Horman wrote:
> > On Thu, Aug 08, 2013 at 02:42:55AM +0400, Sergei Shtylyov wrote:
> > > On 08/08/2013 02:19 AM, Laurent Pinchart wrote:
> > > >>>Add a function to register the DU device with board-specific platform
> > > >>>data. The DU device is named rcar-du-r8a7779, fix the clock entry
> > > >>>accordingly.
> > > >>>
> > > >>>Signed-off-by: Laurent Pinchart
> > > >>><laurent.pinchart+renesas@ideasonboard.com>
> > > >>
> > > >>[...]
> > > >>
> > > >>>diff --git a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > > >>>b/arch/arm/mach-shmobile/include/mach/r8a7779.h index f08d6ec..83c47a2
> > > >>>100644
> > > >>>--- a/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > > >>>+++ b/arch/arm/mach-shmobile/include/mach/r8a7779.h
> > > >>>@@ -6,6 +6,7 @@
> > > >>>
> > > >>>   #include <linux/sh_eth.h>
> > > >>>   
> > > >>>   struct platform_device;
> > > >>>
> > > >>>+struct rcar_du_platform_data;
> > > >>>
> > > >>>   struct r8a7779_pm_ch {
> > > >>>   
> > > >>>   	unsigned long chan_offs;
> > > >>>
> > > >>>@@ -31,6 +32,7 @@ extern void r8a7779_earlytimer_init(void);
> > > >>>
> > > >>>   extern void r8a7779_add_early_devices(void);
> > > >>>   extern void r8a7779_add_standard_devices(void);
> > > >>>   extern void r8a7779_add_standard_devices_dt(void);
> > > >>>
> > > >>>+extern void r8a7779_add_du_device(struct rcar_du_platform_data
> > > >>>*pdata);
> > > >>>
> > > >>>   extern void r8a7779_add_ether_device(struct sh_eth_plat_data
> > > >>>   *pdata);
> > > >>>   extern void r8a7779_init_late(void);
> > > >>>   extern void r8a7779_clock_init(void);
> > > >>>
> > > >>>diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
> > > >>>b/arch/arm/mach-shmobile/setup-r8a7779.c index 6a99333..c587fc9 100644
> > > >>>--- a/arch/arm/mach-shmobile/setup-r8a7779.c
> > > >>>+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> > > >>>@@ -27,6 +27,7 @@
> > > >>>
> > > >>>   #include <linux/of_platform.h>
> > > >>>   #include <linux/platform_data/gpio-rcar.h>
> > > >>>   #include <linux/platform_data/irq-renesas-intc-irqpin.h>
> > > >>>
> > > >>>+#include <linux/platform_data/rcar-du.h>
> > > >>
> > > >>Why not just include it in the r8a7779.h?
> > >>
> > >> Because not all users of r8a7779.h needs the rcar-du.h header. Including
> > >> the header only where it is needed speeds up compilation.
> > >
> > > I'm afraid the most important users (setup-* and board-*) need
> > > it. Anyway, Simon/Magnus will most probably ask you to remove the
> > > header change and rework the patch according to their taste.
> > 
> > At the risk of being inconsistent with feedback for previous patches:
> > I have no strong opinion on this at this time. Though personally I would
> > have just added the #include in r8a7779.h.
> 
> Magnus has asked me to move the setup-*.c code to the board files, I'll do so 
> and will resubmit the patches.

Thanks. I think that should side-step the #include question quite nicely.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-08-08  0:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-07 16:19 [PATCH 1/5] ARM: shmobile: r8a7779: Add DU support Laurent Pinchart
2013-08-07 17:20 ` Sergei Shtylyov
2013-08-07 22:19 ` Laurent Pinchart
2013-08-07 22:42 ` Sergei Shtylyov
2013-08-08  0:12 ` Kuninori Morimoto
2013-08-08  0:24 ` Simon Horman
2013-08-08  0:27 ` Laurent Pinchart
2013-08-08  0:28 ` Laurent Pinchart
2013-08-08  0:35 ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).