Re: [RFC PATCH 2/2] Add support for Videostrong KII Pro tv box

["Andreas Färber" <afaerber@suse.de>]

Am 15.11.19 um 17:50 schrieb Mohammad Rasim:
> This patch adds support for the Videostrong KII Pro tv box which is based on the gxbb-p201 reference design
> 
> Signed-off-by: Mohammad Rasim <mohammad.rasim96@gmail.com>
> ---
>  arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>  .../boot/dts/amlogic/meson-gxbb-KII-Pro.dts   | 25 +++++++++++++++++++
>  2 files changed, 26 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb-KII-Pro.dts
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index 84afecba9ec0..1d72e93691f2 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nexbox-a95x.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-odroidc2.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-p200.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-p201.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-KII-Pro.dtb

Lowercase: -kii-pro.dtb
And please sort alphabetically.

>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-pro.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-meta.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-telos.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-KII-Pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-KII-Pro.dts
> new file mode 100644
> index 000000000000..e79d75bfa8dd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-KII-Pro.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + */

What is this? :) You may want to add a copyright here.

> +
> +/dts-v1/;
> +
> +#include "meson-gxbb-p201.dts"

This is rather unusual, normally you would include the SoC's .dtsi, not
another board.

> +
> +/ {
> +	compatible = "Videostrong,KII-Pro","amlogic,p201", "amlogic,s905", "amlogic,meson-gxbb";

This is not a valid vendor prefix (undefined, should be lowercase), same
for the compatible string, which should be lowercase as all others.

Please run ./scripts/checkpatch.pl, which would warn you of such issues.

Also a space missing after comma.


> +	model = "Videostrong KII Pro";

Here you can spell it the original way, like you did.

> +
> +
> +};
> +
> +&uart_A {
> +	status = "okay";
> +	pinctrl-0 = <&uart_a_pins>, <&uart_a_cts_rts_pins>;
> +	pinctrl-names = "default";
> +	uart-has-rtscts;

Leave an empty line here for spacing.

> +	bluetooth {
> +		compatible = "brcm,bcm4335A0";
> +		shutdown-gpios = <&gpio GPIOX_20 GPIO_ACTIVE_HIGH>;
> +	};
> +};

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)