[PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER

[PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER

[Geert Uytterhoeven]

The Marvell CN10K True Random Number generator is only present on
Marvell CN10K SoCs, and not available as an independent PCIe endpoint.
Hence add a dependency on ARCH_THUNDER, to prevent asking the user about
this driver when configuring a kernel without Cavium Thunder (incl.
Marvell CN10K) SoC support.

Fixes: 38e9791a02090414 ("hwrng: cn10k - Add random number generator support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/char/hw_random/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index c91cb48a1db168dd..b33c01e9935336f7 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -540,7 +540,7 @@ config HW_RANDOM_ARM_SMCCC_TRNG
 
 config HW_RANDOM_CN10K
        tristate "Marvell CN10K Random Number Generator support"
-       depends on HW_RANDOM && PCI && ARM64
+       depends on HW_RANDOM && PCI && ARCH_THUNDER
        default HW_RANDOM
        help
 	 This driver provides support for the True Random Number
-- 
2.25.1

Re: [EXT] [PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER

[Arnd Bergmann]

On Wed, Jan 12, 2022 at 4:55 PM Sunil Kovvuri Goutham
<sgoutham@marvell.com> wrote:
>
> >From: Geert Uytterhoeven <geert+renesas@glider.be>
> >Sent: Wednesday, January 12, 2022 7:33 PM
> >To: Herbert Xu <herbert@gondor.apana.org.au>; Sunil Kovvuri Goutham <sgoutham@marvell.com>; Bharat Bhushan <bbhushan2@marvell.com>; Joseph Longever <jlongever@marvell.com>
> >Cc: Arnd Bergmann <arnd@arndb.de>; linux-crypto@vger.kernel.org <linux-crypto@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>
> >Subject: [EXT] [PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER
>
> >The Marvell CN10K True Random Number generator is only present on
> >Marvell CN10K SoCs, and not available as an independent PCIe endpoint.
> >Hence add a dependency on ARCH_THUNDER, to prevent asking the user about
> >this driver when configuring a kernel without Cavium Thunder (incl.
> >Marvell CN10K) SoC support.
> >
> >Fixes: 38e9791a02090414 ("hwrng: cn10k - Add random number generator support")
> >Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >---
> >drivers/char/hw_random/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> >index c91cb48a1db168dd..b33c01e9935336f7 100644
> >--- a/drivers/char/hw_random/Kconfig
> >+++ b/drivers/char/hw_random/Kconfig
> >@@ -540,7 +540,7 @@ config HW_RANDOM_ARM_SMCCC_TRNG
> >
> > config HW_RANDOM_CN10K
> >        tristate "Marvell CN10K Random Number Generator support"
> >-       depends on HW_RANDOM && PCI && ARM64
> >+       depends on HW_RANDOM && PCI && ARCH_THUNDER
> >        default HW_RANDOM
> >        help
> >          This driver provides support for the True Random Number
>
> Nack.
> ARCH_THUNDER/THUNDER2 are old Cavium server class silicon series
> which are not related to Marvell CN10K silicon.

Can you tell me where you would draw the line? Based on a discussion we had on
IRC, I was going to send a patch to rename ARCH_THUNDER to ARCH_OCTEON
and clarify how it relates to the other families. Here is what I
understood it should be:

config ARCH_OCTEON
        bool "Marvell OCTEON and ThunderX data processing units"
        help
          This enables support for Marvell (formerly Cavium) OCTEON
          Family of DPUs and SoCs, including OCTEON 10, Octeon TX2
          CN92xx/CN96xx/CN98xx, OcteonTX CN8xxx, ThunderX CN88xx, and
          Octeon Fusion products.

          Note: these are unrelated to the similarly named ThunderX2
          CN99xx server processors, the Octeon TX2 91xx SoCs and the
          Armada processors.

config ARCH_THUNDER2
        bool "Marvell/Cavium ThunderX2 Server Processors"
        select GPIOLIB
        help
          This enables support for Marvell's discontinued ThunderX2
          CN99XX family of server processors, originally sold by Cavium.

          Note: these do not include the unrelated ThunderX CN88xx or
          OCTEON TX2 processors, despite the similarities in naming.

config ARCH_MVEBU
        bool "Marvell EBU SoC Family"
        help
          This enables support for Marvell EBU familly, including:
           - Armada 3700 SoC Family
           - Armada 7K SoC Family
           - Armada 8K SoC Family
           - Octeon TX2 CN91xx Family

If that's not the correct interpretation, does that mean that OCTEON 10
and Octeon TX2 CN92xx/CN96xx/CN98xx are a different family from
Octeon/TX CN8xxx and ThunderX CN88xx and should have a fourth
symbol, or are they part of the Armada family?


      Arnd

Re: [EXT] [PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER

[Arnd Bergmann]

On Wed, Jan 12, 2022 at 5:34 PM Sunil Kovvuri Goutham
<sgoutham@marvell.com> wrote:
> >Subject: Re: [EXT] [PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER
> >On Wed, Jan 12, 2022 at 4:55 PM Sunil Kovvuri Goutham <sgoutham@marvell.com> wrote:
> >config ARCH_OCTEON
> >        bool "Marvell OCTEON and ThunderX data processing units"
> >       help
> >          This enables support for Marvell (formerly Cavium) OCTEON
> >          Family of DPUs and SoCs, including OCTEON 10, Octeon TX2
> >          CN92xx/CN96xx/CN98xx, OcteonTX CN8xxx, ThunderX CN88xx, and
> >          Octeon Fusion products.
> >
> >          Note: these are unrelated to the similarly named ThunderX2
> >         CN99xx server processors, the Octeon TX2 91xx SoCs and the
> >          Armada processors.
> >
> >config ARCH_THUNDER2
> >        bool "Marvell/Cavium ThunderX2 Server Processors"
> >        select GPIOLIB
> >        help
> >          This enables support for Marvell's discontinued ThunderX2
> >          CN99XX family of server processors, originally sold by Cavium.
> >
> >          Note: these do not include the unrelated ThunderX CN88xx or
> >          OCTEON TX2 processors, despite the similarities in naming.
> >
> >config ARCH_MVEBU
> >        bool "Marvell EBU SoC Family"
> >        help
> >          This enables support for Marvell EBU familly, including:
> >           - Armada 3700 SoC Family
> >           - Armada 7K SoC Family
> >           - Armada 8K SoC Family
> >           - Octeon TX2 CN91xx Family
> >
> >If that's not the correct interpretation, does that mean that OCTEON 10
> >and Octeon TX2 CN92xx/CN96xx/CN98xx are a different family from
> >Octeon/TX CN8xxx and ThunderX CN88xx and should have a fourth
> >symbol, or are they part of the Armada family?
>
> OcteonTx (8xx) are derivatives of ThunderX.

Ok, and those are the same family as the earlier cnMIPS based OCTEON
CN3xxx/CN5xxx/CN6xxx/CN7xxx/CNF7xxx and the later ARMv8.2
Fusion CNF9xxx, right?

> Octeon 10 and OcteonTx2 (9x series) are different families and not related to
> Armada as well.

I assume with '9x' you mean only the CN92xx/CN96xx/CN98xx/CN10x/DPU400
family here, not the OcteonTx2 CN91xx that is clearly part of the Marvell
Sheeva/Kirkwood/MVEBU/Armada family, and the CN99xx in the
Netlogic/Broadcom family.

Is there anything that you can say about this product line? It looked like it
was derived from the cnMIPS/OcteonTX line, and it seems to share its
mystery (presumably ThunderX1-derived rather than Cortex or Vulcan)
ARMv8.2 core with the CNF9xxx.

> But I am fine if ARCH_THUNDER is renamed to ARCH_OCTEON to include all.

I'd really prefer to have sensible names, so if there are six unrelated Marvell
SoC families (pxa/mmp, armada/mvebu/cn91xx, xlp/vulcan/thunderx2,
berlin/synaptics, cavium/octeon/thunderx/fusion, and whatever turned into
non-cavium cn92xx/cn96xx/cn98xx/cn10x/dpu400) instead of five, we should
probably have six ARCH_* names for those. (Note: for some other
manufacturers such as Broadcom or Mediatek, we do use just the company
name as the CONFIG_ARCH_* symbol, but I feel that for Marvell or NXP,
that ship has sailed long ago, based on the number of acquisitions and
spinoffs).

Any suggestions for the name? Were these acquired from some other
company?

        Arnd

Re: [EXT] [PATCH] hwrng: cn10k - HW_RANDOM_CN10K should depend on ARCH_THUNDER

[Arnd Bergmann]

On Thu, Jan 13, 2022 at 10:42 AM Sunil Kovvuri Goutham
<sgoutham@marvell.com> wrote:
> >From: Arnd Bergmann <arnd@arndb.de>

> >
> >Ok, and those are the same family as the earlier cnMIPS based OCTEON
> >CN3xxx/CN5xxx/CN6xxx/CN7xxx/CNF7xxx and the later ARMv8.2
> >Fusion CNF9xxx, right?
>
> Yes, except the CNF9x.

Ok, what is CNF9x then?

> >> Octeon 10 and OcteonTx2 (9x series) are different families and not related to
> >> Armada as well.
>
> >I assume with '9x' you mean only the CN92xx/CN96xx/CN98xx/CN10x/DPU400
> >family here, not the OcteonTx2 CN91xx that is clearly part of the Marvell
> >Sheeva/Kirkwood/MVEBU/Armada family, and the CN99xx in the
> >Netlogic/Broadcom family.
>
> By 9x I mean CN96xx, CN98xx, CNF9xx, base architecture is same across all.

> >Is there anything that you can say about this product line? It looked like it
> >was derived from the cnMIPS/OcteonTX line, and it seems to share its
> >mystery (presumably ThunderX1-derived rather than Cortex or Vulcan)
> >ARMv8.2 core with the CNF9xxx.
>
> 9x core is Marvell (or Cavium) developed core and is next-gen of 8x.
> 10x and DPU400 have ARM cores.

I don't care about the CPU core, only about the SoC design for the family,
the old octeon family includes both cnMIPS and ThunderX cores already
and those are not even the same ISA, so going from custom ARMv8.2
cores to Neoverse ARMv9 is not relevant at all.

> CN91xx is Armada.

Ok

> >> But I am fine if ARCH_THUNDER is renamed to ARCH_OCTEON to include all.
>
> >I'd really prefer to have sensible names, so if there are six unrelated Marvell
> >SoC families (pxa/mmp, armada/mvebu/cn91xx, xlp/vulcan/thunderx2,
> >berlin/synaptics, cavium/octeon/thunderx/fusion, and whatever turned into
> >non-cavium cn92xx/cn96xx/cn98xx/cn10x/dpu400) instead of five, we should
> >probably have six ARCH_* names for those. (Note: for some other
> >manufacturers such as Broadcom or Mediatek, we do use just the company
> >name as the CONFIG_ARCH_* symbol, but I feel that for Marvell or NXP,
> >that ship has sailed long ago, based on the number of acquisitions and
> >spinoffs).
> >
> >Any suggestions for the name? Were these acquired from some other company?
>
> For OcteonTx2, CN10K and future silicons, ARCH_MARVELL_OCTEON  / ARCH_MRVL_OCTEON ?
> Different ARCH_* config for each family is not needed IMHO.

It does sound like these are all the same family then. Ideally we'd use the same
name as the MIPS based parts, but those currently use
CONFIG_CAVIUM_OCTEON_SOC, which doesn't fit the same naming scheme,
and we seem to have a duplicate set of drivers.

We usually pick shorter names, so I'd just leave it with CONFIG_ARCH_OCTEON
as I have in my current patch. That also means it's less likely to
require a rename
after the next acquisition or spinoff ;-)

        Arnd