Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <15ddd798-873e-d90d-11e9-c6dd46ca03f4@linaro.org>
Date:   Fri, 12 Aug 2022 16:42:36 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH v4 2/2] arm64: dts: imx8mp-msc-sm2s: Add device trees for
 MSC SM2S-IMX8PLUS SoM and carrier board
Content-Language: en-US
To:     Martyn Welch <martyn.welch@collabora.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>
Cc:     kernel@collabora.com, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
References: <20220812084120.376042-1-martyn.welch@collabora.com>
 <20220812084120.376042-2-martyn.welch@collabora.com>
 <8962b7ed-a21c-0b7f-7a6d-5db3db84e4cb@linaro.org>
 <65a094d5d03ad8f7b35196c9dff6ffc6cf0ea151.camel@collabora.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <65a094d5d03ad8f7b35196c9dff6ffc6cf0ea151.camel@collabora.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 12/08/2022 14:35, Martyn Welch wrote:
> On Fri, 2022-08-12 at 12:47 +0300, Krzysztof Kozlowski wrote:
>> On 12/08/2022 11:41, Martyn Welch wrote:
>>> Add device trees for one of a number of MSC's (parent company,
>>> Avnet)
>>> variants of the SM2S-IMX8PLUS system on module along with the
>>> compatible
>>> SM2S-SK-AL-EP1 carrier board. As the name suggests, this family of
>>> SoMs use
>>> the NXP i.MX8MP SoC and provide the SMARC module interface.
>>>
>>> Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
>>> ---
>>>
>>> Changes in v2
>>>   - Added compatibles
>>>   - Removed underscores from node names
>>>   - Make node names more generic
>>>   - Reorder properties
>>>   - Fix issues found by dtbs_check in these files
>>>
>>> Changes in v3:
>>>   - Switched to avnet vendor string in compatibles
>>>   - Corrected patch description
>>>
>>> Changes in v4:
>>>   - Switched from phy-reset-gpios to reset-gpios, removing
>>> duplication
>>>   - Removed unneeded sdma1 node
>>>
>>>  arch/arm64/boot/dts/freescale/Makefile        |   1 +
>>>  .../freescale/imx8mp-msc-sm2s-14N0600E.dts    |  72 ++
>>>  .../dts/freescale/imx8mp-msc-sm2s-ep1.dts     |  53 ++
>>>  .../boot/dts/freescale/imx8mp-msc-sm2s.dtsi   | 812
>>> ++++++++++++++++++
>>>  4 files changed, 938 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-
>>> 14N0600E.dts
>>>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-
>>> ep1.dts
>>>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-
>>> sm2s.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/Makefile
>>> b/arch/arm64/boot/dts/freescale/Makefile
>>> index 8bf7f7ecebaa..139c8b95c9c9 100644
>>> --- a/arch/arm64/boot/dts/freescale/Makefile
>>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>>> @@ -83,6 +83,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-
>>> gw7902.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk2.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
>>> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-tqma8mpql-mba8mpxl.dtb
>>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-venice-gw74xx.dtb
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-
>>> 14N0600E.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-
>>> 14N0600E.dts
>>> new file mode 100644
>>> index 000000000000..9e976e8baaee
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-14N0600E.dts
>>> @@ -0,0 +1,72 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2022 Avnet Embedded GmbH
>>> + */
>>> +/dts-v1/;
>>> +
>>> +#include "imx8mp-msc-sm2s.dtsi"
>>> +
>>> +/ {
>>> +       model = "MSC SM2S-IMX8PLUS-QC6-14N0600E SoM";
>>> +       compatible = "avnet,sm2s-imx8mp-14N0600E", "avnet,sm2s-
>>> imx8mp",
>>> +                    "fsl,imx8mp";
>>
>> This does not match your bindings. Please test your DTS.
>>
> 
> Hi Krzysztof,
> 
> I'm not sure I follow. This is the DTS for the SoM. 

SoMs usually do not have DTSes because they cannot be run on their own.
SoMs almost always require a baseboard/carrier. Therefore this should
not be DTS, but that was not my comment.

> The only way I can
> test the SoM at the moment is on combination with the "EP1" carrier
> board. 

... so you basically say it cannot be a DTS.

> That has been tested. The strings match those specified in the
> bindings unless I'm being blind to something.

Test the DTS - make dtbs_check (there are several
variations/arguments/helpers):
Documentation/devicetree/bindings/writing-schema.rst

> 
> I guess I can build the DTB for just the SoM 

But you just did it, didn't you? This is a DTS.

> and boot with that or
> thinking about it, rename this as a .dtsi, given that it's unlikely
> that anyone is going to have a carrier barebones enough that it could
> be considered just the SoM?

Anyway, I wanted DT bindings tests for DTS. Not actual tests on
hardware, because the compatibles do not matter in that aspect.

Best regards,
Krzysztof