[PATCH 0/7] Add SE HMBSC board support

Thu Dec 21 14:38:58 CET 2023

On 20/12/2023 13:36, Sumit Garg wrote:
> On Tue, 19 Dec 2023 at 21:56, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>>
>>
>> On 19/12/2023 06:25, Sumit Garg wrote:
>>> Hi Simon,
>>>
>>> On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg at chromium.org> wrote:
>>>>
>>>> Hi Sumit,
>>>>
>>>> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg at linaro.org> wrote:
>>>>>
>>>>> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
>>>>
>>>> Could you please add a doc/ file for this board and explain how to
>>>> build it and how to run U-Boot on it?
>>>
>>> Ah I forgot to add that since the build/boot instructions are quite
>>> similar to db410c. BTW, I will add that in the next spin.
>>
>> Probably just referencing that document or making the language generic
>> would be good.
>>
> 
> I will rename that doc to be rather SoC specific with board specific
> sub-sections.
> 
>> Sumit, could you rebase this series on my generic board support? [1] in
>> it's current form this series conflicts, and includes some of the major
>> anti-patterns I'm trying to move away from in mach-snapdragon.
> 
> Although, I haven't gone through your series but I was expecting those
> conflicts. Let's work together to make this series compatible on top
> of yours.
> 
>>
>> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
>> from what I can tell you shouldn't even need to add the board/schneider
>> directory at all, you can just set the following in your defconfig:
>>
>> CONFIG_SYS_BOARD="dragonboard410c"
> 
> This is simply a misnomer, its HMIBSC board. I suppose we should
> rather separate the SoC specific bits into mach-snapdragon and let
> different boards use them.

Is there any reason why you can't just use the existing db410c board
code as-is? I don't see why you want to duplicate it.

The only reason the db410c and db820c have their board code is because
they're old platforms and already supported. For adding new support
there needs to be some very strong justification to have board-specific
C code.

I think it would be nice to make the db410c code go away, or be toggled
at runtime, probably most of it will just work and not break any other
boards anyway. The db820c code is just part of what should be in the
pinctrl driver...

Let's move away from this old model and towards having more generic
U-Boot images. This will snowball towards making future bringup even easier.
> 
>> CONFIG_SYS_CONFIG_NAME="hmibsc"
>>
>> This will use the db410c board code (which yours is just a copy/paste of
>> from what I can tell) and your custom include/configs/hmibsc.h header.
>>
>> The addresses set in your environment file should be allocated
>> automatically at runtime too (see the ("mach-snapdragon: dynamic load
>> addresses") patch).
>>
>> You should also switch to an upstream board DTS based on my series, and
>> drop the "-uboot.dtsi" file.
> 
> Unfortunately, currently there isn't any upstream DTS for this board
> but I will check with the SE team regarding their plans. Until then we
> have to use a U-Boot specific DTS file.

In that case, perhaps we can take whatever DTS they're using in
production, or a version of it, and at least split the U-Boot changes
(if any) out into a separate file as I've done with the other platforms.
This way we stay consistent and can keep track of what U-Boot specific
DTS changes we need.

I guess this is all new territory for us, but imho if we're adding
support for a board that doesn't have an upstream DTS, we should still
follow the same model, open to input here.
> 
> -Sumit
> 
>>
>> [1]:
>> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/
>>
>> Kind regards,
>>
>>>
>>> -Sumit
>>>
>>>>
>>>>> difference from db410c is serial port where HMIBSC board uses UART1 as
>>>>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
>>>>> driver support.
>>>>>
>>>>> Patch #4 adds main HMIBSC board specific bits, features:
>>>>> - Qualcomm Snapdragon 410C SoC - APQ8016 (4xCortex A53, Adreno 306)
>>>>> - 2GiB RAM
>>>>> - 64GiB eMMC, SD slot
>>>>> - WiFi and Bluetooth
>>>>> - 2x Host, 1x Device USB port
>>>>> - HDMI
>>>>> - Discrete TPM2 chip over SPI
>>>>>
>>>>> Patch #5 - #7 enables specific board features like RAUC support,
>>>>> environment protection and USB networking support.
>>>>>
>>>>> This patch series is based on top of Qcom maintainer tree [1] + the latest
>>>>> PMIC patch-set [2]. Feedback is very much welcome.
>>>>>
>>>>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
>>>>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
>>>>>
>>>>> Sumit Garg (7):
>>>>>   clk: apq8016: Add support for UART1 clocks
>>>>>   serial_msm: Add support for RS232 GPIOs
>>>>>   serial_msm: Enable RS232 flow control
>>>>>   board: Add SE HMIBSC board support
>>>>>   hmibsc: Enable RAUC support
>>>>>   hmibsc: enable U-Boot Environment variables protection
>>>>>   hmibsc: Enable LAN75XX USB ethernet driver
>>>>>
>>>>>  arch/arm/dts/Makefile              |   1 +
>>>>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
>>>>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
>>>>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
>>>>>  arch/arm/mach-snapdragon/Makefile  |   1 +
>>>>>  board/schneider/hmibsc/Kconfig     |  15 +++
>>>>>  board/schneider/hmibsc/MAINTAINERS |   6 +
>>>>>  board/schneider/hmibsc/Makefile    |   5 +
>>>>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
>>>>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
>>>>>  configs/hmibsc_defconfig           |  79 ++++++++++++
>>>>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
>>>>>  drivers/serial/serial_msm.c        |  23 +++-
>>>>>  drivers/usb/host/Kconfig           |   1 +
>>>>>  include/configs/hmibsc.h           |  59 +++++++++
>>>>>  15 files changed, 665 insertions(+), 8 deletions(-)
>>>>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
>>>>>  create mode 100644 arch/arm/dts/hmibsc.dts
>>>>>  create mode 100644 board/schneider/hmibsc/Kconfig
>>>>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
>>>>>  create mode 100644 board/schneider/hmibsc/Makefile
>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
>>>>>  create mode 100644 configs/hmibsc_defconfig
>>>>>  create mode 100644 include/configs/hmibsc.h
>>>>>
>>>>> --
>>>>> 2.34.1
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>
>> --
>> // Caleb (they/them)

-- 
// Caleb (they/them)