Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Paul Menzel]

Dear Nick,


Thank you for the patches.

Am 21.04.22 um 21:21 schrieb nick.hawkins@xxxxxxx:
> From: Nick Hawkins <nick.hawkins@xxxxxxx>

Type in the prefix: s/aach/arch/. Looking at `git log --oneline 
arch/arm`, *ARM* or *arm* seems to be commonly used though.

> The GXP is the HPE BMC SoC that is used in the majority
> of HPE Generation 10 servers. Traditionally the asic will
> last multiple generations of server before being replaced.

Please mention what kind of documentation (datasheets, …) are available.

> In gxp.c we reset the EHCI controller early to boot the asic.

Why does the EHCI controller need to be reset?

> Info about SoC:
>
> HPE GXP is the name of the HPE Soc. This SoC is used to implement
> many BMC features at HPE. It supports ARMv7 architecture based on
> the Cortex A9 core. It is capable of using an AXI bus to which
> a memory controller is attached. It has multiple SPI interfaces
> to connect boot flash and BIOS flash. It uses a 10/100/1000 MAC
> for network connectivity. It has multiple i2c engines to drive
> connectivity with a host infrastructure. The initial patches
> enable the watchdog and timer enabling the host to be able to
> boot.

Maybe doe that in separate commits?

Please reflow the commit message for 75 characters per line.

> Signed-off-by: Nick Hawkins <nick.hawkins@xxxxxxx>
>
> ---
> v5:
> * Fixed version log
> v4:
> * Removed unecessary code: restart, iomap, init_machine

unnecessary

> * Reordered Kconfig depends
> * Removed SPARSE_IRQ, MULTI_IRQ_HANDLER, IRQ_DOMAIN, PINCTL from
>    Kconfig
> v3:
> * Put into proper patchset format
> v2:
> * No change
> ---
>   arch/arm/Kconfig           |  2 ++
>   arch/arm/Makefile          |  1 +
>   arch/arm/mach-hpe/Kconfig  | 17 +++++++++++++++++
>   arch/arm/mach-hpe/Makefile |  1 +
>   arch/arm/mach-hpe/gxp.c    | 16 ++++++++++++++++
>   5 files changed, 37 insertions(+)
>   create mode 100644 arch/arm/mach-hpe/Kconfig
>   create mode 100644 arch/arm/mach-hpe/Makefile
>   create mode 100644 arch/arm/mach-hpe/gxp.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2e8091e2d8a8..13f77eec7c40 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -620,6 +620,8 @@ source "arch/arm/mach-highbank/Kconfig"
>   
>   source "arch/arm/mach-hisi/Kconfig"
>   
> +source "arch/arm/mach-hpe/Kconfig"
> +
>   source "arch/arm/mach-imx/Kconfig"
>   
>   source "arch/arm/mach-integrator/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index a2391b8de5a5..97a89023c10f 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -179,6 +179,7 @@ machine-$(CONFIG_ARCH_FOOTBRIDGE)	+= footbridge
>   machine-$(CONFIG_ARCH_GEMINI)		+= gemini
>   machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
>   machine-$(CONFIG_ARCH_HISI)		+= hisi
> +machine-$(CONFIG_ARCH_HPE)		+= hpe
>   machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
>   machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
>   machine-$(CONFIG_ARCH_IXP4XX)		+= ixp4xx
> diff --git a/arch/arm/mach-hpe/Kconfig b/arch/arm/mach-hpe/Kconfig
> new file mode 100644
> index 000000000000..c075248b259e
> --- /dev/null
> +++ b/arch/arm/mach-hpe/Kconfig
> @@ -0,0 +1,17 @@
> +menuconfig ARCH_HPE
> +	bool "HPE SoC support"
> +	depends on ARCH_MULTI_V7
> +	help
> +	  This enables support for HPE ARM based SoC chips

Add a dot/period at the end?

> +if ARCH_HPE
> +
> +config ARCH_HPE_GXP
> +	bool "HPE GXP SoC"
> +	depends on ARCH_MULTI_V7
> +	select ARM_VIC
> +	select GENERIC_IRQ_CHIP
> +	select CLKSRC_MMIO
> +	help
> +	  Support for GXP SoCs

Please elaborate here, maybe copying parts of the commit message, in 
what servers it is used.

> +
> +endif
> diff --git a/arch/arm/mach-hpe/Makefile b/arch/arm/mach-hpe/Makefile
> new file mode 100644
> index 000000000000..8b0a91234df4
> --- /dev/null
> +++ b/arch/arm/mach-hpe/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ARCH_HPE_GXP) += gxp.o
> diff --git a/arch/arm/mach-hpe/gxp.c b/arch/arm/mach-hpe/gxp.c
> new file mode 100644
> index 000000000000..e2f0c3ae6bd8
> --- /dev/null
> +++ b/arch/arm/mach-hpe/gxp.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/

Space before closing comment delimiter.

> +
> +#include <linux/of_platform.h>
> +#include <asm/mach/arch.h>
> +
> +static const char * const gxp_board_dt_compat[] = {
> +	"hpe,gxp",
> +	NULL,
> +};
> +
> +DT_MACHINE_START(GXP_DT, "HPE GXP")
> +	.dt_compat	= gxp_board_dt_compat,
> +	.l2c_aux_val = 0,
> +	.l2c_aux_mask = 0,
> +MACHINE_END

Where is the EHCI controller reset?


Kind regards,

Paul