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: <c340ef39-0fbc-22d1-c05a-8e6a7d29325d@ti.com>
Date:   Fri, 21 Jul 2023 18:56:10 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v5 3/5] arm64: dts: ti: k3-j784s4: Add WIZ and SERDES PHY
 nodes
From:   Jayesh Choudhary <j-choudhary@ti.com>
To:     Nishanth Menon <nm@ti.com>, Andrew Davis <afd@ti.com>
CC:     <vigneshr@ti.com>, <krzysztof.kozlowski+dt@linaro.org>,
        <s-vadapalli@ti.com>, <kristo@kernel.org>, <robh+dt@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230710101705.154119-1-j-choudhary@ti.com>
 <20230710101705.154119-4-j-choudhary@ti.com>
 <20230712141828.lnpo4mhd5dv34rlz@census>
 <18310450-05f3-172c-e4bc-fda114f333a4@ti.com>
 <20230713182107.ashuygyg4x4j77s5@backboard>
 <af071a46-2e8e-a107-b0d8-d0bb2f845486@ti.com>
 <20230713185835.ek5jskqyengvba56@ascertain>
 <9f5d52fc-f9da-1429-1f97-bdec16d80a43@ti.com>
Content-Language: en-US
In-Reply-To: <9f5d52fc-f9da-1429-1f97-bdec16d80a43@ti.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 20/07/23 15:06, Jayesh Choudhary wrote:
> 
> 
> On 14/07/23 00:28, Nishanth Menon wrote:
>> On 13:31-20230713, Andrew Davis wrote:
>>> On 7/13/23 1:21 PM, Nishanth Menon wrote:
>>>> On 21:01-20230713, Jayesh Choudhary wrote:
>>>>>
>>>>>
>>>>> On 12/07/23 19:48, Nishanth Menon wrote:
>>>>>> On 15:47-20230710, Jayesh Choudhary wrote:
>>>>>>> From: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>>>>
>>>>>>> J784S4 SoC has 4 Serdes instances along with their respective WIZ
>>>>>>> instances. Add device-tree nodes for them and disable them by 
>>>>>>> default.
>>>>>>>
>>>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>>>> [j-choudhary@ti.com: fix serdes_wiz clock order]
>>>>>>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>>>>>>> ---
>>>>>> NAK. This patch introduces the following dtbs_check warning.
>>>>>> arch/arm64/boot/dts/ti/k3-am69-sk.dtb: serdes-refclk: 
>>>>>> 'clock-frequency' is a required property
>>>>>>
>>>>>
>>>>> Sorry for this. This property was added in the final board file.
>>>>> I will fix it in the next revision.
>>>>> I will add '0' as clock-property in the main file similar to j721e[1]
>>>>> which will be overridden in the board file with required value to get
>>>>> rid of this warning.
>>>>
>>>> That would follow what renesas (r8a774a1.dtsi) and imx
>>>> (imx8dxl-ss-conn.dtsi) seem to be doing as well. Just make sure to add
>>>> documentation to the property to indicate expectation. Unless someone
>>>> has objections to this approach.
>>>>
>>>
>>> Would it work better to disable these nodes, only enabling them in the
>>> board files when a real clock-frequency can be provided?
>>>
>>> My initial reaction would be to move the whole external reference clock
>>> node to the board file since that is where it is provided, but seems
>>> that would cause more churn in serdes_wiz* nodes than we would want..
>>
>> I would prefer that as well, but I have'nt gone around looking for
>> similar examples on other SoCs (Jayesh, can you check?). One other
>> approach (alipine and few other places) has been for the bootloader to
>> update the property set in dtb as 0, which is not needed in this case
>> to the best of what I see.. just hoping we use a technique that most
>> board folks are familiar with across SoCs.
>>
> 
> 
> I can see the clock nodes in board files for some vendors. But like
> Andrew said, that would cause issues with serdes_wiz node in the
> main.dtsi.
> 
> So I think it would be better to keep the external clock node in main
> dtsi itself. I will add comments to the property to indicate that this
> value will be over-written in board dts.
> Posting v6 to address these comments and others on the series.
> 

FYI..
I posted v6: 
https://lore.kernel.org/all/20230721132029.123881-1-j-choudhary@ti.com/
And I am using status property instead to address the dtbs_warning here.

Warm Regards,
Jayesh