mbox series

[GIT,PULL] Renesas ARM64 Based SoC SoC Updates for v4.20

Message ID cover.1538127537.git.horms+renesas@verge.net.au
State New
Headers show
Series [GIT,PULL] Renesas ARM64 Based SoC SoC Updates for v4.20 | expand

Pull-request

https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-arm64-soc-for-v4.20

Message

Simon Horman Sept. 28, 2018, 10:21 a.m. UTC
Hi Olof, Hi Kevin, Hi Arnd,

Please consider these Renesas ARM64 based SoC SoC updates for v4.20.


The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:

  Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-arm64-soc-for-v4.20

for you to fetch changes up to 692dce77dfb71b5eaf896d3cdc7ef72f70631b14:

  arm64: Add Renesas R8A774C0 support (2018-09-11 15:50:57 +0200)

----------------------------------------------------------------
Renesas ARM64 Based SoC SoC Updates for v4.20

* Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
* Enable Compare Match Timer (CMT) and Timer Unit (TMU)
  for Renesas SoCs
* Remove no longer needed ARCH_SHMOBILE Kconfig symbol

----------------------------------------------------------------
Biju Das (1):
      arm64: Add Renesas R8A774A1 support

Fabrizio Castro (1):
      arm64: Add Renesas R8A774C0 support

Geert Uytterhoeven (1):
      arm64: renesas: Remove the ARCH_SHMOBILE Kconfig symbol

Sergei Shtylyov (1):
      arm64: enable CMT/TMU support for Renesas SoC

 arch/arm64/Kconfig.platforms | 58 ++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 24 deletions(-)

Comments

Arnd Bergmann Sept. 28, 2018, 8:10 p.m. UTC | #1
On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
<horms+renesas@verge.net.au> wrote:
>
> Hi Olof, Hi Kevin, Hi Arnd,
>
> Please consider these Renesas ARM64 based SoC SoC updates for v4.20.
>
>
> The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:
>
>   Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-arm64-soc-for-v4.20
>
> for you to fetch changes up to 692dce77dfb71b5eaf896d3cdc7ef72f70631b14:
>
>   arm64: Add Renesas R8A774C0 support (2018-09-11 15:50:57 +0200)
>
> ----------------------------------------------------------------
> Renesas ARM64 Based SoC SoC Updates for v4.20
>
> * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
>   for Renesas SoCs
> * Remove no longer needed ARCH_SHMOBILE Kconfig symbol

The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
the newly added symbols for specific SoCs. I see you already have a
couple of those, but the other manufacturers don't.

I think what happened here is that I tried to reject those additions
normally, but I never noticed yours. From what I see in drivers,
you have additional symbols depending on these in clk, pinctrl,
drivers/soc, and the dtb files, so at the very least we can't
just drop the config options, but I'd still like to see this work
more like other platforms.

Any ideas what we can do here?

      Arnd
Geert Uytterhoeven Oct. 2, 2018, 8:30 a.m. UTC | #2
Hi Arnd,

On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> <horms+renesas@verge.net.au> wrote:
> > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> >   for Renesas SoCs
> > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
>
> The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> the newly added symbols for specific SoCs. I see you already have a
> couple of those, but the other manufacturers don't.
>
> I think what happened here is that I tried to reject those additions
> normally, but I never noticed yours. From what I see in drivers,
> you have additional symbols depending on these in clk, pinctrl,
> drivers/soc, and the dtb files, so at the very least we can't
> just drop the config options, but I'd still like to see this work
> more like other platforms.

The main motivation for SoC-specific controls is to avoid including pinctrl
and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
consumes a few tens of KiB, each clock driver a few KiB.
If we remove the SoC-specific ARCH_* controls, the user has to configure
both pinctrl and clock driver selections, thus trading one user-visible
symbol for two new user-visible symbols (except for RZ/A and RZ/N,
where the pinctrl symbols are already visible, to allow reducing kernel
size).
Perhaps that is acceptable?

For the smaller parts in drivers/soc/, removing SoC-specific dependencies
may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
may be used without external RAM, and can run with a few MiB of internal
SRAM, and XIP (with a still out-of-tree patch).
So at least a family-specific dependency is worth to keep.  For now, RZ/A
is arm32 only, though.

Note that on arm64, there are no family-specific Kconfig symbols yet.
We just have ARCH_RENESAS, which is also set on arm32.
So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
(and RZ/G2) SoCs explicitly.
Choosing the latter option means we may need to migrate to the former option
later, once other families appear (e.g. RZ/A SoCs may start using
Cortex-A53 instead of Cortex-A9 cores).

For the DTB files, I believe we can just remove the dependencies, and
always build all DTBs.

What do you think?
Thanks!

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
Arnd Bergmann Oct. 2, 2018, 12:18 p.m. UTC | #3
On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Arnd,
>
> On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > <horms+renesas@verge.net.au> wrote:
> > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > >   for Renesas SoCs
> > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> >
> > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > the newly added symbols for specific SoCs. I see you already have a
> > couple of those, but the other manufacturers don't.
> >
> > I think what happened here is that I tried to reject those additions
> > normally, but I never noticed yours. From what I see in drivers,
> > you have additional symbols depending on these in clk, pinctrl,
> > drivers/soc, and the dtb files, so at the very least we can't
> > just drop the config options, but I'd still like to see this work
> > more like other platforms.
>
> The main motivation for SoC-specific controls is to avoid including pinctrl
> and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> consumes a few tens of KiB, each clock driver a few KiB.
> If we remove the SoC-specific ARCH_* controls, the user has to configure
> both pinctrl and clock driver selections, thus trading one user-visible
> symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> where the pinctrl symbols are already visible, to allow reducing kernel
> size).
> Perhaps that is acceptable?

It's definitely fine with me. I understand that this is less user friendly,
but it does address my concern about consistency between the platforms.

> For the smaller parts in drivers/soc/, removing SoC-specific dependencies
> may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
> may be used without external RAM, and can run with a few MiB of internal
> SRAM, and XIP (with a still out-of-tree patch).
> So at least a family-specific dependency is worth to keep.  For now, RZ/A
> is arm32 only, though.
>
> Note that on arm64, there are no family-specific Kconfig symbols yet.
> We just have ARCH_RENESAS, which is also set on arm32.
> So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
> ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
> in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
> (and RZ/G2) SoCs explicitly.
> Choosing the latter option means we may need to migrate to the former option
> later, once other families appear (e.g. RZ/A SoCs may start using
> Cortex-A53 instead of Cortex-A9 cores).

Maybe introducing a hidden ARCH_RCAR_GEN3 makes sense here, and
that can start out with the same value as ARCH_RENESAS. I think
if we want to add 64-bit RZ/A support later, having a separate top-level
symbol for that is also reasonable: as you explained this is rather
sensitive to memory usage, and that is enough reason to deviate from
what we do elsewhere.

> For the DTB files, I believe we can just remove the dependencies, and
> always build all DTBs.

Yes, that seems best, and consistent with how we handle other
manufacturers.

> What do you think?
> Thanks!
Geert Uytterhoeven Oct. 2, 2018, 12:31 p.m. UTC | #4
Hi Arnd,

On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > <horms+renesas@verge.net.au> wrote:
> > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > >   for Renesas SoCs
> > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > >
> > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > the newly added symbols for specific SoCs. I see you already have a
> > > couple of those, but the other manufacturers don't.
> > >
> > > I think what happened here is that I tried to reject those additions
> > > normally, but I never noticed yours. From what I see in drivers,
> > > you have additional symbols depending on these in clk, pinctrl,
> > > drivers/soc, and the dtb files, so at the very least we can't
> > > just drop the config options, but I'd still like to see this work
> > > more like other platforms.
> >
> > The main motivation for SoC-specific controls is to avoid including pinctrl
> > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > consumes a few tens of KiB, each clock driver a few KiB.
> > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > both pinctrl and clock driver selections, thus trading one user-visible
> > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > where the pinctrl symbols are already visible, to allow reducing kernel
> > size).
> > Perhaps that is acceptable?
>
> It's definitely fine with me. I understand that this is less user friendly,
> but it does address my concern about consistency between the platforms.
>
> > For the smaller parts in drivers/soc/, removing SoC-specific dependencies
> > may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
> > may be used without external RAM, and can run with a few MiB of internal
> > SRAM, and XIP (with a still out-of-tree patch).
> > So at least a family-specific dependency is worth to keep.  For now, RZ/A
> > is arm32 only, though.
> >
> > Note that on arm64, there are no family-specific Kconfig symbols yet.
> > We just have ARCH_RENESAS, which is also set on arm32.
> > So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
> > ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
> > in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
> > (and RZ/G2) SoCs explicitly.
> > Choosing the latter option means we may need to migrate to the former option
> > later, once other families appear (e.g. RZ/A SoCs may start using
> > Cortex-A53 instead of Cortex-A9 cores).
>
> Maybe introducing a hidden ARCH_RCAR_GEN3 makes sense here, and
> that can start out with the same value as ARCH_RENESAS. I think
> if we want to add 64-bit RZ/A support later, having a separate top-level
> symbol for that is also reasonable: as you explained this is rather
> sensitive to memory usage, and that is enough reason to deviate from
> what we do elsewhere.
>
> > For the DTB files, I believe we can just remove the dependencies, and
> > always build all DTBs.
>
> Yes, that seems best, and consistent with how we handle other
> manufacturers.

OK, I'll put it on my list.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Oct. 4, 2018, 7:57 a.m. UTC | #5
Hi Arnd,

On Tue, Oct 2, 2018 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > > <horms+renesas@verge.net.au> wrote:
> > > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > > >   for Renesas SoCs
> > > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > > >
> > > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > > the newly added symbols for specific SoCs. I see you already have a
> > > > couple of those, but the other manufacturers don't.
> > > >
> > > > I think what happened here is that I tried to reject those additions
> > > > normally, but I never noticed yours. From what I see in drivers,
> > > > you have additional symbols depending on these in clk, pinctrl,
> > > > drivers/soc, and the dtb files, so at the very least we can't
> > > > just drop the config options, but I'd still like to see this work
> > > > more like other platforms.
> > >
> > > The main motivation for SoC-specific controls is to avoid including pinctrl
> > > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > > consumes a few tens of KiB, each clock driver a few KiB.
> > > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > > both pinctrl and clock driver selections, thus trading one user-visible
> > > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > > where the pinctrl symbols are already visible, to allow reducing kernel
> > > size).
> > > Perhaps that is acceptable?
> >
> > It's definitely fine with me. I understand that this is less user friendly,
> > but it does address my concern about consistency between the platforms.
> >
> > > For the smaller parts in drivers/soc/, removing SoC-specific dependencies
> > > may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
> > > may be used without external RAM, and can run with a few MiB of internal
> > > SRAM, and XIP (with a still out-of-tree patch).
> > > So at least a family-specific dependency is worth to keep.  For now, RZ/A
> > > is arm32 only, though.
> > >
> > > Note that on arm64, there are no family-specific Kconfig symbols yet.
> > > We just have ARCH_RENESAS, which is also set on arm32.
> > > So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
> > > ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
> > > in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
> > > (and RZ/G2) SoCs explicitly.
> > > Choosing the latter option means we may need to migrate to the former option
> > > later, once other families appear (e.g. RZ/A SoCs may start using
> > > Cortex-A53 instead of Cortex-A9 cores).
> >
> > Maybe introducing a hidden ARCH_RCAR_GEN3 makes sense here, and
> > that can start out with the same value as ARCH_RENESAS. I think
> > if we want to add 64-bit RZ/A support later, having a separate top-level
> > symbol for that is also reasonable: as you explained this is rather
> > sensitive to memory usage, and that is enough reason to deviate from
> > what we do elsewhere.
> >
> > > For the DTB files, I believe we can just remove the dependencies, and
> > > always build all DTBs.
> >
> > Yes, that seems best, and consistent with how we handle other
> > manufacturers.
>
> OK, I'll put it on my list.

Given the current time in the cycle, we can no longer do this for v4.20.
Hence, can you please accept Simon's PR as-is, to enable us to move
forward?

Thanks!

Gr{oetje,eeting}s,

                        Geert
Arnd Bergmann Oct. 4, 2018, 3:36 p.m. UTC | #6
On Thu, Oct 4, 2018 at 9:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Oct 2, 2018 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > > > <horms+renesas@verge.net.au> wrote:
> > > > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > > > >   for Renesas SoCs
> > > > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > > > >
> > > > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > > > the newly added symbols for specific SoCs. I see you already have a
> > > > > couple of those, but the other manufacturers don't.
> > > > >
> > > > > I think what happened here is that I tried to reject those additions
> > > > > normally, but I never noticed yours. From what I see in drivers,
> > > > > you have additional symbols depending on these in clk, pinctrl,
> > > > > drivers/soc, and the dtb files, so at the very least we can't
> > > > > just drop the config options, but I'd still like to see this work
> > > > > more like other platforms.
> > > >
> > > > The main motivation for SoC-specific controls is to avoid including pinctrl
> > > > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > > > consumes a few tens of KiB, each clock driver a few KiB.
> > > > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > > > both pinctrl and clock driver selections, thus trading one user-visible
> > > > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > > > where the pinctrl symbols are already visible, to allow reducing kernel
> > > > size).
> > > > Perhaps that is acceptable?
> > >
> > > It's definitely fine with me. I understand that this is less user friendly,
> > > but it does address my concern about consistency between the platforms.
> > >
> > > > For the smaller parts in drivers/soc/, removing SoC-specific dependencies
> > > > may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
> > > > may be used without external RAM, and can run with a few MiB of internal
> > > > SRAM, and XIP (with a still out-of-tree patch).
> > > > So at least a family-specific dependency is worth to keep.  For now, RZ/A
> > > > is arm32 only, though.
> > > >
> > > > Note that on arm64, there are no family-specific Kconfig symbols yet.
> > > > We just have ARCH_RENESAS, which is also set on arm32.
> > > > So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
> > > > ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
> > > > in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
> > > > (and RZ/G2) SoCs explicitly.
> > > > Choosing the latter option means we may need to migrate to the former option
> > > > later, once other families appear (e.g. RZ/A SoCs may start using
> > > > Cortex-A53 instead of Cortex-A9 cores).
> > >
> > > Maybe introducing a hidden ARCH_RCAR_GEN3 makes sense here, and
> > > that can start out with the same value as ARCH_RENESAS. I think
> > > if we want to add 64-bit RZ/A support later, having a separate top-level
> > > symbol for that is also reasonable: as you explained this is rather
> > > sensitive to memory usage, and that is enough reason to deviate from
> > > what we do elsewhere.
> > >
> > > > For the DTB files, I believe we can just remove the dependencies, and
> > > > always build all DTBs.
> > >
> > > Yes, that seems best, and consistent with how we handle other
> > > manufacturers.
> >
> > OK, I'll put it on my list.
>
> Given the current time in the cycle, we can no longer do this for v4.20.
> Hence, can you please accept Simon's PR as-is, to enable us to move
> forward?

Fair enough, pulled into next/soc now.

      Arnd
Geert Uytterhoeven Oct. 4, 2018, 3:39 p.m. UTC | #7
Hi Arnd,

On Tue, Oct 2, 2018 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > > <horms+renesas@verge.net.au> wrote:
> > > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > > >   for Renesas SoCs
> > > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > > >
> > > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > > the newly added symbols for specific SoCs. I see you already have a
> > > > couple of those, but the other manufacturers don't.
> > > >
> > > > I think what happened here is that I tried to reject those additions
> > > > normally, but I never noticed yours. From what I see in drivers,
> > > > you have additional symbols depending on these in clk, pinctrl,
> > > > drivers/soc, and the dtb files, so at the very least we can't
> > > > just drop the config options, but I'd still like to see this work
> > > > more like other platforms.
> > >
> > > The main motivation for SoC-specific controls is to avoid including pinctrl
> > > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > > consumes a few tens of KiB, each clock driver a few KiB.
> > > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > > both pinctrl and clock driver selections, thus trading one user-visible
> > > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > > where the pinctrl symbols are already visible, to allow reducing kernel
> > > size).
> > > Perhaps that is acceptable?
> >
> > It's definitely fine with me. I understand that this is less user friendly,

Upon closer look, the SoC-specific controls are also used to control compilation
of the SYSC (power domain) tables,  each consuming a few 100 bytes.
So either they become user-visible, too, or always compiled in (depending
on SoC family?).

> > but it does address my concern about consistency between the platforms.

I've just noticed Tegra also has SoC-specific controls, but they're located
in drivers/soc/tegra/Kconfig, not the main arch/arm64/Kconfig.platforms.
Do you consider that a viable alternative?

Thanks again!

Gr{oetje,eeting}s,

                        Geert
Arnd Bergmann Oct. 4, 2018, 3:50 p.m. UTC | #8
On Thu, Oct 4, 2018 at 5:39 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Oct 2, 2018 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > > > <horms+renesas@verge.net.au> wrote:
> > > > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > > > >   for Renesas SoCs
> > > > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > > > >
> > > > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > > > the newly added symbols for specific SoCs. I see you already have a
> > > > > couple of those, but the other manufacturers don't.
> > > > >
> > > > > I think what happened here is that I tried to reject those additions
> > > > > normally, but I never noticed yours. From what I see in drivers,
> > > > > you have additional symbols depending on these in clk, pinctrl,
> > > > > drivers/soc, and the dtb files, so at the very least we can't
> > > > > just drop the config options, but I'd still like to see this work
> > > > > more like other platforms.
> > > >
> > > > The main motivation for SoC-specific controls is to avoid including pinctrl
> > > > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > > > consumes a few tens of KiB, each clock driver a few KiB.
> > > > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > > > both pinctrl and clock driver selections, thus trading one user-visible
> > > > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > > > where the pinctrl symbols are already visible, to allow reducing kernel
> > > > size).
> > > > Perhaps that is acceptable?
> > >
> > > It's definitely fine with me. I understand that this is less user friendly,
>
> Upon closer look, the SoC-specific controls are also used to control compilation
> of the SYSC (power domain) tables,  each consuming a few 100 bytes.
> So either they become user-visible, too, or always compiled in (depending
> on SoC family?).
>
> > > but it does address my concern about consistency between the platforms.
>
> I've just noticed Tegra also has SoC-specific controls, but they're located
> in drivers/soc/tegra/Kconfig, not the main arch/arm64/Kconfig.platforms.
> Do you consider that a viable alternative?

Yes, I think that would be ok as well.

      Arnd
Simon Horman Oct. 8, 2018, 7:31 a.m. UTC | #9
On Thu, Oct 04, 2018 at 05:36:15PM +0200, Arnd Bergmann wrote:
> On Thu, Oct 4, 2018 at 9:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Oct 2, 2018 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Oct 2, 2018 at 2:18 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Tue, Oct 2, 2018 at 10:31 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Fri, Sep 28, 2018 at 10:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > On Fri, Sep 28, 2018 at 12:21 PM Simon Horman
> > > > > > <horms+renesas@verge.net.au> wrote:
> > > > > > > * Add support for RZ/G2E (r8a774c0) and RZ/G2M (r8a774a1) SoCs
> > > > > > > * Enable Compare Match Timer (CMT) and Timer Unit (TMU)
> > > > > > >   for Renesas SoCs
> > > > > > > * Remove no longer needed ARCH_SHMOBILE Kconfig symbol
> > > > > >
> > > > > > The ARCH_SHMOBILE removal and cleanup is fine, but I wonder about
> > > > > > the newly added symbols for specific SoCs. I see you already have a
> > > > > > couple of those, but the other manufacturers don't.
> > > > > >
> > > > > > I think what happened here is that I tried to reject those additions
> > > > > > normally, but I never noticed yours. From what I see in drivers,
> > > > > > you have additional symbols depending on these in clk, pinctrl,
> > > > > > drivers/soc, and the dtb files, so at the very least we can't
> > > > > > just drop the config options, but I'd still like to see this work
> > > > > > more like other platforms.
> > > > >
> > > > > The main motivation for SoC-specific controls is to avoid including pinctrl
> > > > > and (to a lesser extent) clock tables for unused SoCs.  Each pinctrl driver
> > > > > consumes a few tens of KiB, each clock driver a few KiB.
> > > > > If we remove the SoC-specific ARCH_* controls, the user has to configure
> > > > > both pinctrl and clock driver selections, thus trading one user-visible
> > > > > symbol for two new user-visible symbols (except for RZ/A and RZ/N,
> > > > > where the pinctrl symbols are already visible, to allow reducing kernel
> > > > > size).
> > > > > Perhaps that is acceptable?
> > > >
> > > > It's definitely fine with me. I understand that this is less user friendly,
> > > > but it does address my concern about consistency between the platforms.
> > > >
> > > > > For the smaller parts in drivers/soc/, removing SoC-specific dependencies
> > > > > may be viable for R-Car and RZ/G Socs, but not for e.g. RZ/A.  The latter
> > > > > may be used without external RAM, and can run with a few MiB of internal
> > > > > SRAM, and XIP (with a still out-of-tree patch).
> > > > > So at least a family-specific dependency is worth to keep.  For now, RZ/A
> > > > > is arm32 only, though.
> > > > >
> > > > > Note that on arm64, there are no family-specific Kconfig symbols yet.
> > > > > We just have ARCH_RENESAS, which is also set on arm32.
> > > > > So either we introduce e.g. ARCH_RCAR_GEN3 (cfr. ARCH_RCAR_GEN1 and
> > > > > ARCH_RCAR_GEN2 or arm32), or use "ARCH_RENESAS && ARM64" for e.g. RST_RCAR
> > > > > in drivers/soc/renesas/Kconfig, which currently depends on all R-Car Gen3
> > > > > (and RZ/G2) SoCs explicitly.
> > > > > Choosing the latter option means we may need to migrate to the former option
> > > > > later, once other families appear (e.g. RZ/A SoCs may start using
> > > > > Cortex-A53 instead of Cortex-A9 cores).
> > > >
> > > > Maybe introducing a hidden ARCH_RCAR_GEN3 makes sense here, and
> > > > that can start out with the same value as ARCH_RENESAS. I think
> > > > if we want to add 64-bit RZ/A support later, having a separate top-level
> > > > symbol for that is also reasonable: as you explained this is rather
> > > > sensitive to memory usage, and that is enough reason to deviate from
> > > > what we do elsewhere.
> > > >
> > > > > For the DTB files, I believe we can just remove the dependencies, and
> > > > > always build all DTBs.
> > > >
> > > > Yes, that seems best, and consistent with how we handle other
> > > > manufacturers.
> > >
> > > OK, I'll put it on my list.
> >
> > Given the current time in the cycle, we can no longer do this for v4.20.
> > Hence, can you please accept Simon's PR as-is, to enable us to move
> > forward?
> 
> Fair enough, pulled into next/soc now.

Thanks Arnd!