[PATCH u-boot v2019.04-aspeed-openbmc 2/3] board: ast2600_qualcomm: add initial version of Qualcomm DC-SCM V1 board

[Joel Stanley]

On Tue, 5 Jul 2022 at 14:28, Jae Hyun Yoo <quic_jaehyoo at quicinc.com> wrote:
>
> Hello Joel,
>
> On 7/4/2022 11:51 PM, Joel Stanley wrote:
> > On Thu, 30 Jun 2022 at 20:02, Jae Hyun Yoo <quic_jaehyoo at quicinc.com> wrote:
> >>
> >> Add initial version of Qualcomm DC-SCM V1 board to support Qualcomm
> >> specific options.
> >>
> >> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
> >> ---
> >>   arch/arm/mach-aspeed/ast2600/Kconfig      |  8 ++++++
> >>   board/aspeed/ast2600_qualcomm/Kconfig     | 15 +++++++++++
> >>   board/aspeed/ast2600_qualcomm/Makefile    |  1 +
> >>   board/aspeed/ast2600_qualcomm/dc-scm-v1.c | 33 +++++++++++++++++++++++
> >>   4 files changed, 57 insertions(+)
> >>   create mode 100644 board/aspeed/ast2600_qualcomm/Kconfig
> >>   create mode 100644 board/aspeed/ast2600_qualcomm/Makefile
> >>   create mode 100644 board/aspeed/ast2600_qualcomm/dc-scm-v1.c
> >>
> >> diff --git a/arch/arm/mach-aspeed/ast2600/Kconfig b/arch/arm/mach-aspeed/ast2600/Kconfig
> >> index 46cc1ad1dbd9..252458846a84 100644
> >> --- a/arch/arm/mach-aspeed/ast2600/Kconfig
> >> +++ b/arch/arm/mach-aspeed/ast2600/Kconfig
> >> @@ -46,6 +46,13 @@ config TARGET_AST2600_INTEL
> >>            AST2600-INTEL is an Intel Eagle Stream CRB with
> >>            AST2600 as the BMC.
> >>
> >> +config TARGET_AST2600_QUALCOMM_DC_SCM_V1
> >> +       bool "AST2600_QUALCOMM_DC_SCM_V1"
> >> +       depends on ASPEED_AST2600
> >> +       help
> >> +         AST2600-QUALCOMM-DC-SCM-V1 is a Qualcomm DC-SCM V1 board which is
> >> +         equipped with AST2600.
> >> +
> >>   endchoice
> >>
> >>   source "board/aspeed/evb_ast2600/Kconfig"
> >> @@ -53,5 +60,6 @@ source "board/aspeed/fpga_ast2600/Kconfig"
> >>   source "board/aspeed/slt_ast2600/Kconfig"
> >>   source "board/aspeed/ast2600_ibm/Kconfig"
> >>   source "board/aspeed/ast2600_intel/Kconfig"
> >> +source "board/aspeed/ast2600_qualcomm/Kconfig"
> >>
> >>   endif
> >> diff --git a/board/aspeed/ast2600_qualcomm/Kconfig b/board/aspeed/ast2600_qualcomm/Kconfig
> >> new file mode 100644
> >> index 000000000000..3ede24c34dee
> >> --- /dev/null
> >> +++ b/board/aspeed/ast2600_qualcomm/Kconfig
> >> @@ -0,0 +1,15 @@
> >> +if TARGET_AST2600_QUALCOMM_DC_SCM_V1
> >> +
> >> +config SYS_BOARD
> >> +       default "ast2600_qualcomm"
> >> +
> >> +config SYS_VENDOR
> >> +       default "aspeed"
> >
> > Out of interest, how does u-boot handle this upstream? Does a board
> > made by company Y with a chip made by company X considered vendor X,
> > or vendor Y?
>
> This code is added into a sub-directory of 'board/aspeed' so the
> SYS_VENDOR must be set to 'aspeed'. If I set that to 'qcom', then
> builder will look for an Makefile under 'board/qcom' instead and it
> makes a failure. The same pattern is already used for ast2600_ibm and
> ast2600_intel.

Yes, that is true, but it's not what I'm asking. There's momentum to
use the upstream tree for u-boot and I want to ensure any patches that
go in from now on are applicable upstream.

Can you do a survey of the upstream tree and see what the pattern is?

If it's to use the manufacturer

> >> +int board_late_init(void)
> >
> > Do you need to include this if it's doing nothing?
>
> The defconfig has 'CONFIG_BOARD_LATE_INIT=y' so this function should be
> added as a dummy function otherwise it meets a build failure. This
> function will be filled by following commits later.

Doesn't it make sense to not set CONFIG_BOARD_LATE_INIT=y if you don't need it?

You can introduce it when you need it (or add the code that uses it
with this commit).

> >> Is there a reason you don't use the gpio driver?
>
> Forgot to reply on this comment. This function is called from
> board_early_init_f and gpio driver is not ready at that timing so
> it uses direct register access.

Why not call it at a later boot stage, so the gpio driver is ready?

Cheers,

Joel