* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
@ 2015-05-06 4:20 ` Laurent Pinchart
2015-05-12 4:06 ` Simon Horman
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Laurent Pinchart @ 2015-05-06 4:20 UTC (permalink / raw)
To: linux-sh
Hi Ulrich,
Thank you for the patch.
On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> Minimal support without power management or SMP.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> arch/arm/mach-shmobile/Kconfig | 4 ++++
> arch/arm/mach-shmobile/Makefile | 1 +
> arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++++++++++
> 3 files changed, 38 insertions(+)
> create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
>
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 0fb4842..140b696 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -80,6 +80,10 @@ config ARCH_R8A7791
> select ARCH_RCAR_GEN2
> select I2C
>
> +config ARCH_R8A7793
> + bool "R-Car M2-N (R8A7793)"
> + select ARCH_RCAR_GEN2
> +
> config ARCH_R8A7794
> bool "R-Car E2 (R8A77940)"
> select ARCH_RCAR_GEN2
> diff --git a/arch/arm/mach-shmobile/Makefile
> b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> --- a/arch/arm/mach-shmobile/Makefile
> +++ b/arch/arm/mach-shmobile/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> index 0000000..1d2825c
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> @@ -0,0 +1,33 @@
> +/*
> + * r8a7793 processor support
> + *
> + * Copyright (C) 2015 Ulrich Hecht
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/init.h>
> +#include <asm/mach/arch.h>
> +
> +#include "common.h"
> +#include "rcar-gen2.h"
> +
> +static const char *r8a7793_boards_compat_dt[] __initconst = {
> + "renesas,r8a7793",
> + NULL,
> +};
> +
> +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device Tree)")
> + .init_early = shmobile_init_delay,
> + .init_time = rcar_gen2_timer_init,
> + .init_late = shmobile_init_late,
> + .reserve = rcar_gen2_reserve,
> + .dt_compat = r8a7793_boards_compat_dt,
> +MACHINE_END
How about moving that to setup-rcar-gen2.c instead ? There's nothing specific
to r8a7793 here. We could even introduce a "renesas,rcar-gen2" DT compatible
entry and match on that instead of "renesas,r8a7793" (which should of course
be specified as well in DT). You could also add "renesas,r8a7794" to the
setup-rcar-gen2.c compat list and remove setup-r8a7794.c.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
2015-05-06 4:20 ` Laurent Pinchart
@ 2015-05-12 4:06 ` Simon Horman
2015-05-12 6:51 ` Laurent Pinchart
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2015-05-12 4:06 UTC (permalink / raw)
To: linux-sh
Hi Laurent,
On Wed, May 06, 2015 at 07:20:05AM +0300, Laurent Pinchart wrote:
> Hi Ulrich,
>
> Thank you for the patch.
>
> On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> > Minimal support without power management or SMP.
> >
> > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > ---
> > arch/arm/mach-shmobile/Kconfig | 4 ++++
> > arch/arm/mach-shmobile/Makefile | 1 +
> > arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++++++++++
> > 3 files changed, 38 insertions(+)
> > create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
> >
> > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> > index 0fb4842..140b696 100644
> > --- a/arch/arm/mach-shmobile/Kconfig
> > +++ b/arch/arm/mach-shmobile/Kconfig
> > @@ -80,6 +80,10 @@ config ARCH_R8A7791
> > select ARCH_RCAR_GEN2
> > select I2C
> >
> > +config ARCH_R8A7793
> > + bool "R-Car M2-N (R8A7793)"
> > + select ARCH_RCAR_GEN2
> > +
> > config ARCH_R8A7794
> > bool "R-Car E2 (R8A77940)"
> > select ARCH_RCAR_GEN2
> > diff --git a/arch/arm/mach-shmobile/Makefile
> > b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> > --- a/arch/arm/mach-shmobile/Makefile
> > +++ b/arch/arm/mach-shmobile/Makefile
> > @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> > obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> > obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> > obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> > +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> > obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> > obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> > obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> > diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> > b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> > index 0000000..1d2825c
> > --- /dev/null
> > +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> > @@ -0,0 +1,33 @@
> > +/*
> > + * r8a7793 processor support
> > + *
> > + * Copyright (C) 2015 Ulrich Hecht
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <asm/mach/arch.h>
> > +
> > +#include "common.h"
> > +#include "rcar-gen2.h"
> > +
> > +static const char *r8a7793_boards_compat_dt[] __initconst = {
> > + "renesas,r8a7793",
> > + NULL,
> > +};
> > +
> > +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device Tree)")
> > + .init_early = shmobile_init_delay,
> > + .init_time = rcar_gen2_timer_init,
> > + .init_late = shmobile_init_late,
> > + .reserve = rcar_gen2_reserve,
> > + .dt_compat = r8a7793_boards_compat_dt,
> > +MACHINE_END
>
> How about moving that to setup-rcar-gen2.c instead ? There's nothing specific
> to r8a7793 here. We could even introduce a "renesas,rcar-gen2" DT compatible
> entry and match on that instead of "renesas,r8a7793" (which should of course
> be specified as well in DT). You could also add "renesas,r8a7794" to the
> setup-rcar-gen2.c compat list and remove setup-r8a7794.c.
I believe there are two issues here:
1. Consolidating code
From my point of view I think this is an excellent idea.
However I would rather add this patch as-is and then
address consolidation later.
This avoids blocking on consolidation and I believe we have
a pretty good record of coming back and doing consolidation
(at least in arch/arm/mach-shmobile/) that has been suggested
as a result of adding support for new SoCs.
2. A "renesas,rcar-gen2" DT compatible entry
This seems to be a separate issue that depends on 1.
I would like to treat it separately.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
2015-05-06 4:20 ` Laurent Pinchart
2015-05-12 4:06 ` Simon Horman
@ 2015-05-12 6:51 ` Laurent Pinchart
2015-05-12 7:19 ` Simon Horman
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Laurent Pinchart @ 2015-05-12 6:51 UTC (permalink / raw)
To: linux-sh
Hi Simon,
On Tuesday 12 May 2015 13:06:11 Simon Horman wrote:
> On Wed, May 06, 2015 at 07:20:05AM +0300, Laurent Pinchart wrote:
> > On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> > > Minimal support without power management or SMP.
> > >
> > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > > ---
> > >
> > > arch/arm/mach-shmobile/Kconfig | 4 ++++
> > > arch/arm/mach-shmobile/Makefile | 1 +
> > > arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++++++
> > > 3 files changed, 38 insertions(+)
> > > create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
> > >
> > > diff --git a/arch/arm/mach-shmobile/Kconfig
> > > b/arch/arm/mach-shmobile/Kconfig index 0fb4842..140b696 100644
> > > --- a/arch/arm/mach-shmobile/Kconfig
> > > +++ b/arch/arm/mach-shmobile/Kconfig
> > > @@ -80,6 +80,10 @@ config ARCH_R8A7791
> > > select ARCH_RCAR_GEN2
> > > select I2C
> > >
> > > +config ARCH_R8A7793
> > > + bool "R-Car M2-N (R8A7793)"
> > > + select ARCH_RCAR_GEN2
> > > +
> > > config ARCH_R8A7794
> > > bool "R-Car E2 (R8A77940)"
> > >
> > > diff --git a/arch/arm/mach-shmobile/Makefile
> > > b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> > > --- a/arch/arm/mach-shmobile/Makefile
> > > +++ b/arch/arm/mach-shmobile/Makefile
> > > @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> > > obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> > > obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> > > obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> > > +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> > > obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> > > obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> > > obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> > > diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> > > b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> > > index 0000000..1d2825c
> > > --- /dev/null
> > > +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> > > @@ -0,0 +1,33 @@
> > > +/*
> > > + * r8a7793 processor support
> > > + *
> > > + * Copyright (C) 2015 Ulrich Hecht
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; version 2 of the License.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#include <linux/init.h>
> > > +#include <asm/mach/arch.h>
> > > +
> > > +#include "common.h"
> > > +#include "rcar-gen2.h"
> > > +
> > > +static const char *r8a7793_boards_compat_dt[] __initconst = {
> > > + "renesas,r8a7793",
> > > + NULL,
> > > +};
> > > +
> > > +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device Tree)")
> > > + .init_early = shmobile_init_delay,
> > > + .init_time = rcar_gen2_timer_init,
> > > + .init_late = shmobile_init_late,
> > > + .reserve = rcar_gen2_reserve,
> > > + .dt_compat = r8a7793_boards_compat_dt,
> > > +MACHINE_END
> >
> > How about moving that to setup-rcar-gen2.c instead ? There's nothing
> > specific to r8a7793 here. We could even introduce a "renesas,rcar-gen2"
> > DT compatible entry and match on that instead of "renesas,r8a7793" (which
> > should of course be specified as well in DT). You could also add
> > "renesas,r8a7794" to the setup-rcar-gen2.c compat list and remove
> > setup-r8a7794.c.
>
> I believe there are two issues here:
>
> 1. Consolidating code
>
> From my point of view I think this is an excellent idea.
> However I would rather add this patch as-is and then
> address consolidation later.
>
> This avoids blocking on consolidation and I believe we have
> a pretty good record of coming back and doing consolidation
> (at least in arch/arm/mach-shmobile/) that has been suggested
> as a result of adding support for new SoCs.
I'm fine with that, as long as we use CPU_METHOD_OF_DECLARE() in patch 10/15,
otherwise it would block any future consolidation.
> 2. A "renesas,rcar-gen2" DT compatible entry
>
> This seems to be a separate issue that depends on 1.
> I would like to treat it separately.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
` (2 preceding siblings ...)
2015-05-12 6:51 ` Laurent Pinchart
@ 2015-05-12 7:19 ` Simon Horman
2015-05-12 7:27 ` Laurent Pinchart
2015-05-12 7:29 ` Simon Horman
5 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2015-05-12 7:19 UTC (permalink / raw)
To: linux-sh
On Tue, May 12, 2015 at 09:51:26AM +0300, Laurent Pinchart wrote:
> Hi Simon,
>
> On Tuesday 12 May 2015 13:06:11 Simon Horman wrote:
> > On Wed, May 06, 2015 at 07:20:05AM +0300, Laurent Pinchart wrote:
> > > On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> > > > Minimal support without power management or SMP.
> > > >
> > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > > > ---
> > > >
> > > > arch/arm/mach-shmobile/Kconfig | 4 ++++
> > > > arch/arm/mach-shmobile/Makefile | 1 +
> > > > arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++++++
> > > > 3 files changed, 38 insertions(+)
> > > > create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
> > > >
> > > > diff --git a/arch/arm/mach-shmobile/Kconfig
> > > > b/arch/arm/mach-shmobile/Kconfig index 0fb4842..140b696 100644
> > > > --- a/arch/arm/mach-shmobile/Kconfig
> > > > +++ b/arch/arm/mach-shmobile/Kconfig
> > > > @@ -80,6 +80,10 @@ config ARCH_R8A7791
> > > > select ARCH_RCAR_GEN2
> > > > select I2C
> > > >
> > > > +config ARCH_R8A7793
> > > > + bool "R-Car M2-N (R8A7793)"
> > > > + select ARCH_RCAR_GEN2
> > > > +
> > > > config ARCH_R8A7794
> > > > bool "R-Car E2 (R8A77940)"
> > > >
> > > > diff --git a/arch/arm/mach-shmobile/Makefile
> > > > b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> > > > --- a/arch/arm/mach-shmobile/Makefile
> > > > +++ b/arch/arm/mach-shmobile/Makefile
> > > > @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> > > > obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> > > > obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> > > > obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> > > > +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> > > > obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> > > > obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> > > > obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> > > > diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> > > > index 0000000..1d2825c
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > @@ -0,0 +1,33 @@
> > > > +/*
> > > > + * r8a7793 processor support
> > > > + *
> > > > + * Copyright (C) 2015 Ulrich Hecht
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; version 2 of the License.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + */
> > > > +
> > > > +#include <linux/init.h>
> > > > +#include <asm/mach/arch.h>
> > > > +
> > > > +#include "common.h"
> > > > +#include "rcar-gen2.h"
> > > > +
> > > > +static const char *r8a7793_boards_compat_dt[] __initconst = {
> > > > + "renesas,r8a7793",
> > > > + NULL,
> > > > +};
> > > > +
> > > > +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device Tree)")
> > > > + .init_early = shmobile_init_delay,
> > > > + .init_time = rcar_gen2_timer_init,
> > > > + .init_late = shmobile_init_late,
> > > > + .reserve = rcar_gen2_reserve,
> > > > + .dt_compat = r8a7793_boards_compat_dt,
> > > > +MACHINE_END
> > >
> > > How about moving that to setup-rcar-gen2.c instead ? There's nothing
> > > specific to r8a7793 here. We could even introduce a "renesas,rcar-gen2"
> > > DT compatible entry and match on that instead of "renesas,r8a7793" (which
> > > should of course be specified as well in DT). You could also add
> > > "renesas,r8a7794" to the setup-rcar-gen2.c compat list and remove
> > > setup-r8a7794.c.
> >
> > I believe there are two issues here:
> >
> > 1. Consolidating code
> >
> > From my point of view I think this is an excellent idea.
> > However I would rather add this patch as-is and then
> > address consolidation later.
> >
> > This avoids blocking on consolidation and I believe we have
> > a pretty good record of coming back and doing consolidation
> > (at least in arch/arm/mach-shmobile/) that has been suggested
> > as a result of adding support for new SoCs.
>
> I'm fine with that, as long as we use CPU_METHOD_OF_DECLARE() in patch 10/15,
> otherwise it would block any future consolidation.
I'm not sure that I follow. Could you expand on this a little?
> > 2. A "renesas,rcar-gen2" DT compatible entry
> >
> > This seems to be a separate issue that depends on 1.
> > I would like to treat it separately.
>
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
` (3 preceding siblings ...)
2015-05-12 7:19 ` Simon Horman
@ 2015-05-12 7:27 ` Laurent Pinchart
2015-05-12 7:29 ` Simon Horman
5 siblings, 0 replies; 7+ messages in thread
From: Laurent Pinchart @ 2015-05-12 7:27 UTC (permalink / raw)
To: linux-sh
Hi Simon,
On Tuesday 12 May 2015 16:19:33 Simon Horman wrote:
> On Tue, May 12, 2015 at 09:51:26AM +0300, Laurent Pinchart wrote:
> > On Tuesday 12 May 2015 13:06:11 Simon Horman wrote:
> > > On Wed, May 06, 2015 at 07:20:05AM +0300, Laurent Pinchart wrote:
> > > > On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> > > > > Minimal support without power management or SMP.
> > > > >
> > > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > > > > ---
> > > > >
> > > > > arch/arm/mach-shmobile/Kconfig | 4 ++++
> > > > > arch/arm/mach-shmobile/Makefile | 1 +
> > > > > arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++
> > > > > 3 files changed, 38 insertions(+)
> > > > > create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
> > > > >
> > > > > diff --git a/arch/arm/mach-shmobile/Kconfig
> > > > > b/arch/arm/mach-shmobile/Kconfig index 0fb4842..140b696 100644
> > > > > --- a/arch/arm/mach-shmobile/Kconfig
> > > > > +++ b/arch/arm/mach-shmobile/Kconfig
> > > > > @@ -80,6 +80,10 @@ config ARCH_R8A7791
> > > > > select ARCH_RCAR_GEN2
> > > > > select I2C
> > > > >
> > > > > +config ARCH_R8A7793
> > > > > + bool "R-Car M2-N (R8A7793)"
> > > > > + select ARCH_RCAR_GEN2
> > > > > +
> > > > > config ARCH_R8A7794
> > > > > bool "R-Car E2 (R8A77940)"
> > > > > diff --git a/arch/arm/mach-shmobile/Makefile
> > > > > b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> > > > > --- a/arch/arm/mach-shmobile/Makefile
> > > > > +++ b/arch/arm/mach-shmobile/Makefile
> > > > > @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> > > > > obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> > > > > obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> > > > > obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> > > > > +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> > > > > obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> > > > > obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> > > > > obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> > > > > diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > > b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> > > > > index 0000000..1d2825c
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > > @@ -0,0 +1,33 @@
> > > > > +/*
> > > > > + * r8a7793 processor support
> > > > > + *
> > > > > + * Copyright (C) 2015 Ulrich Hecht
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or
> > > > > modify
> > > > > + * it under the terms of the GNU General Public License as
> > > > > published by
> > > > > + * the Free Software Foundation; version 2 of the License.
> > > > > + *
> > > > > + * This program is distributed in the hope that it will be useful,
> > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > > + * GNU General Public License for more details.
> > > > > + */
> > > > > +
> > > > > +#include <linux/init.h>
> > > > > +#include <asm/mach/arch.h>
> > > > > +
> > > > > +#include "common.h"
> > > > > +#include "rcar-gen2.h"
> > > > > +
> > > > > +static const char *r8a7793_boards_compat_dt[] __initconst = {
> > > > > + "renesas,r8a7793",
> > > > > + NULL,
> > > > > +};
> > > > > +
> > > > > +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device
> > > > > Tree)")
> > > > > + .init_early = shmobile_init_delay,
> > > > > + .init_time = rcar_gen2_timer_init,
> > > > > + .init_late = shmobile_init_late,
> > > > > + .reserve = rcar_gen2_reserve,
> > > > > + .dt_compat = r8a7793_boards_compat_dt,
> > > > > +MACHINE_END
> > > >
> > > > How about moving that to setup-rcar-gen2.c instead ? There's nothing
> > > > specific to r8a7793 here. We could even introduce a
> > > > "renesas,rcar-gen2" DT compatible entry and match on that instead of
> > > > "renesas,r8a7793" (which should of course be specified as well in DT).
> > > > You could also add "renesas,r8a7794" to the setup-rcar-gen2.c compat
> > > > list and remove setup-r8a7794.c.
> > >
> > > I believe there are two issues here:
> > >
> > > 1. Consolidating code
> > >
> > > From my point of view I think this is an excellent idea.
> > > However I would rather add this patch as-is and then
> > > address consolidation later.
> > >
> > > This avoids blocking on consolidation and I believe we have
> > > a pretty good record of coming back and doing consolidation
> > > (at least in arch/arm/mach-shmobile/) that has been suggested
> > > as a result of adding support for new SoCs.
> >
> > I'm fine with that, as long as we use CPU_METHOD_OF_DECLARE() in patch
> > 10/15, otherwise it would block any future consolidation.
>
> I'm not sure that I follow. Could you expand on this a little?
Our current SMP implementations plug the SMP ops into the .smp field of the
machine description. This requires one machine description per SoC. Switching
to the CPU_METHOD_OF_DECLARE() method of wiring up the SMP ops would allow
merging all otherwise identical machine descriptions together. However, as
this requires an enable-method property in the cpu node, we can't switch an
existing platform from .smp to CPU_METHOD_OF_DECLARE() without breaking
backward compatibility with older DTs. I personally don't care, but not
everybody seems to agree ;-)
So, if we want to enable later consolidation, we must start using
CPU_METHOD_OF_DECLARE() right away.
> > > 2. A "renesas,rcar-gen2" DT compatible entry
> > >
> > > This seems to be a separate issue that depends on 1.
> > > I would like to treat it separately.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support
2015-04-30 14:18 [PATCH v2 07/15] ARM: shmobile: Basic r8a7793 SoC support Ulrich Hecht
` (4 preceding siblings ...)
2015-05-12 7:27 ` Laurent Pinchart
@ 2015-05-12 7:29 ` Simon Horman
5 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2015-05-12 7:29 UTC (permalink / raw)
To: linux-sh
Hi Laurent,
On Tue, May 12, 2015 at 10:27:51AM +0300, Laurent Pinchart wrote:
> Hi Simon,
>
> On Tuesday 12 May 2015 16:19:33 Simon Horman wrote:
> > On Tue, May 12, 2015 at 09:51:26AM +0300, Laurent Pinchart wrote:
> > > On Tuesday 12 May 2015 13:06:11 Simon Horman wrote:
> > > > On Wed, May 06, 2015 at 07:20:05AM +0300, Laurent Pinchart wrote:
> > > > > On Thursday 30 April 2015 16:18:56 Ulrich Hecht wrote:
> > > > > > Minimal support without power management or SMP.
> > > > > >
> > > > > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > > > > > ---
> > > > > >
> > > > > > arch/arm/mach-shmobile/Kconfig | 4 ++++
> > > > > > arch/arm/mach-shmobile/Makefile | 1 +
> > > > > > arch/arm/mach-shmobile/setup-r8a7793.c | 33 +++++++++++++++++++++++
> > > > > > 3 files changed, 38 insertions(+)
> > > > > > create mode 100644 arch/arm/mach-shmobile/setup-r8a7793.c
> > > > > >
> > > > > > diff --git a/arch/arm/mach-shmobile/Kconfig
> > > > > > b/arch/arm/mach-shmobile/Kconfig index 0fb4842..140b696 100644
> > > > > > --- a/arch/arm/mach-shmobile/Kconfig
> > > > > > +++ b/arch/arm/mach-shmobile/Kconfig
> > > > > > @@ -80,6 +80,10 @@ config ARCH_R8A7791
> > > > > > select ARCH_RCAR_GEN2
> > > > > > select I2C
> > > > > >
> > > > > > +config ARCH_R8A7793
> > > > > > + bool "R-Car M2-N (R8A7793)"
> > > > > > + select ARCH_RCAR_GEN2
> > > > > > +
> > > > > > config ARCH_R8A7794
> > > > > > bool "R-Car E2 (R8A77940)"
> > > > > > diff --git a/arch/arm/mach-shmobile/Makefile
> > > > > > b/arch/arm/mach-shmobile/Makefile index 89e463d..e4b8fdb 100644
> > > > > > --- a/arch/arm/mach-shmobile/Makefile
> > > > > > +++ b/arch/arm/mach-shmobile/Makefile
> > > > > > @@ -13,6 +13,7 @@ obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
> > > > > > obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
> > > > > > obj-$(CONFIG_ARCH_R8A7790) += setup-r8a7790.o
> > > > > > obj-$(CONFIG_ARCH_R8A7791) += setup-r8a7791.o
> > > > > > +obj-$(CONFIG_ARCH_R8A7793) += setup-r8a7793.o
> > > > > > obj-$(CONFIG_ARCH_R8A7794) += setup-r8a7794.o
> > > > > > obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
> > > > > > obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
> > > > > > diff --git a/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > > > b/arch/arm/mach-shmobile/setup-r8a7793.c new file mode 100644
> > > > > > index 0000000..1d2825c
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/mach-shmobile/setup-r8a7793.c
> > > > > > @@ -0,0 +1,33 @@
> > > > > > +/*
> > > > > > + * r8a7793 processor support
> > > > > > + *
> > > > > > + * Copyright (C) 2015 Ulrich Hecht
> > > > > > + *
> > > > > > + * This program is free software; you can redistribute it and/or
> > > > > > modify
> > > > > > + * it under the terms of the GNU General Public License as
> > > > > > published by
> > > > > > + * the Free Software Foundation; version 2 of the License.
> > > > > > + *
> > > > > > + * This program is distributed in the hope that it will be useful,
> > > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > > > + * GNU General Public License for more details.
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/init.h>
> > > > > > +#include <asm/mach/arch.h>
> > > > > > +
> > > > > > +#include "common.h"
> > > > > > +#include "rcar-gen2.h"
> > > > > > +
> > > > > > +static const char *r8a7793_boards_compat_dt[] __initconst = {
> > > > > > + "renesas,r8a7793",
> > > > > > + NULL,
> > > > > > +};
> > > > > > +
> > > > > > +DT_MACHINE_START(R8A7793_DT, "Generic R8A7793 (Flattened Device
> > > > > > Tree)")
> > > > > > + .init_early = shmobile_init_delay,
> > > > > > + .init_time = rcar_gen2_timer_init,
> > > > > > + .init_late = shmobile_init_late,
> > > > > > + .reserve = rcar_gen2_reserve,
> > > > > > + .dt_compat = r8a7793_boards_compat_dt,
> > > > > > +MACHINE_END
> > > > >
> > > > > How about moving that to setup-rcar-gen2.c instead ? There's nothing
> > > > > specific to r8a7793 here. We could even introduce a
> > > > > "renesas,rcar-gen2" DT compatible entry and match on that instead of
> > > > > "renesas,r8a7793" (which should of course be specified as well in DT).
> > > > > You could also add "renesas,r8a7794" to the setup-rcar-gen2.c compat
> > > > > list and remove setup-r8a7794.c.
> > > >
> > > > I believe there are two issues here:
> > > >
> > > > 1. Consolidating code
> > > >
> > > > From my point of view I think this is an excellent idea.
> > > > However I would rather add this patch as-is and then
> > > > address consolidation later.
> > > >
> > > > This avoids blocking on consolidation and I believe we have
> > > > a pretty good record of coming back and doing consolidation
> > > > (at least in arch/arm/mach-shmobile/) that has been suggested
> > > > as a result of adding support for new SoCs.
> > >
> > > I'm fine with that, as long as we use CPU_METHOD_OF_DECLARE() in patch
> > > 10/15, otherwise it would block any future consolidation.
> >
> > I'm not sure that I follow. Could you expand on this a little?
>
> Our current SMP implementations plug the SMP ops into the .smp field of the
> machine description. This requires one machine description per SoC. Switching
> to the CPU_METHOD_OF_DECLARE() method of wiring up the SMP ops would allow
> merging all otherwise identical machine descriptions together. However, as
> this requires an enable-method property in the cpu node, we can't switch an
> existing platform from .smp to CPU_METHOD_OF_DECLARE() without breaking
> backward compatibility with older DTs. I personally don't care, but not
> everybody seems to agree ;-)
>
> So, if we want to enable later consolidation, we must start using
> CPU_METHOD_OF_DECLARE() right away.
Thanks, that seems like a good idea to me.
^ permalink raw reply [flat|nested] 7+ messages in thread