linux-kernel - Re: [PATCH v4 3/3] arm64: dts: cn913x: add device trees for COM Express boards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 30 Oct 2023 14:32:53 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Elad Nachman <enachman@...vell.com>, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        andrew@...n.ch, gregory.clement@...tlin.com,
        sebastian.hesselbarth@...il.com, pali@...nel.org,
        mrkiko.rs@...il.com, chris.packham@...iedtelesis.co.nz,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Cc:     cyuval@...vell.com
Subject: Re: [PATCH v4 3/3] arm64: dts: cn913x: add device trees for COM
 Express boards

On 29/10/2023 18:48, Elad Nachman wrote:
> From: Elad Nachman <enachman@...vell.com>
> 
> Add support for CN9130 and CN9131 COM Express Type 7 CPU
> module boards by Marvell.
> Define these COM Express CPU modules as dtsi and
> provide a dtsi file for a carrier board (Marvell AC5X RD
> COM Express type 7 carrier board).
> This Carrier board only utilizes the PCIe link, hence no
> special device / driver support is provided by this dtsi file.
> Finally, provide a dts file for the com express carrier and
> CPU module combination.
> 
> These COM Express boards differ from the existing CN913x DB
> boards by the type of ethernet connection (RGMII),
> the type of voltage regulators (not i2c expander based)
> and the USB phy (not UTMI based).
> Note - PHY + RGMII connector is OOB on CPU module.
> CN9131 COM Express board is basically CN9130 COM Express board
> with an additional CP115 I/O co-processor, which in this case
> provides an additional USB host controller on the board.
> 
> Signed-off-by: Elad Nachman <enachman@...vell.com>
> ---
>  arch/arm64/boot/dts/marvell/Makefile          |   1 +
>  .../marvell/ac5x-rd-carrier-with-cn9131.dts   |  20 ++++
>  .../boot/dts/marvell/ac5x-rd-carrier.dtsi     |  15 +++
>  .../dts/marvell/cn9130-db-comexpress.dtsi     | 101 ++++++++++++++++
>  .../dts/marvell/cn9131-db-comexpress.dtsi     | 113 ++++++++++++++++++
>  5 files changed, 250 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier-with-cn9131.dts
>  create mode 100644 arch/arm64/boot/dts/marvell/ac5x-rd-carrier.dtsi
>  create mode 100644 arch/arm64/boot/dts/marvell/cn9130-db-comexpress.dtsi
>  create mode 100644 arch/arm64/boot/dts/marvell/cn9131-db-comexpress.dtsi
> 
> diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
> index 79ac09b58a89..88c0f357a778 100644
> --- a/arch/arm64/boot/dts/marvell/Makefile
> +++ b/arch/arm64/boot/dts/marvell/Makefile
> @@ -26,4 +26,5 @@ dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db-B.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-A.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-B.dtb
> +dtb-$(CONFIG_ARCH_MVEBU) += ac5x-rd-carrier-with-cn9131.dtb
>  dtb-$(CONFIG_ARCH_MVEBU) += ac5-98dx35xx-rd.dtb
> diff --git a/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-with-cn9131.dts b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-with-cn9131.dts
> new file mode 100644
> index 000000000000..9ca2725184e2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/marvell/ac5x-rd-carrier-with-cn9131.dts
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2023 Marvell International Ltd.
> + *
> + * Device tree for the AC5X RD Type 7 Com Express carrier board,
> + * Utilizing the CN913x COM Express CPU module board.
> + * This specific board only maintains a PCIe link with the CPU CPU module
> + * module, which does not require any special DTS definitions.
> + */
> +
> +#include "cn9131-db-comexpress.dtsi"
> +#include "ac5x-rd-carrier.dtsi"
> +
> +/ {
> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board with CN9131 CPU module";
> +	compatible = "marvell,rd-ac5x-carrier-with-cn9131", "marvell,rd-ac5x-carrier",
> +		     "marvell,cn9131", "marvell,cn9130",
> +		     "marvell,armada-ap807-quad", "marvell,armada-ap807";

So you clearly did not test what you wrote.

This is v4, so at this point testing should be obvious. You must test
your bindings and you must test your DTS against bindings.

Successful test means: ZERO warnings.

Standard disclaimer:

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.



Best regards,
Krzysztof