Re: [EXT] Re: [PATCH RESEND v3 3/3] arm64: dts: imx: add imx8dxl support

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 18/07/2022 20:50, Shenwei Wang wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Monday, July 18, 2022 8:01 AM
>> To: Shenwei Wang <shenwei.wang@nxp.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>; Peng Fan <peng.fan@nxp.com>
>> Cc: devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
>> Subject: [EXT] Re: [PATCH RESEND v3 3/3] arm64: dts: imx: add imx8dxl support
>>
>> Caution: EXT Email
>>
>> On 16/07/2022 01:12, Shenwei Wang wrote:
>>> i.MX8DXL is a device targeting the automotive and industrial market
>>> segments. The chip is designed to achieve both high performance and
>>> low power consumption. It has a dual (2x) Cortex-A35 processor.
>>>
>>> This patch adds the imx8dxl soc and EVK board support.
>>
>> I saw this patch and I was already commenting it:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
>> el.org%2Fall%2F20220404134609.2676793-2-
>> abel.vesa%40nxp.com%2F&amp;data=05%7C01%7Cshenwei.wang%40nxp.com
>> %7C235450e576d44c030c1e08da68bd88de%7C686ea1d3bc2b4c6fa92cd99c5c3
>> 01635%7C0%7C0%7C637937460494602259%7CUnknown%7CTWFpbGZsb3d8ey
>> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>> 3000%7C%7C%7C&amp;sdata=ZiDgTTZbcifNMBjDHTCdMKC1hgmf7BGzuvCJe%2
>> FyagzQ%3D&amp;reserved=0
>>
>> Doing things twice, reviewing twice is waste of time. I actually spotted this
>> duplication after I perform a review, but this patch should be abandoned and
>> Abel's patches should rather go.
> 
> I am not sure if Abel is still working on this task. The goal is to get the imx8dxl supported by upstreaming kernel. As both patches were picked up from the company internal repo and modified for upstreaming requirements, I don't mind whose patches to get included.  Please let me know which one is easy for you to go ahead, and I can continue with Abel's patch if needed.

I just don't see the point for doing the review second time. Why sending
poor code based on internal repo instead of continuing something which
got review?

(...)

>>> +     xtal24m: clock-xtal24m {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <24000000>;
>>
>>
>> Didn't you ignore (again) comments?
> 
> The SoC requires two Crystal clock inputs, one is 24MHz and the other is 32KHz. The board design doesn't have an option to change the values. That's why we keep the nodes here.

It's the same everywhere, nothing new here. Where is the clock? On the
board. Not in the Soc. For convenience clock could be here, but at least
the frequency by convention is put to the board.

Best regards,
Krzysztof