> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Tuesday, July 19, 2022 1:36 AM > To: Shenwei Wang <shenwei.wang@xxxxxxx>; Rob Herring > <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; > Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team > <kernel@xxxxxxxxxxxxxx>; Peng Fan <peng.fan@xxxxxxx> > Cc: devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > Subject: Re: [EXT] Re: [PATCH RESEND v3 3/3] arm64: dts: imx: add imx8dxl > support > > Caution: EXT Email > > On 18/07/2022 20:50, Shenwei Wang wrote: > > Hi Krzysztof, > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > >> Sent: Monday, July 18, 2022 8:01 AM > >> To: Shenwei Wang <shenwei.wang@xxxxxxx>; Rob Herring > >> <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski > >> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; > >> Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team > >> <kernel@xxxxxxxxxxxxxx>; Peng Fan <peng.fan@xxxxxxx> > >> Cc: devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > >> 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%2Flor > >> > e.kern%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7Cf05fa50044f > c495 > >> > 748c808da695100a6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 37938 > >> > 093863475409%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2lu > >> > MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Zp > U2whYQ > >> GJ1z8%2Fnn0hFwHu5KP0BAjx5DTy612oJGwzw%3D&reserved=0 > >> el.org%2Fall%2F20220404134609.2676793-2- > >> > abel.vesa%40nxp.com%2F&data=05%7C01%7Cshenwei.wang%40nxp.com > >> %7C235450e576d44c030c1e08da68bd88de%7C686ea1d3bc2b4c6fa92cd99c > 5c3 > >> > 01635%7C0%7C0%7C637937460494602259%7CUnknown%7CTWFpbGZsb3d8ey > >> > JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > >> > 3000%7C%7C%7C&sdata=ZiDgTTZbcifNMBjDHTCdMKC1hgmf7BGzuvCJe%2 > >> FyagzQ%3D&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? I am sorry to introduce double effort for your review. When I took this task, I just found that this part of codes were not in the upstreaming kernel, and Abel didn't seem to push what he has done onto the internal repo yet. The process is to get the latest company release package, and do the upstreaming based on it. The code base may have some gaps to the upstreaming standard, but I am working on it. Thank you very much again for your time and the valuable comments. > > (...) > > >>> + 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. It is a little different here. Although the XTAL itself is on the board, a user has no choice here to select the clock frequency. It must use the 24MHz and 32KHz XTALs, and the two frequencies can't be changed. This is the mandatory requirement. > > Best regards, > Krzysztof
