All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  2:06 ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  2:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
Register SATA controller as a "late" platform device on r8a7790 SoC.

Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
Vladimir Barinov.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
 arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index b393592..3619a52 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -50,6 +50,7 @@
 #define SMSTPCR2 0xe6150138
 #define SMSTPCR3 0xe615013c
 #define SMSTPCR7 0xe615014c
+#define SMSTPCR8 0xe6150990
 
 #define MODEMR		0xE6160060
 #define SDCKCR		0xE6150074
@@ -180,6 +181,7 @@ static struct clk div6_clks[DIV6_NR] = {
 
 /* MSTP */
 enum {
+	MSTP814,
 	MSTP721, MSTP720,
 	MSTP304,
 	MSTP216, MSTP207, MSTP206, MSTP204, MSTP203, MSTP202,
@@ -187,6 +189,7 @@ enum {
 };
 
 static struct clk mstp_clks[MSTP_NR] = {
+	[MSTP814] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 15, 0), /* SATA1 */
 	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
 	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
 	[MSTP304] = SH_CLK_MSTP32(&cp_clk, SMSTPCR3, 4, 0), /* TPU0 */
@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
 	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
 	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
 };
 
 #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
index ed7ee24..5a2a073 100644
--- a/arch/arm/mach-shmobile/setup-r8a7790.c
+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <linux/dma-mapping.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/of_platform.h>
@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
 				      sizeof(struct plat_sci_port));
 }
 
+static __init void r8a7790_register_sata1(void)
+{
+
+	struct resource res[] = {
+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
+		DEFINE_RES_IRQ(gic_iid(106)),
+	};
+
+	struct platform_device_info pdevinfo = {
+		.name = "sata_rcar",
+		.id = 1,
+		.res = res,
+		.num_res = ARRAY_SIZE(res),
+		.dma_mask = DMA_BIT_MASK(32),
+	};
+
+	platform_device_register_full(&pdevinfo);
+}
+
 static struct renesas_irqc_config irqc0_data = {
 	.irq_base = irq_pin(0), /* IRQ0 -> IRQ3 */
 };
@@ -148,6 +168,7 @@ void __init r8a7790_add_standard_devices(void)
 	r8a7790_register_scif(SCIF0);
 	r8a7790_register_scif(SCIF1);
 	r8a7790_register_irqc(0);
+	r8a7790_register_sata1();
 }
 
 void __init r8a7790_timer_init(void)
-- 
1.8.2.1


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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  2:06 ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  2:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
Register SATA controller as a "late" platform device on r8a7790 SoC.

Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
Vladimir Barinov.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
 arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index b393592..3619a52 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -50,6 +50,7 @@
 #define SMSTPCR2 0xe6150138
 #define SMSTPCR3 0xe615013c
 #define SMSTPCR7 0xe615014c
+#define SMSTPCR8 0xe6150990
 
 #define MODEMR		0xE6160060
 #define SDCKCR		0xE6150074
@@ -180,6 +181,7 @@ static struct clk div6_clks[DIV6_NR] = {
 
 /* MSTP */
 enum {
+	MSTP814,
 	MSTP721, MSTP720,
 	MSTP304,
 	MSTP216, MSTP207, MSTP206, MSTP204, MSTP203, MSTP202,
@@ -187,6 +189,7 @@ enum {
 };
 
 static struct clk mstp_clks[MSTP_NR] = {
+	[MSTP814] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 15, 0), /* SATA1 */
 	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
 	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
 	[MSTP304] = SH_CLK_MSTP32(&cp_clk, SMSTPCR3, 4, 0), /* TPU0 */
@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
 	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
 	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
 };
 
 #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
index ed7ee24..5a2a073 100644
--- a/arch/arm/mach-shmobile/setup-r8a7790.c
+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <linux/dma-mapping.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/of_platform.h>
@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
 				      sizeof(struct plat_sci_port));
 }
 
+static __init void r8a7790_register_sata1(void)
+{
+
+	struct resource res[] = {
+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
+		DEFINE_RES_IRQ(gic_iid(106)),
+	};
+
+	struct platform_device_info pdevinfo = {
+		.name = "sata_rcar",
+		.id = 1,
+		.res = res,
+		.num_res = ARRAY_SIZE(res),
+		.dma_mask = DMA_BIT_MASK(32),
+	};
+
+	platform_device_register_full(&pdevinfo);
+}
+
 static struct renesas_irqc_config irqc0_data = {
 	.irq_base = irq_pin(0), /* IRQ0 -> IRQ3 */
 };
@@ -148,6 +168,7 @@ void __init r8a7790_add_standard_devices(void)
 	r8a7790_register_scif(SCIF0);
 	r8a7790_register_scif(SCIF1);
 	r8a7790_register_irqc(0);
+	r8a7790_register_sata1();
 }
 
 void __init r8a7790_timer_init(void)
-- 
1.8.2.1

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  2:06 ` Simon Horman
@ 2013-05-24  2:17   ` Kuninori Morimoto
  -1 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  2:17 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> Vladimir Barinov.
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---

> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
(snip)
> +static __init void r8a7790_register_sata1(void)
> +{
> +
> +	struct resource res[] = {
> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> +		DEFINE_RES_IRQ(gic_iid(106)),
> +	};
> +
> +	struct platform_device_info pdevinfo = {
> +		.name = "sata_rcar",
> +		.id = 1,
> +		.res = res,
> +		.num_res = ARRAY_SIZE(res),
> +		.dma_mask = DMA_BIT_MASK(32),
> +	};
> +
> +	platform_device_register_full(&pdevinfo);
> +}

This is a little bit strange for me.
Why you don't care SATA0 ?

Can you use like this ?

static __init void r8a7790_register_sata(int id);


BTW, we need to decide "driver registration function naming" ?
In r8a7778 case, it is using 

	r8a7778_add_xxx_device()
	r8a7778_add_xxx_devices()

but r8a7778_register_xxx seems nice naming :)

Best regards
---
Kuninori Morimoto

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  2:17   ` Kuninori Morimoto
  0 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  2:17 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> Vladimir Barinov.
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---

> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
(snip)
> +static __init void r8a7790_register_sata1(void)
> +{
> +
> +	struct resource res[] = {
> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> +		DEFINE_RES_IRQ(gic_iid(106)),
> +	};
> +
> +	struct platform_device_info pdevinfo = {
> +		.name = "sata_rcar",
> +		.id = 1,
> +		.res = res,
> +		.num_res = ARRAY_SIZE(res),
> +		.dma_mask = DMA_BIT_MASK(32),
> +	};
> +
> +	platform_device_register_full(&pdevinfo);
> +}

This is a little bit strange for me.
Why you don't care SATA0 ?

Can you use like this ?

static __init void r8a7790_register_sata(int id);


BTW, we need to decide "driver registration function naming" ?
In r8a7778 case, it is using 

	r8a7778_add_xxx_device()
	r8a7778_add_xxx_devices()

but r8a7778_register_xxx seems nice naming :)

Best regards
---
Kuninori Morimoto

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  2:17   ` Kuninori Morimoto
@ 2013-05-24  2:21     ` Kuninori Morimoto
  -1 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  2:21 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> > +static __init void r8a7790_register_sata1(void)
> > +{
> > +
> > +	struct resource res[] = {
> > +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> > +		DEFINE_RES_IRQ(gic_iid(106)),
> > +	};
> > +
> > +	struct platform_device_info pdevinfo = {
> > +		.name = "sata_rcar",
> > +		.id = 1,
> > +		.res = res,
> > +		.num_res = ARRAY_SIZE(res),
> > +		.dma_mask = DMA_BIT_MASK(32),
> > +	};
> > +
> > +	platform_device_register_full(&pdevinfo);
> > +}

This "using local variable" make sense.
It don't need "__initdata"


Best regards
---
Kuninori Morimoto

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  2:21     ` Kuninori Morimoto
  0 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  2:21 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> > +static __init void r8a7790_register_sata1(void)
> > +{
> > +
> > +	struct resource res[] = {
> > +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> > +		DEFINE_RES_IRQ(gic_iid(106)),
> > +	};
> > +
> > +	struct platform_device_info pdevinfo = {
> > +		.name = "sata_rcar",
> > +		.id = 1,
> > +		.res = res,
> > +		.num_res = ARRAY_SIZE(res),
> > +		.dma_mask = DMA_BIT_MASK(32),
> > +	};
> > +
> > +	platform_device_register_full(&pdevinfo);
> > +}

This "using local variable" make sense.
It don't need "__initdata"


Best regards
---
Kuninori Morimoto

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  2:17   ` Kuninori Morimoto
@ 2013-05-24  2:59     ` Simon Horman
  -1 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  2:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 23, 2013 at 07:17:22PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> > Register SATA controller as a "late" platform device on r8a7790 SoC.
> > 
> > Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> > Vladimir Barinov.
> > 
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> 
> > +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> (snip)
> > +static __init void r8a7790_register_sata1(void)
> > +{
> > +
> > +	struct resource res[] = {
> > +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> > +		DEFINE_RES_IRQ(gic_iid(106)),
> > +	};
> > +
> > +	struct platform_device_info pdevinfo = {
> > +		.name = "sata_rcar",
> > +		.id = 1,
> > +		.res = res,
> > +		.num_res = ARRAY_SIZE(res),
> > +		.dma_mask = DMA_BIT_MASK(32),
> > +	};
> > +
> > +	platform_device_register_full(&pdevinfo);
> > +}
> 
> This is a little bit strange for me.
> Why you don't care SATA0 ?

Because the lager board only seems to expose SATA1 and I was
interested in something I could test.

I am happy to start caring about SATA0 if you think it is
a good idea.

> Can you use like this ?
> 
> static __init void r8a7790_register_sata(int id);

Sure, but I'm unsure how you want r8a7790_register_sata to handle id.

Is it just for the id field of struct platform_device_info pdevinfo?
Or also to switch between resources somehow?

> BTW, we need to decide "driver registration function naming" ?
> In r8a7778 case, it is using 
> 
> 	r8a7778_add_xxx_device()
> 	r8a7778_add_xxx_devices()
> 
> but r8a7778_register_xxx seems nice naming :)

I chose register as it seemed to be consistent with the
existing contents of r8a7790_add_standard_devices().

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  2:59     ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  2:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 23, 2013 at 07:17:22PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> > Register SATA controller as a "late" platform device on r8a7790 SoC.
> > 
> > Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> > Vladimir Barinov.
> > 
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> 
> > +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> (snip)
> > +static __init void r8a7790_register_sata1(void)
> > +{
> > +
> > +	struct resource res[] = {
> > +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> > +		DEFINE_RES_IRQ(gic_iid(106)),
> > +	};
> > +
> > +	struct platform_device_info pdevinfo = {
> > +		.name = "sata_rcar",
> > +		.id = 1,
> > +		.res = res,
> > +		.num_res = ARRAY_SIZE(res),
> > +		.dma_mask = DMA_BIT_MASK(32),
> > +	};
> > +
> > +	platform_device_register_full(&pdevinfo);
> > +}
> 
> This is a little bit strange for me.
> Why you don't care SATA0 ?

Because the lager board only seems to expose SATA1 and I was
interested in something I could test.

I am happy to start caring about SATA0 if you think it is
a good idea.

> Can you use like this ?
> 
> static __init void r8a7790_register_sata(int id);

Sure, but I'm unsure how you want r8a7790_register_sata to handle id.

Is it just for the id field of struct platform_device_info pdevinfo?
Or also to switch between resources somehow?

> BTW, we need to decide "driver registration function naming" ?
> In r8a7778 case, it is using 
> 
> 	r8a7778_add_xxx_device()
> 	r8a7778_add_xxx_devices()
> 
> but r8a7778_register_xxx seems nice naming :)

I chose register as it seemed to be consistent with the
existing contents of r8a7790_add_standard_devices().

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  2:59     ` Simon Horman
@ 2013-05-24  3:11       ` Kuninori Morimoto
  -1 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  3:11 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> Because the lager board only seems to expose SATA1 and I was
> interested in something I could test.
> 
> I am happy to start caring about SATA0 if you think it is
> a good idea.

Caring both SATA0/1 are nice IMO :)

> > Can you use like this ?
> > 
> > static __init void r8a7790_register_sata(int id);
> 
> Sure, but I'm unsure how you want r8a7790_register_sata to handle id.
> 
> Is it just for the id field of struct platform_device_info pdevinfo?
> Or also to switch between resources somehow?

can you check r8a7778_sdhi_init(int id, xxx) ?
It is caring both id field and resources.

> > BTW, we need to decide "driver registration function naming" ?
> > In r8a7778 case, it is using 
> > 
> > 	r8a7778_add_xxx_device()
> > 	r8a7778_add_xxx_devices()
> > 
> > but r8a7778_register_xxx seems nice naming :)
> 
> I chose register as it seemed to be consistent with the
> existing contents of r8a7790_add_standard_devices().

OK, nice.

Do you think other chip (= r8a7778) should follow same naming?


Best regards
---
Kuninori Morimoto

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  3:11       ` Kuninori Morimoto
  0 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2013-05-24  3:11 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Simon

> Because the lager board only seems to expose SATA1 and I was
> interested in something I could test.
> 
> I am happy to start caring about SATA0 if you think it is
> a good idea.

Caring both SATA0/1 are nice IMO :)

> > Can you use like this ?
> > 
> > static __init void r8a7790_register_sata(int id);
> 
> Sure, but I'm unsure how you want r8a7790_register_sata to handle id.
> 
> Is it just for the id field of struct platform_device_info pdevinfo?
> Or also to switch between resources somehow?

can you check r8a7778_sdhi_init(int id, xxx) ?
It is caring both id field and resources.

> > BTW, we need to decide "driver registration function naming" ?
> > In r8a7778 case, it is using 
> > 
> > 	r8a7778_add_xxx_device()
> > 	r8a7778_add_xxx_devices()
> > 
> > but r8a7778_register_xxx seems nice naming :)
> 
> I chose register as it seemed to be consistent with the
> existing contents of r8a7790_add_standard_devices().

OK, nice.

Do you think other chip (= r8a7778) should follow same naming?


Best regards
---
Kuninori Morimoto

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  3:11       ` Kuninori Morimoto
@ 2013-05-24  3:44         ` Simon Horman
  -1 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  3:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 23, 2013 at 08:11:05PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Because the lager board only seems to expose SATA1 and I was
> > interested in something I could test.
> > 
> > I am happy to start caring about SATA0 if you think it is
> > a good idea.
> 
> Caring both SATA0/1 are nice IMO :)
> 
> > > Can you use like this ?
> > > 
> > > static __init void r8a7790_register_sata(int id);
> > 
> > Sure, but I'm unsure how you want r8a7790_register_sata to handle id.
> > 
> > Is it just for the id field of struct platform_device_info pdevinfo?
> > Or also to switch between resources somehow?
> 
> can you check r8a7778_sdhi_init(int id, xxx) ?
> It is caring both id field and resources.

Thanks, I will look at that.

> > > BTW, we need to decide "driver registration function naming" ?
> > > In r8a7778 case, it is using 
> > > 
> > > 	r8a7778_add_xxx_device()
> > > 	r8a7778_add_xxx_devices()
> > > 
> > > but r8a7778_register_xxx seems nice naming :)
> > 
> > I chose register as it seemed to be consistent with the
> > existing contents of r8a7790_add_standard_devices().
> 
> OK, nice.
> 
> Do you think other chip (= r8a7778) should follow same naming?

I think it is not so important.

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24  3:44         ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-24  3:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 23, 2013 at 08:11:05PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Because the lager board only seems to expose SATA1 and I was
> > interested in something I could test.
> > 
> > I am happy to start caring about SATA0 if you think it is
> > a good idea.
> 
> Caring both SATA0/1 are nice IMO :)
> 
> > > Can you use like this ?
> > > 
> > > static __init void r8a7790_register_sata(int id);
> > 
> > Sure, but I'm unsure how you want r8a7790_register_sata to handle id.
> > 
> > Is it just for the id field of struct platform_device_info pdevinfo?
> > Or also to switch between resources somehow?
> 
> can you check r8a7778_sdhi_init(int id, xxx) ?
> It is caring both id field and resources.

Thanks, I will look at that.

> > > BTW, we need to decide "driver registration function naming" ?
> > > In r8a7778 case, it is using 
> > > 
> > > 	r8a7778_add_xxx_device()
> > > 	r8a7778_add_xxx_devices()
> > > 
> > > but r8a7778_register_xxx seems nice naming :)
> > 
> > I chose register as it seemed to be consistent with the
> > existing contents of r8a7790_add_standard_devices().
> 
> OK, nice.
> 
> Do you think other chip (= r8a7778) should follow same naming?

I think it is not so important.

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24  2:06 ` Simon Horman
@ 2013-05-24 14:02   ` Sergei Shtylyov
  -1 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2013-05-24 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 24-05-2013 6:06, Simon Horman wrote:

> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).

     I don't see where you are adding clock for device tree case.

> Register SATA controller as a "late" platform device on r8a7790 SoC.

     Does R8A7790 have these "early" and "late" lists? I guess not.

> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> Vladimir Barinov.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>   arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
>   arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
>   2 files changed, 25 insertions(+)

> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> index b393592..3619a52 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
[...]
> @@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
>   	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
>   	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
>   	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
>   };
>
>   #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> index ed7ee24..5a2a073 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
[...]
> @@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
>   				      sizeof(struct plat_sci_port));
>   }
>
> +static __init void r8a7790_register_sata1(void)

    What's with SATA0?

> +{
> +
> +	struct resource res[] = {

    Non-static variable with an intializer? Won't this be a waste of 
code? Wouldn't it be better to have it static __initdata?

> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),

     2 megabytes? Won't it be a waste of memory?

> +		DEFINE_RES_IRQ(gic_iid(106)),
> +	};
> +
> +	struct platform_device_info pdevinfo = {

    Same comment about non-static initialized structure...

> +		.name = "sata_rcar",
> +		.id = 1,
> +		.res = res,
> +		.num_res = ARRAY_SIZE(res),
> +		.dma_mask = DMA_BIT_MASK(32),
> +	};
> +
> +	platform_device_register_full(&pdevinfo);
> +}
> +

WBR, Sergei


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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-24 14:02   ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2013-05-24 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 24-05-2013 6:06, Simon Horman wrote:

> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).

     I don't see where you are adding clock for device tree case.

> Register SATA controller as a "late" platform device on r8a7790 SoC.

     Does R8A7790 have these "early" and "late" lists? I guess not.

> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> Vladimir Barinov.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>   arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
>   arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
>   2 files changed, 25 insertions(+)

> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> index b393592..3619a52 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
[...]
> @@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
>   	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
>   	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
>   	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
>   };
>
>   #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> index ed7ee24..5a2a073 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
[...]
> @@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
>   				      sizeof(struct plat_sci_port));
>   }
>
> +static __init void r8a7790_register_sata1(void)

    What's with SATA0?

> +{
> +
> +	struct resource res[] = {

    Non-static variable with an intializer? Won't this be a waste of 
code? Wouldn't it be better to have it static __initdata?

> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),

     2 megabytes? Won't it be a waste of memory?

> +		DEFINE_RES_IRQ(gic_iid(106)),
> +	};
> +
> +	struct platform_device_info pdevinfo = {

    Same comment about non-static initialized structure...

> +		.name = "sata_rcar",
> +		.id = 1,
> +		.res = res,
> +		.num_res = ARRAY_SIZE(res),
> +		.dma_mask = DMA_BIT_MASK(32),
> +	};
> +
> +	platform_device_register_full(&pdevinfo);
> +}
> +

WBR, Sergei

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-24 14:02   ` Sergei Shtylyov
@ 2013-05-25  1:03     ` Simon Horman
  -1 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-25  1:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 24, 2013 at 06:02:32PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 24-05-2013 6:06, Simon Horman wrote:
> 
> >Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> 
>     I don't see where you are adding clock for device tree case.
> 
> >Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
>     Does R8A7790 have these "early" and "late" lists? I guess not.
> 
> >Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> >Vladimir Barinov.
> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >---
> >  arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
> >  arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
> >  2 files changed, 25 insertions(+)
> 
> >diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> >index b393592..3619a52 100644
> >--- a/arch/arm/mach-shmobile/clock-r8a7790.c
> >+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> [...]
> >@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
> >  	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
> >  	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
> >  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> >+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> >  };
> >
> >  #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> >diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> >index ed7ee24..5a2a073 100644
> >--- a/arch/arm/mach-shmobile/setup-r8a7790.c
> >+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> [...]
> >@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
> >  				      sizeof(struct plat_sci_port));
> >  }
> >
> >+static __init void r8a7790_register_sata1(void)
> 
>    What's with SATA0?
> 
> >+{
> >+
> >+	struct resource res[] = {
> 
>    Non-static variable with an intializer? Won't this be a waste of
> code? Wouldn't it be better to have it static __initdata?

Sure, if you prefer that.

> 
> >+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> 
>     2 megabytes? Won't it be a waste of memory?

2 megabytes is specified in the data sheet.
Do you think it is reasonable to use a smaller value?

> 
> >+		DEFINE_RES_IRQ(gic_iid(106)),
> >+	};
> >+
> >+	struct platform_device_info pdevinfo = {
> 
>    Same comment about non-static initialized structure...
> 
> >+		.name = "sata_rcar",
> >+		.id = 1,
> >+		.res = res,
> >+		.num_res = ARRAY_SIZE(res),
> >+		.dma_mask = DMA_BIT_MASK(32),
> >+	};
> >+
> >+	platform_device_register_full(&pdevinfo);
> >+}
> >+
> 
> WBR, Sergei
> 

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-25  1:03     ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-25  1:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 24, 2013 at 06:02:32PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 24-05-2013 6:06, Simon Horman wrote:
> 
> >Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> 
>     I don't see where you are adding clock for device tree case.
> 
> >Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
>     Does R8A7790 have these "early" and "late" lists? I guess not.
> 
> >Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> >Vladimir Barinov.
> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >---
> >  arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
> >  arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
> >  2 files changed, 25 insertions(+)
> 
> >diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> >index b393592..3619a52 100644
> >--- a/arch/arm/mach-shmobile/clock-r8a7790.c
> >+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> [...]
> >@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
> >  	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
> >  	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
> >  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> >+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> >  };
> >
> >  #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> >diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> >index ed7ee24..5a2a073 100644
> >--- a/arch/arm/mach-shmobile/setup-r8a7790.c
> >+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> [...]
> >@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
> >  				      sizeof(struct plat_sci_port));
> >  }
> >
> >+static __init void r8a7790_register_sata1(void)
> 
>    What's with SATA0?
> 
> >+{
> >+
> >+	struct resource res[] = {
> 
>    Non-static variable with an intializer? Won't this be a waste of
> code? Wouldn't it be better to have it static __initdata?

Sure, if you prefer that.

> 
> >+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> 
>     2 megabytes? Won't it be a waste of memory?

2 megabytes is specified in the data sheet.
Do you think it is reasonable to use a smaller value?

> 
> >+		DEFINE_RES_IRQ(gic_iid(106)),
> >+	};
> >+
> >+	struct platform_device_info pdevinfo = {
> 
>    Same comment about non-static initialized structure...
> 
> >+		.name = "sata_rcar",
> >+		.id = 1,
> >+		.res = res,
> >+		.num_res = ARRAY_SIZE(res),
> >+		.dma_mask = DMA_BIT_MASK(32),
> >+	};
> >+
> >+	platform_device_register_full(&pdevinfo);
> >+}
> >+
> 
> WBR, Sergei
> 

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-25  1:03     ` Simon Horman
@ 2013-05-25 18:32       ` Sergei Shtylyov
  -1 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2013-05-25 18:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 25-05-2013 5:03, Simon Horman wrote:

>>> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).

>>      I don't see where you are adding clock for device tree case.

>>> Register SATA controller as a "late" platform device on r8a7790 SoC.

>>      Does R8A7790 have these "early" and "late" lists? I guess not.

>>> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
>>> Vladimir Barinov.

>>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>>> ---
>>>   arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
>>>   arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
>>>   2 files changed, 25 insertions(+)

>>> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
>>> index b393592..3619a52 100644
>>> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
>>> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
>> [...]
>>> @@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
>>>   	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
>>>   	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
>>>   	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
>>> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
>>>   };
>>>
>>>   #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
>>> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
>>> index ed7ee24..5a2a073 100644
>>> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
>>> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
>> [...]
>>> @@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
>>>   				      sizeof(struct plat_sci_port));
>>>   }

>>> +static __init void r8a7790_register_sata1(void)

>>     What's with SATA0?

>>> +{
>>> +
>>> +	struct resource res[] = {

>>     Non-static variable with an intializer? Won't this be a waste of
>> code? Wouldn't it be better to have it static __initdata?

> Sure, if you prefer that.

    I don't know exactly what gcc will generate for the intializers but 
expect nothing particularly good. The static intialized data should 
definitely take up less space.

>>
>>> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),

>>      2 megabytes? Won't it be a waste of memory?

> 2 megabytes is specified in the data sheet.
> Do you think it is reasonable to use a smaller value?

    We used 0x2000 for R8A7779 SATA device -- it should be enough to 
cover all the registers.

>>> +		DEFINE_RES_IRQ(gic_iid(106)),
>>> +	};
>>> +
>>> +	struct platform_device_info pdevinfo = {

>>     Same comment about non-static initialized structure...

>>> +		.name = "sata_rcar",
>>> +		.id = 1,
>>> +		.res = res,
>>> +		.num_res = ARRAY_SIZE(res),
>>> +		.dma_mask = DMA_BIT_MASK(32),
>>> +	};
>>> +
>>> +	platform_device_register_full(&pdevinfo);
>>> +}
>>> +

WBR, Sergei


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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-25 18:32       ` Sergei Shtylyov
  0 siblings, 0 replies; 20+ messages in thread
From: Sergei Shtylyov @ 2013-05-25 18:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 25-05-2013 5:03, Simon Horman wrote:

>>> Add SATA clock for r8a7790 SoC (for both device tree and usual cases).

>>      I don't see where you are adding clock for device tree case.

>>> Register SATA controller as a "late" platform device on r8a7790 SoC.

>>      Does R8A7790 have these "early" and "late" lists? I guess not.

>>> Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
>>> Vladimir Barinov.

>>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>>> ---
>>>   arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
>>>   arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
>>>   2 files changed, 25 insertions(+)

>>> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
>>> index b393592..3619a52 100644
>>> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
>>> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
>> [...]
>>> @@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
>>>   	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
>>>   	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
>>>   	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
>>> +	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
>>>   };
>>>
>>>   #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
>>> diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
>>> index ed7ee24..5a2a073 100644
>>> --- a/arch/arm/mach-shmobile/setup-r8a7790.c
>>> +++ b/arch/arm/mach-shmobile/setup-r8a7790.c
>> [...]
>>> @@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
>>>   				      sizeof(struct plat_sci_port));
>>>   }

>>> +static __init void r8a7790_register_sata1(void)

>>     What's with SATA0?

>>> +{
>>> +
>>> +	struct resource res[] = {

>>     Non-static variable with an intializer? Won't this be a waste of
>> code? Wouldn't it be better to have it static __initdata?

> Sure, if you prefer that.

    I don't know exactly what gcc will generate for the intializers but 
expect nothing particularly good. The static intialized data should 
definitely take up less space.

>>
>>> +		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),

>>      2 megabytes? Won't it be a waste of memory?

> 2 megabytes is specified in the data sheet.
> Do you think it is reasonable to use a smaller value?

    We used 0x2000 for R8A7779 SATA device -- it should be enough to 
cover all the registers.

>>> +		DEFINE_RES_IRQ(gic_iid(106)),
>>> +	};
>>> +
>>> +	struct platform_device_info pdevinfo = {

>>     Same comment about non-static initialized structure...

>>> +		.name = "sata_rcar",
>>> +		.id = 1,
>>> +		.res = res,
>>> +		.num_res = ARRAY_SIZE(res),
>>> +		.dma_mask = DMA_BIT_MASK(32),
>>> +	};
>>> +
>>> +	platform_device_register_full(&pdevinfo);
>>> +}
>>> +

WBR, Sergei

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

* Re: [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
  2013-05-25 18:32       ` Sergei Shtylyov
@ 2013-05-26 13:25         ` Simon Horman
  -1 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-26 13:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, May 25, 2013 at 10:32:57PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 25-05-2013 5:03, Simon Horman wrote:
> 
> >>>Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> 
> >>     I don't see where you are adding clock for device tree case.
> 
> >>>Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
> >>     Does R8A7790 have these "early" and "late" lists? I guess not.
> 
> >>>Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> >>>Vladimir Barinov.
> 
> >>>Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >>>---
> >>>  arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
> >>>  arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
> >>>  2 files changed, 25 insertions(+)
> 
> >>>diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> >>>index b393592..3619a52 100644
> >>>--- a/arch/arm/mach-shmobile/clock-r8a7790.c
> >>>+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> >>[...]
> >>>@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
> >>>  	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
> >>>  	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
> >>>  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> >>>+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> >>>  };
> >>>
> >>>  #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> >>>diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> >>>index ed7ee24..5a2a073 100644
> >>>--- a/arch/arm/mach-shmobile/setup-r8a7790.c
> >>>+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> >>[...]
> >>>@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
> >>>  				      sizeof(struct plat_sci_port));
> >>>  }
> 
> >>>+static __init void r8a7790_register_sata1(void)
> 
> >>    What's with SATA0?
> 
> >>>+{
> >>>+
> >>>+	struct resource res[] = {
> 
> >>    Non-static variable with an intializer? Won't this be a waste of
> >>code? Wouldn't it be better to have it static __initdata?
> 
> >Sure, if you prefer that.
> 
>    I don't know exactly what gcc will generate for the intializers
> but expect nothing particularly good. The static intialized data
> should definitely take up less space.
> 
> >>
> >>>+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> 
> >>     2 megabytes? Won't it be a waste of memory?
> 
> >2 megabytes is specified in the data sheet.
> >Do you think it is reasonable to use a smaller value?
> 
>    We used 0x2000 for R8A7779 SATA device -- it should be enough to
> cover all the registers.

Thanks for your advice, I'll see about doing the same for r8a7790.

> >>>+		DEFINE_RES_IRQ(gic_iid(106)),
> >>>+	};
> >>>+
> >>>+	struct platform_device_info pdevinfo = {
> 
> >>    Same comment about non-static initialized structure...
> 
> >>>+		.name = "sata_rcar",
> >>>+		.id = 1,
> >>>+		.res = res,
> >>>+		.num_res = ARRAY_SIZE(res),
> >>>+		.dma_mask = DMA_BIT_MASK(32),
> >>>+	};
> >>>+
> >>>+	platform_device_register_full(&pdevinfo);
> >>>+}
> >>>+
> 
> WBR, Sergei
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* [PATCH] ARM: mach-shmobile: r8a7790: add SATA support
@ 2013-05-26 13:25         ` Simon Horman
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Horman @ 2013-05-26 13:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, May 25, 2013 at 10:32:57PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 25-05-2013 5:03, Simon Horman wrote:
> 
> >>>Add SATA clock for r8a7790 SoC (for both device tree and usual cases).
> 
> >>     I don't see where you are adding clock for device tree case.
> 
> >>>Register SATA controller as a "late" platform device on r8a7790 SoC.
> 
> >>     Does R8A7790 have these "early" and "late" lists? I guess not.
> 
> >>>Based on "ARM: mach-shmobile: r8a7779: add SATA support" by
> >>>Vladimir Barinov.
> 
> >>>Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >>>---
> >>>  arch/arm/mach-shmobile/clock-r8a7790.c |  4 ++++
> >>>  arch/arm/mach-shmobile/setup-r8a7790.c | 21 +++++++++++++++++++++
> >>>  2 files changed, 25 insertions(+)
> 
> >>>diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> >>>index b393592..3619a52 100644
> >>>--- a/arch/arm/mach-shmobile/clock-r8a7790.c
> >>>+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> >>[...]
> >>>@@ -249,6 +252,7 @@ static struct clk_lookup lookups[] = {
> >>>  	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP202]),
> >>>  	CLKDEV_DEV_ID("sh-sci.6", &mstp_clks[MSTP721]),
> >>>  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> >>>+	CLKDEV_DEV_ID("sata_rcar.1", &mstp_clks[MSTP814]),
> >>>  };
> >>>
> >>>  #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
> >>>diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c
> >>>index ed7ee24..5a2a073 100644
> >>>--- a/arch/arm/mach-shmobile/setup-r8a7790.c
> >>>+++ b/arch/arm/mach-shmobile/setup-r8a7790.c
> >>[...]
> >>>@@ -118,6 +119,25 @@ static inline void r8a7790_register_scif(int idx)
> >>>  				      sizeof(struct plat_sci_port));
> >>>  }
> 
> >>>+static __init void r8a7790_register_sata1(void)
> 
> >>    What's with SATA0?
> 
> >>>+{
> >>>+
> >>>+	struct resource res[] = {
> 
> >>    Non-static variable with an intializer? Won't this be a waste of
> >>code? Wouldn't it be better to have it static __initdata?
> 
> >Sure, if you prefer that.
> 
>    I don't know exactly what gcc will generate for the intializers
> but expect nothing particularly good. The static intialized data
> should definitely take up less space.
> 
> >>
> >>>+		DEFINE_RES_MEM_NAMED(0xee300000, 0x200000, "rcar-sata"),
> 
> >>     2 megabytes? Won't it be a waste of memory?
> 
> >2 megabytes is specified in the data sheet.
> >Do you think it is reasonable to use a smaller value?
> 
>    We used 0x2000 for R8A7779 SATA device -- it should be enough to
> cover all the registers.

Thanks for your advice, I'll see about doing the same for r8a7790.

> >>>+		DEFINE_RES_IRQ(gic_iid(106)),
> >>>+	};
> >>>+
> >>>+	struct platform_device_info pdevinfo = {
> 
> >>    Same comment about non-static initialized structure...
> 
> >>>+		.name = "sata_rcar",
> >>>+		.id = 1,
> >>>+		.res = res,
> >>>+		.num_res = ARRAY_SIZE(res),
> >>>+		.dma_mask = DMA_BIT_MASK(32),
> >>>+	};
> >>>+
> >>>+	platform_device_register_full(&pdevinfo);
> >>>+}
> >>>+
> 
> WBR, Sergei
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2013-05-26 13:25 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-24  2:06 [PATCH] ARM: mach-shmobile: r8a7790: add SATA support Simon Horman
2013-05-24  2:06 ` Simon Horman
2013-05-24  2:17 ` Kuninori Morimoto
2013-05-24  2:17   ` Kuninori Morimoto
2013-05-24  2:21   ` Kuninori Morimoto
2013-05-24  2:21     ` Kuninori Morimoto
2013-05-24  2:59   ` Simon Horman
2013-05-24  2:59     ` Simon Horman
2013-05-24  3:11     ` Kuninori Morimoto
2013-05-24  3:11       ` Kuninori Morimoto
2013-05-24  3:44       ` Simon Horman
2013-05-24  3:44         ` Simon Horman
2013-05-24 14:02 ` Sergei Shtylyov
2013-05-24 14:02   ` Sergei Shtylyov
2013-05-25  1:03   ` Simon Horman
2013-05-25  1:03     ` Simon Horman
2013-05-25 18:32     ` Sergei Shtylyov
2013-05-25 18:32       ` Sergei Shtylyov
2013-05-26 13:25       ` Simon Horman
2013-05-26 13:25         ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.