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

[Jae Hyun Yoo]

On 7/5/2022 7:28 AM, Jae Hyun Yoo 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.
> 
> Thanks,
> Jae
> 
>>> +
>>> +config SYS_SOC
>>> +       default "ast2600"
>>> +
>>> +config SYS_CONFIG_NAME
>>> +       default "evb_ast2600"
>>> +
>>> +endif
>>> diff --git a/board/aspeed/ast2600_qualcomm/Makefile 
>>> b/board/aspeed/ast2600_qualcomm/Makefile
>>> new file mode 100644
>>> index 000000000000..cb2aae7f9298
>>> --- /dev/null
>>> +++ b/board/aspeed/ast2600_qualcomm/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-y += dc-scm-v1.o
>>> diff --git a/board/aspeed/ast2600_qualcomm/dc-scm-v1.c 
>>> b/board/aspeed/ast2600_qualcomm/dc-scm-v1.c
>>> new file mode 100644
>>> index 000000000000..40bc85e068ad
>>> --- /dev/null
>>> +++ b/board/aspeed/ast2600_qualcomm/dc-scm-v1.c
>>> @@ -0,0 +1,33 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights 
>>> reserved.
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <asm/io.h>
>>> +
>>> +/* GPIO registers */
>>> +#define GPIO_BASE              0x1e780000
>>> +#define GPIO_UVWX_VAL          (GPIO_BASE + 0x88)
>>> +#define GPIO_UVWX_VAL_V3       BIT(11)
>>> +#define GPIO_UVWX_DIR          (GPIO_BASE + 0x8c)
>>> +#define GPIO_UVWX_DIR_V3       BIT(11)
>>> +
>>> +static void gpio_init(void)
>>> +{
>>> +       /* Set GPIOV3 (BMC_OK) as an output with value low 
>>> explicitly. */
>>
>> 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.

>> (Again, you need to explain your decisions in the commit message).

Okay. I'll make the commit message descriptive in v2.

Thanks,
Jae

>>> +       writel(readl(GPIO_UVWX_DIR) | GPIO_UVWX_DIR_V3, GPIO_UVWX_DIR);
>>> +       writel(readl(GPIO_UVWX_VAL) & ~GPIO_UVWX_VAL_V3, GPIO_UVWX_VAL);
>>> +}
>>> +
>>> +int board_early_init_f(void)
>>> +{
>>> +       gpio_init();
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +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.
> 
> Thanks,
> Jae
> 
>>> +{
>>> +       return 0;
>>> +}
>>> -- 
>>> 2.25.1
>>>