All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
@ 2015-07-06  6:59 Magnus Damm
  2015-07-06  7:41 ` Simon Horman
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Magnus Damm @ 2015-07-06  6:59 UTC (permalink / raw)
  To: linux-sh

From: Magnus Damm <damm+renesas@opensource.se>

Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.

As it is today Marzen multiplatform is working, but the legacy code
is broken. This patch intends to leave multiplatform as-is but do
a simple one-shot fix to unbreak the legacy code base.

Without this patch there is no preset delay and the TWD is defined
both in C code and in DT. Solve the delay by invoking shmobile_init_delay()
and simply exclude the TWD DT instance in case of building for legacy.

Marzen legacy code will be removed in the near future, but until then
we may as well avoid breaking it.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Built on top of renesas-devel-20150629-v4.1

 arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
 arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

--- 0001/arch/arm/boot/dts/r8a7779.dtsi
+++ work/arch/arm/boot/dts/r8a7779.dtsi	2015-07-06 15:42:28.182366518 +0900
@@ -62,7 +62,7 @@
 		reg = <0xf0001000 0x1000>,
 		      <0xf0000100 0x100>;
 	};
-
+#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy case */
 	timer@f0000600 {
 		compatible = "arm,cortex-a9-twd-timer";
 		reg = <0xf0000600 0x20>;
@@ -70,7 +70,7 @@
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		clocks = <&cpg_clocks R8A7779_CLK_ZS>;
 	};
-
+#endif
 	gpio0: gpio@ffc40000 {
 		compatible = "renesas,gpio-r8a7779", "renesas,gpio-rcar";
 		reg = <0xffc40000 0x2c>;
--- 0001/arch/arm/mach-shmobile/setup-r8a7779.c
+++ work/arch/arm/mach-shmobile/setup-r8a7779.c	2015-07-06 15:43:58.472366518 +0900
@@ -677,6 +677,8 @@ void __init r8a7779_add_standard_devices
 
 void __init r8a7779_add_early_devices(void)
 {
+	shmobile_init_delay();
+
 	early_platform_add_devices(r8a7779_early_devices,
 				   ARRAY_SIZE(r8a7779_early_devices));
 

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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
@ 2015-07-06  7:41 ` Simon Horman
  2015-07-06  7:53 ` Laurent Pinchart
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2015-07-06  7:41 UTC (permalink / raw)
  To: linux-sh

On Mon, Jul 06, 2015 at 04:02:16PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
> 
> As it is today Marzen multiplatform is working, but the legacy code
> is broken. This patch intends to leave multiplatform as-is but do
> a simple one-shot fix to unbreak the legacy code base.
> 
> Without this patch there is no preset delay and the TWD is defined
> both in C code and in DT. Solve the delay by invoking shmobile_init_delay()
> and simply exclude the TWD DT instance in case of building for legacy.
> 
> Marzen legacy code will be removed in the near future, but until then
> we may as well avoid breaking it.

Thanks, that does seem to improve things a lot.

Tested-by: Simon Horman <horms+renesas@verge.net.au>

I wonder if we should also add the following?

Fixes: f2a5473861cf ("clocksource: sh_tmu: Set cpu_possible_mask to fix SMP broadcast")


For the record, I found that after I enabled CONFIG_SMSC_PHY the boot
now gets as far as initialising SDHI.

...

i2c-rcar i2c-rcar.0: probed
i2c-rcar i2c-rcar.1: probed
i2c-rcar i2c-rcar.2: probed
i2c-rcar i2c-rcar.3: probed
initcall rcar_i2c_driver_init+0x0/0x10 returned 0 after 10428 usecs
calling  soc_camera_pdrv_init+0x0/0x10 @ 1
initcall soc_camera_pdrv_init+0x0/0x10 returned 0 after 74 usecs
calling  soc_mbus_init+0x0/0x8 @ 1
initcall soc_mbus_init+0x0/0x8 returned 0 after 0 usecs
calling  rcar_vin_driver_init+0x0/0x10 @ 1
soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
soc-camera-pdrv soc-camera-pdrv.1: Probing soc-camera-pdrv.1
initcall rcar_vin_driver_init+0x0/0x10 returned 0 after 11502 usecs
calling  rcar_thermal_driver_init+0x0/0x10 @ 1
rcar_thermal rcar_thermal.0: 1 sensor probed
initcall rcar_thermal_driver_init+0x0/0x10 returned 0 after 4596 usecs
calling  mmc_blk_init+0x0/0x88 @ 1
initcall mmc_blk_init+0x0/0x88 returned 0 after 31 usecs
calling  sh_mobile_sdhi_driver_init+0x0/0x10 @ 1

> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Built on top of renesas-devel-20150629-v4.1
> 
>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> +++ work/arch/arm/boot/dts/r8a7779.dtsi	2015-07-06 15:42:28.182366518 +0900
> @@ -62,7 +62,7 @@
>  		reg = <0xf0001000 0x1000>,
>  		      <0xf0000100 0x100>;
>  	};
> -
> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy case */
>  	timer@f0000600 {
>  		compatible = "arm,cortex-a9-twd-timer";
>  		reg = <0xf0000600 0x20>;
> @@ -70,7 +70,7 @@
>  			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>  		clocks = <&cpg_clocks R8A7779_CLK_ZS>;
>  	};
> -
> +#endif
>  	gpio0: gpio@ffc40000 {
>  		compatible = "renesas,gpio-r8a7779", "renesas,gpio-rcar";
>  		reg = <0xffc40000 0x2c>;
> --- 0001/arch/arm/mach-shmobile/setup-r8a7779.c
> +++ work/arch/arm/mach-shmobile/setup-r8a7779.c	2015-07-06 15:43:58.472366518 +0900
> @@ -677,6 +677,8 @@ void __init r8a7779_add_standard_devices
>  
>  void __init r8a7779_add_early_devices(void)
>  {
> +	shmobile_init_delay();
> +
>  	early_platform_add_devices(r8a7779_early_devices,
>  				   ARRAY_SIZE(r8a7779_early_devices));
>  
> 

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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
  2015-07-06  7:41 ` Simon Horman
@ 2015-07-06  7:53 ` Laurent Pinchart
  2015-07-06  7:58 ` Magnus Damm
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-07-06  7:53 UTC (permalink / raw)
  To: linux-sh

Hi Magnus,

Thank you for the patch.

On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
> 
> As it is today Marzen multiplatform is working, but the legacy code
> is broken. This patch intends to leave multiplatform as-is but do
> a simple one-shot fix to unbreak the legacy code base.
> 
> Without this patch there is no preset delay and the TWD is defined
> both in C code and in DT. Solve the delay by invoking shmobile_init_delay()
> and simply exclude the TWD DT instance in case of building for legacy.
> 
> Marzen legacy code will be removed in the near future, but until then
> we may as well avoid breaking it.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Built on top of renesas-devel-20150629-v4.1
> 
>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> +++ work/arch/arm/boot/dts/r8a7779.dtsi	2015-07-06 15:42:28.182366518 +0900
> @@ -62,7 +62,7 @@
>  		reg = <0xf0001000 0x1000>,
>  		      <0xf0000100 0x100>;
>  	};
> -
> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
> case */ timer@f0000600 {
>  		compatible = "arm,cortex-a9-twd-timer";
>  		reg = <0xf0000600 0x20>;
> @@ -70,7 +70,7 @@
>  			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>  		clocks = <&cpg_clocks R8A7779_CLK_ZS>;
>  	};
> -
> +#endif

DT should really not depend on kernel configuration. Can't you disable the 
device from legacy board/SoC code instead ?

>  	gpio0: gpio@ffc40000 {
>  		compatible = "renesas,gpio-r8a7779", "renesas,gpio-rcar";
>  		reg = <0xffc40000 0x2c>;
> --- 0001/arch/arm/mach-shmobile/setup-r8a7779.c
> +++ work/arch/arm/mach-shmobile/setup-r8a7779.c	2015-07-06
> 15:43:58.472366518 +0900 @@ -677,6 +677,8 @@ void __init
> r8a7779_add_standard_devices
> 
>  void __init r8a7779_add_early_devices(void)
>  {
> +	shmobile_init_delay();
> +
>  	early_platform_add_devices(r8a7779_early_devices,
>  				   ARRAY_SIZE(r8a7779_early_devices));

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
  2015-07-06  7:41 ` Simon Horman
  2015-07-06  7:53 ` Laurent Pinchart
@ 2015-07-06  7:58 ` Magnus Damm
  2015-07-06  7:59 ` Laurent Pinchart
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Magnus Damm @ 2015-07-06  7:58 UTC (permalink / raw)
  To: linux-sh

Hi Laurent,

On Mon, Jul 6, 2015 at 4:53 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
>>
>> As it is today Marzen multiplatform is working, but the legacy code
>> is broken. This patch intends to leave multiplatform as-is but do
>> a simple one-shot fix to unbreak the legacy code base.
>>
>> Without this patch there is no preset delay and the TWD is defined
>> both in C code and in DT. Solve the delay by invoking shmobile_init_delay()
>> and simply exclude the TWD DT instance in case of building for legacy.
>>
>> Marzen legacy code will be removed in the near future, but until then
>> we may as well avoid breaking it.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Built on top of renesas-devel-20150629-v4.1
>>
>>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
>>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2015-07-06 15:42:28.182366518 +0900
>> @@ -62,7 +62,7 @@
>>               reg = <0xf0001000 0x1000>,
>>                     <0xf0000100 0x100>;
>>       };
>> -
>> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
>> case */ timer@f0000600 {
>>               compatible = "arm,cortex-a9-twd-timer";
>>               reg = <0xf0000600 0x20>;
>> @@ -70,7 +70,7 @@
>>                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>>               clocks = <&cpg_clocks R8A7779_CLK_ZS>;
>>       };
>> -
>> +#endif
>
> DT should really not depend on kernel configuration. Can't you disable the
> device from legacy board/SoC code instead ?

I agree about that dependency issue, and it may be quite possible to
do something at runtime.

At this time I feel it becomes slightly more advanced than I initially
had hoped for. I would of course prefer to remove the legacy rather
soon, and once we do that we can simply get rid of that ugly ifdef.

Anyway, I'll have a look at this tomorrow.

Thanks,

/ magnus

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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
                   ` (2 preceding siblings ...)
  2015-07-06  7:58 ` Magnus Damm
@ 2015-07-06  7:59 ` Laurent Pinchart
  2015-07-06  8:07 ` Geert Uytterhoeven
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-07-06  7:59 UTC (permalink / raw)
  To: linux-sh

Hi Magnus,

On Monday 06 July 2015 16:58:12 Magnus Damm wrote:
> On Mon, Jul 6, 2015 at 4:53 PM, Laurent Pinchart wrote:
> > On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
> >> 
> >> As it is today Marzen multiplatform is working, but the legacy code
> >> is broken. This patch intends to leave multiplatform as-is but do
> >> a simple one-shot fix to unbreak the legacy code base.
> >> 
> >> Without this patch there is no preset delay and the TWD is defined
> >> both in C code and in DT. Solve the delay by invoking
> >> shmobile_init_delay()
> >> and simply exclude the TWD DT instance in case of building for legacy.
> >> 
> >> Marzen legacy code will be removed in the near future, but until then
> >> we may as well avoid breaking it.
> >> 
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >> 
> >>  Built on top of renesas-devel-20150629-v4.1
> >>  
> >>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
> >>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> >> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2015-07-06
> >> 15:42:28.182366518 +0900 @@ -62,7 +62,7 @@
> >>               reg = <0xf0001000 0x1000>,
> >>                     <0xf0000100 0x100>;
> >>       };
> >> -
> >> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
> >> case */
> >>       timer@f0000600 {
> >>               compatible = "arm,cortex-a9-twd-timer";
> >>               reg = <0xf0000600 0x20>;
> >> @@ -70,7 +70,7 @@
> >>                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> >>               clocks = <&cpg_clocks R8A7779_CLK_ZS>;
> >>       };
> >> -
> >> +#endif
> > 
> > DT should really not depend on kernel configuration. Can't you disable the
> > device from legacy board/SoC code instead ?
> 
> I agree about that dependency issue, and it may be quite possible to
> do something at runtime.
> 
> At this time I feel it becomes slightly more advanced than I initially
> had hoped for. I would of course prefer to remove the legacy rather
> soon, and once we do that we can simply get rid of that ugly ifdef.

That would of course be my preferred solution too :-)

> Anyway, I'll have a look at this tomorrow.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
                   ` (3 preceding siblings ...)
  2015-07-06  7:59 ` Laurent Pinchart
@ 2015-07-06  8:07 ` Geert Uytterhoeven
  2015-07-06  8:23 ` Laurent Pinchart
  2015-07-06 11:08 ` Magnus Damm
  6 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-07-06  8:07 UTC (permalink / raw)
  To: linux-sh

On Mon, Jul 6, 2015 at 9:53 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
>>
>> As it is today Marzen multiplatform is working, but the legacy code
>> is broken. This patch intends to leave multiplatform as-is but do
>> a simple one-shot fix to unbreak the legacy code base.
>>
>> Without this patch there is no preset delay and the TWD is defined
>> both in C code and in DT. Solve the delay by invoking shmobile_init_delay()
>> and simply exclude the TWD DT instance in case of building for legacy.
>>
>> Marzen legacy code will be removed in the near future, but until then
>> we may as well avoid breaking it.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Built on top of renesas-devel-20150629-v4.1
>>
>>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
>>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2015-07-06 15:42:28.182366518 +0900
>> @@ -62,7 +62,7 @@
>>               reg = <0xf0001000 0x1000>,
>>                     <0xf0000100 0x100>;
>>       };
>> -
>> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
>> case */ timer@f0000600 {
>>               compatible = "arm,cortex-a9-twd-timer";
>>               reg = <0xf0000600 0x20>;
>> @@ -70,7 +70,7 @@
>>                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>>               clocks = <&cpg_clocks R8A7779_CLK_ZS>;
>>       };
>> -
>> +#endif

I was wondering how we did it for sh73a0/kzm9g, as I remember the TWD in
DT was incompatible with kzm9g-reference, but it didn't cause problems on
kzm9g-legacy.

Turned out we still had separate DTSes for -legacy and -reference at that
time, cfr.
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324219.html

> DT should really not depend on kernel configuration. Can't you disable the
> device from legacy board/SoC code instead ?

You mean setting its status to "disabled", before CLOCKSOURCE_OF_DECLARE()
is processed, i.e. before clocksource_of_init()?

However, I believe clocksource_of_init() isn't called by
arch/arm/kernel/time.c:time_init() on marzen-legacy, as board-marzen.c
populates .init_time(), and board-marzen.c doesn't call it itself.

board-marzen-reference.c does call clocksource_of_init() from it's .init_time()
callback.

Hence am I missing something, or is marzen-legacy fine, and is only
marzen-reference (which is "legacy" in some sense, too) broken?
But you mentioned marzen_defconfig, which is for marzen-legacy?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
                   ` (4 preceding siblings ...)
  2015-07-06  8:07 ` Geert Uytterhoeven
@ 2015-07-06  8:23 ` Laurent Pinchart
  2015-07-06 11:08 ` Magnus Damm
  6 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-07-06  8:23 UTC (permalink / raw)
  To: linux-sh

Hi Geert,

On Monday 06 July 2015 10:07:35 Geert Uytterhoeven wrote:
> On Mon, Jul 6, 2015 at 9:53 AM, Laurent Pinchart wrote:
> > On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
> >> 
> >> As it is today Marzen multiplatform is working, but the legacy code
> >> is broken. This patch intends to leave multiplatform as-is but do
> >> a simple one-shot fix to unbreak the legacy code base.
> >> 
> >> Without this patch there is no preset delay and the TWD is defined
> >> both in C code and in DT. Solve the delay by invoking
> >> shmobile_init_delay()
> >> and simply exclude the TWD DT instance in case of building for legacy.
> >> 
> >> Marzen legacy code will be removed in the near future, but until then
> >> we may as well avoid breaking it.
> >> 
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >> 
> >>  Built on top of renesas-devel-20150629-v4.1
> >>  
> >>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
> >>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> >> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2015-07-06
> >> 15:42:28.182366518 +0900 @@ -62,7 +62,7 @@
> >>               reg = <0xf0001000 0x1000>,
> >>                     <0xf0000100 0x100>;
> >>       };
> >> -
> >> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
> >> case */
> >>       timer@f0000600 {
> >>               compatible = "arm,cortex-a9-twd-timer";
> >>               reg = <0xf0000600 0x20>;
> >> @@ -70,7 +70,7 @@
> >>                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> >>               clocks = <&cpg_clocks R8A7779_CLK_ZS>;
> >>       };
> >> -
> >> +#endif
> 
> I was wondering how we did it for sh73a0/kzm9g, as I remember the TWD in
> DT was incompatible with kzm9g-reference, but it didn't cause problems on
> kzm9g-legacy.
> 
> Turned out we still had separate DTSes for -legacy and -reference at that
> time, cfr.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324219.h
> tml
>
> > DT should really not depend on kernel configuration. Can't you disable the
> > device from legacy board/SoC code instead ?
> 
> You mean setting its status to "disabled", before CLOCKSOURCE_OF_DECLARE()
> is processed, i.e. before clocksource_of_init()?

Something like that yes. I haven't checked what exactly is possible, but I'd 
prefer adding hacks to legacy C code that will soon be removed than to DT 
filed that we will keep.

> However, I believe clocksource_of_init() isn't called by
> arch/arm/kernel/time.c:time_init() on marzen-legacy, as board-marzen.c
> populates .init_time(), and board-marzen.c doesn't call it itself.
> 
> board-marzen-reference.c does call clocksource_of_init() from it's
> .init_time() callback.
> 
> Hence am I missing something, or is marzen-legacy fine, and is only
> marzen-reference (which is "legacy" in some sense, too) broken?
> But you mentioned marzen_defconfig, which is for marzen-legacy?

I'll let Magnus comment on that.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix
  2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
                   ` (5 preceding siblings ...)
  2015-07-06  8:23 ` Laurent Pinchart
@ 2015-07-06 11:08 ` Magnus Damm
  6 siblings, 0 replies; 8+ messages in thread
From: Magnus Damm @ 2015-07-06 11:08 UTC (permalink / raw)
  To: linux-sh

Hi Laurent and Geert,

On Mon, Jul 6, 2015 at 5:23 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Geert,
>
> On Monday 06 July 2015 10:07:35 Geert Uytterhoeven wrote:
>> On Mon, Jul 6, 2015 at 9:53 AM, Laurent Pinchart wrote:
>> > On Monday 06 July 2015 16:02:16 Magnus Damm wrote:
>> >> From: Magnus Damm <damm+renesas@opensource.se>
>> >>
>> >> Unbreak the r8a7779 Marzen legacy code provided by marzen_defconfig.
>> >>
>> >> As it is today Marzen multiplatform is working, but the legacy code
>> >> is broken. This patch intends to leave multiplatform as-is but do
>> >> a simple one-shot fix to unbreak the legacy code base.
>> >>
>> >> Without this patch there is no preset delay and the TWD is defined
>> >> both in C code and in DT. Solve the delay by invoking
>> >> shmobile_init_delay()
>> >> and simply exclude the TWD DT instance in case of building for legacy.
>> >>
>> >> Marzen legacy code will be removed in the near future, but until then
>> >> we may as well avoid breaking it.
>> >>
>> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> >> ---
>> >>
>> >>  Built on top of renesas-devel-20150629-v4.1
>> >>
>> >>  arch/arm/boot/dts/r8a7779.dtsi         |    4 ++--
>> >>  arch/arm/mach-shmobile/setup-r8a7779.c |    2 ++
>> >>  2 files changed, 4 insertions(+), 2 deletions(-)
>> >>
>> >> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>> >> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2015-07-06
>> >> 15:42:28.182366518 +0900 @@ -62,7 +62,7 @@
>> >>               reg = <0xf0001000 0x1000>,
>> >>                     <0xf0000100 0x100>;
>> >>       };
>> >> -
>> >> +#ifndef CONFIG_ARCH_SHMOBILE_LEGACY /* TWD defined in C code for legacy
>> >> case */
>> >>       timer@f0000600 {
>> >>               compatible = "arm,cortex-a9-twd-timer";
>> >>               reg = <0xf0000600 0x20>;
>> >> @@ -70,7 +70,7 @@
>> >>                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>> >>               clocks = <&cpg_clocks R8A7779_CLK_ZS>;
>> >>       };
>> >> -
>> >> +#endif
>>
>> I was wondering how we did it for sh73a0/kzm9g, as I remember the TWD in
>> DT was incompatible with kzm9g-reference, but it didn't cause problems on
>> kzm9g-legacy.
>>
>> Turned out we still had separate DTSes for -legacy and -reference at that
>> time, cfr.
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324219.h
>> tml

Right, the r8a7779 TWD DT bits were basically merged prematurely and
we did not keep track of the dependencies. It seems that only bits and
pieces get merged now and then, and in such case no one seems to keep
track of the order.

>> > DT should really not depend on kernel configuration. Can't you disable the
>> > device from legacy board/SoC code instead ?
>>
>> You mean setting its status to "disabled", before CLOCKSOURCE_OF_DECLARE()
>> is processed, i.e. before clocksource_of_init()?
>
> Something like that yes. I haven't checked what exactly is possible, but I'd
> prefer adding hacks to legacy C code that will soon be removed than to DT
> filed that we will keep.

I agree with Laurent that adding some code to the legacy bits to fix
them is much better than polluting code that will stay around. I
recall parsing DT device data in the board staging at some point, and
it may possible to simply disable DT devices at run time.

>> However, I believe clocksource_of_init() isn't called by
>> arch/arm/kernel/time.c:time_init() on marzen-legacy, as board-marzen.c
>> populates .init_time(), and board-marzen.c doesn't call it itself.

I need to look into detail about this, but if board-marzen.c never
calls clocksource_of_init() then it may be because we historically did
not use to need to call clocksource_of_init() to get the TWD going. I
think only the DT case requires that bit, but I'm not sure.

>> board-marzen-reference.c does call clocksource_of_init() from it's
>> .init_time() callback.

Right, that' inline with what we do (or at least should do) for all
multiplatform SoCs.

>> Hence am I missing something, or is marzen-legacy fine, and is only
>> marzen-reference (which is "legacy" in some sense, too) broken?
>> But you mentioned marzen_defconfig, which is for marzen-legacy?
>
> I'll let Magnus comment on that.

For this patch I target marzen_defconfig that is tied together with
board-marzen.c.

What I can tell r8a7779 multiplatform (including
board-marzen-reference.c) is fine. To avoid this kind of headache it
would have been nice to merge the TWD DT bits after removing the
legacy code.

Cheers,

/ magnus

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

end of thread, other threads:[~2015-07-06 11:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-06  6:59 [PATCH] ARM: shmobile: r8a7779 Marzen legacy fix Magnus Damm
2015-07-06  7:41 ` Simon Horman
2015-07-06  7:53 ` Laurent Pinchart
2015-07-06  7:58 ` Magnus Damm
2015-07-06  7:59 ` Laurent Pinchart
2015-07-06  8:07 ` Geert Uytterhoeven
2015-07-06  8:23 ` Laurent Pinchart
2015-07-06 11:08 ` Magnus Damm

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.