Hi Quentin, Thanks for your review.
Quentin Schulz <quentin.sch...@cherry.de> 于2025年8月11日周一 23:01写道： > > Hi WeiHao Li, > > On 8/7/25 9:44 AM, WeiHao Li wrote: > > [You don't often get email from cn.liwei...@gmail.com. Learn why this is > > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > Please always include a few words in the commit log. > I will add some necessary description in the commit log. > I believe the description of the board as told in the cover letter is > good enough, so start with that maybe :) > > > Signed-off-by: WeiHao Li <ie...@outlook.com> > > --- > > arch/arm/dts/Makefile | 1 + > > arch/arm/dts/rk3368-ymd8-mb-u-boot.dtsi | 44 ++++ > > arch/arm/dts/rk3368-ymd8-mb.dts | 326 ++++++++++++++++++++++++ > > arch/arm/dts/rk3368.dtsi | 258 +++++++++++++++++++ > > arch/arm/mach-rockchip/rk3368/Kconfig | 6 + > > board/rockchip/ymd8_mb/Kconfig | 12 + > > board/rockchip/ymd8_mb/MAINTAINERS | 6 + > > board/rockchip/ymd8_mb/Makefile | 7 + > > board/rockchip/ymd8_mb/README | 1 + > > board/rockchip/ymd8_mb/ymd8_mb_rk3368.c | 19 ++ > > configs/ymd8-mb_defconfig | 72 ++++++ > > include/configs/ymd8_mb.h | 11 + > > 12 files changed, 763 insertions(+) > > create mode 100644 arch/arm/dts/rk3368-ymd8-mb-u-boot.dtsi > > create mode 100644 arch/arm/dts/rk3368-ymd8-mb.dts > > create mode 100644 board/rockchip/ymd8_mb/Kconfig > > create mode 100644 board/rockchip/ymd8_mb/MAINTAINERS > > create mode 100644 board/rockchip/ymd8_mb/Makefile > > create mode 100644 board/rockchip/ymd8_mb/README > > create mode 100644 board/rockchip/ymd8_mb/ymd8_mb_rk3368.c > > create mode 100644 configs/ymd8-mb_defconfig > > create mode 100644 include/configs/ymd8_mb.h > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > index 0dc7e190eb..1dfd1c5236 100644 > > --- a/arch/arm/dts/Makefile > > +++ b/arch/arm/dts/Makefile > > @@ -73,6 +73,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3368) += \ > > rk3368-sheep.dtb \ > > rk3368-geekbox.dtb \ > > rk3368-px5-evb.dtb \ > > + rk3368-ymd8-mb.dtb > > > > dtb-$(CONFIG_ARCH_S5P4418) += \ > > s5p4418-nanopi2.dtb > > diff --git a/arch/arm/dts/rk3368-ymd8-mb-u-boot.dtsi > > b/arch/arm/dts/rk3368-ymd8-mb-u-boot.dtsi > > new file mode 100644 > > index 0000000000..925264e620 > > --- /dev/null > > +++ b/arch/arm/dts/rk3368-ymd8-mb-u-boot.dtsi > > @@ -0,0 +1,44 @@ > > +// SPDX-License-Identifier: GPL-2.0+ OR X11 > > +/* > > + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH > > + */ > > + > > I'm quite sure Theobroma didn't work on that, and not in 2017 :) > > Please remove us (Theobroma got renamed to Cherry Embedded Solutions > last year) from this copyright notice and provide the appropriate > copyright holder there instead. > Got it. > > +#include "rk3368-u-boot.dtsi" > > + > > +&pinctrl { > > + bootph-all; > > +}; > > + > > +&service_msch { > > + bootph-all; > > +}; > > + > > +&dmc { > > + bootph-all; > > + status = "okay"; > > +}; > > + > > +&pmugrf { > > + bootph-all; > > +}; > > + > > +&cru { > > + bootph-all; > > +}; > > + > > +&grf { > > + bootph-all; > > +}; > > + > > +&uart2 { > > + bootph-all; > > + clock-frequency = <24000000>; > > +}; > > + > > +&pwm1 { > > + bootph-all; > > +}; > > + > > +&vop { > > + bootph-all; > > +}; > > diff --git a/arch/arm/dts/rk3368-ymd8-mb.dts > > b/arch/arm/dts/rk3368-ymd8-mb.dts > > new file mode 100644 > > index 0000000000..661390b9d5 > > --- /dev/null > > +++ b/arch/arm/dts/rk3368-ymd8-mb.dts > > Are you planning to support this board in upstream Linux as well? I'm > sure Heiko would welcome it. If so, maybe think about selecting > OF_UPSTREAM for your board once the device tree is merged upstream and > appears in dts/upstream directory in U-Boot? It may not be easy to do as > no RK3368-based board has been migrated to use OF_UPSTREAM yet. > > We typically do not allow (in Rockchip) to support new boards without > OF_UPSTREAM, but considering how old and how little activity we see for > RK3368 maybe Kever will let this pass? > Yes, I'm planning to suport this board in upstream Linux. but it may take some time. Maybe I can separate this patch serial to 2 set, one for driver fix, another for board support., the board support patchset submit after upstream Linux merged? > > @@ -0,0 +1,326 @@ > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > +/* > > + * Copyright (c) 2025 Weihao Li > > + */ > > + > > +/dts-v1/; > > +#include "rk3368.dtsi" > > +#include <dt-bindings/input/input.h> > > + > > +/ { > > + model = "YMD8_MB"; > > + compatible = "rockchip,YMD8_MB", "rockchip,rk3368"; > > The first compatible isn't right if Rockchip isn't the actual > manufacturer/seller of the whole product. I couldn't see anything on the > PCB picture you posted on your wiki but maybe there's some information > on the casing or elsewhere? I do not find any manufacturer info for now, I will try to search later. > > [...] > > > diff --git a/arch/arm/dts/rk3368.dtsi b/arch/arm/dts/rk3368.dtsi > > index 4c64fbefb4..77ff551681 100644 > > --- a/arch/arm/dts/rk3368.dtsi > > +++ b/arch/arm/dts/rk3368.dtsi > > Nope, this we won't allow. > > If you add support for a board, don't modify the device tree of the SoC > in the same patch. Please separate those changes from the patch adding > supoprt for the board. > > Here, because the rk3368.dtsi already exists, you'll need a patch per > logical thing you are adding. There are a few I could notice here: > - hdmi > - lvds > - rgb > - edp > - dsi > - vop > > First, only add what you tested. I believe this would be DSI+VOP only? > It's reasonable, I'll fix that. > Second, where did you get all the info? I don't see anything in upstream > Linux's rk3368.dtsi so it must come from somewhere else? Where? Please > specify this in the commit logs. it would be very nice to have this sent > and reviewed by the Linux kernel community as well. > I cross-referenced the downstream U-Boot and Linux repositories of Rockchip and made some reasonable inferences based on drivers for the same series of SoCs. I will add this information to commit log at next submit. > [...] > > > diff --git a/arch/arm/mach-rockchip/rk3368/Kconfig > > b/arch/arm/mach-rockchip/rk3368/Kconfig > > index a7be30bbd8..1d05a6784f 100644 > > --- a/arch/arm/mach-rockchip/rk3368/Kconfig > > +++ b/arch/arm/mach-rockchip/rk3368/Kconfig > > @@ -21,6 +21,11 @@ config TARGET_EVB_PX5 > > HDMI video input/output interface, audio codec ES8396, > > WIFI/BT (on RTL8723BS), Gsensor BMA250E and light&proximity > > sensor STK3410. > > + > > +config TARGET_YMD8_MB > > + select BOARD_EARLY_INIT_R > > + bool "YMD8_MB board" > > + > > Please add a help text so we have some clue what this board actually is. > Got it. > [...] > > > diff --git a/board/rockchip/ymd8_mb/MAINTAINERS > > b/board/rockchip/ymd8_mb/MAINTAINERS > > new file mode 100644 > > index 0000000000..a5156b8be3 > > --- /dev/null > > +++ b/board/rockchip/ymd8_mb/MAINTAINERS > > @@ -0,0 +1,6 @@ > > +RK3368 YMD8_MB Board > > +M: Weihao Li <cn.liwei...@gmail.com> > > +S: Maintained > > +F: board/rockchip/ymd8_mb_rk3368/ > > This is also missing the new device tree you added which you need to > maintain. Got it > > > +F: include/configs/ymd8_mb.h > > +F: configs/ymd8-mb_defconfig > > diff --git a/board/rockchip/ymd8_mb/Makefile > > b/board/rockchip/ymd8_mb/Makefile > > new file mode 100644 > > index 0000000000..a3a34edb43 > > --- /dev/null > > +++ b/board/rockchip/ymd8_mb/Makefile > > @@ -0,0 +1,7 @@ > > +# > > +# (C) Copyright 2016 Rockchip Electronics Co., Ltd > > +# > > +# SPDX-License-Identifier: GPL-2.0+ > > +# > > + > > +obj-y += ymd8_mb_rk3368.o > > diff --git a/board/rockchip/ymd8_mb/README b/board/rockchip/ymd8_mb/README > > new file mode 100644 > > index 0000000000..de980f2f23 > > --- /dev/null > > +++ b/board/rockchip/ymd8_mb/README > > @@ -0,0 +1 @@ > > +see board/rockchip/sheep_rk3368/README > > I don't think that's right. > > Remove this file and add a proper entry in doc/board/rockchip for this > board (or its own documentation file like we did for Theobroma boards, > up to you if there are important details that typically differ from > other Rockchip boards). I forgot to modify it when I separate patch, I'll adjust that. > > Cheers, > Quentin Best regards, WeiHao
