Received-SPF: pass (google.com: domain of linux-kernel+bounces-16486-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <7c4f12d8-e336-41b9-b0ab-2a8ab3574036@quicinc.com>
Date: Thu, 4 Jan 2024 18:06:55 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 5/5] dt-bindings: net: ipq4019-mdio: Document ipq5332
 platform
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, <agross@kernel.org>,
        <andersson@kernel.org>, <konrad.dybcio@linaro.org>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <andrew@lunn.ch>, <hkallweit1@gmail.com>, <linux@armlinux.org.uk>,
        <robert.marko@sartura.hr>
CC: <linux-arm-msm@vger.kernel.org>, <netdev@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_srichara@quicinc.com>
References: <20231225084424.30986-1-quic_luoj@quicinc.com>
 <20231225084424.30986-6-quic_luoj@quicinc.com>
 <dee72ce8-b24e-467a-b265-1b965588807f@linaro.org>
 <aeb364a3-6c05-4a1b-ba32-e687a89f20f8@quicinc.com>
 <58dde1a7-ed4a-442c-bb5c-c3f6d926fb7e@linaro.org>
 <06ddbae8-1502-41fb-8cf8-9a3390dad557@quicinc.com>
 <28c8b31c-8dcb-4a19-9084-22c77a74b9a1@linaro.org>
 <d231b7e2-51f5-4676-8fc0-e4bc6154e2d7@quicinc.com>
 <c4831e26-5ff0-40b1-98d4-addfdc1ee5a8@linaro.org>
From: Jie Luo <quic_luoj@quicinc.com>
In-Reply-To: <c4831e26-5ff0-40b1-98d4-addfdc1ee5a8@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/4/2024 3:47 PM, Krzysztof Kozlowski wrote:
> On 28/12/2023 08:38, Jie Luo wrote:
>>>> Sorry for this confusion.
>>>> Rob said the internal reference source can be decided by the absence of
>>>> the property combined with compatible string, because i said the
>>>
>>> So all your three DT maintainers agree that lack of property for
>>> choosing clock, defines the usage of interrupt source.
>>
>> This is the reference clock source selection of CMN block, which
>> generates the clocks for the Ethernet devices.
>>
>>>
>>> Now we had huge amount of arguments that you do not represent properly
>>> the clock relationships. Still.
>>
>> here is the clock topology.
>> reference clock sources ---> CMN PLL ---> various output clocks
> 
> How do you guarantee that these clocks are enabled without proper
> relationships described in DT? In current and future designs?

Will update the patch to add the clock relationship in the DT, thanks.

> 
>>
>> the output clocks are provided to the Ethernet devices(such as the
>> qca808x PHY devices).
>>
>> These information is also provided the commit message of the patch
>> <net: mdio: ipq4019: configure CMN PLL clock for ipq5332>.
>>
>>>
>>>> internal 96MHZ is used on ipq5018 currently in the previous message.
>>>>
>>>> per double checked the current IPQ platforms, the internal 96MHZ is also
>>>> possible on ipq9574, and the reference clock source should be kept as
>>>> configurable instead of limited by the compatible string, maybe the
>>>> different reference clock source is acquired in the future, even
>>>> currently it is not used on the special platform for now.
>>>>
>>>> so i update the solution with a little bit of changes.
>>>
>>> You still do not want to implement our suggestions and I don't
>>> understand your arguments. Nothing in above paragraph explains me why
>>> you cannot use clock provider/consumer relationships.
>>
>> Hi Krzysztof,
>>
>> The reference clock source can be registered as the fix clock provider,
>>   From the current fix clock provider, the clock rate is useful for the
>> clock consumer, the fix clock rate is used to generate the output clocks
>> by the divider or multiplier.
>>
>> For the CMN block to select reference clock, which is configuring the
>> clock source, we don't know the formula to get the output clock value
>> based on the reference clock value.
> 
> I don't understand what does it mean. You do not know how to program CMN
> block?

The output clock value of CMN block is not related to the clock value of
the reference source clock, the output clocks of CMN block are fixed to
25M and 50M, which are provided to the different Ethernet devices, there
is no formula for the relationship of input clock value and output clock
value of CMN block.

> 
>>
>> i also see there is an example in the upstream code, which is same as
>> the CMN block to select the reference clock source.
> 
> Oh, the old argument. So if there is a bug in the code, you are going
> for example to implement it as well?

The reference source clock can be registered as fix clock, and we can
get the clock rate of reference source clock with the external or
internal flag to distinguish the reference clock source, then program
the CMN block corresponding.
> 
>>
>> the property "ref-clock-frequency" is defined in the yaml file below.
>> Documentation/devicetree/bindings/net/wireless/ti,wlcore.yaml.
> 
> And how does the hardware look like there? It's TI, so how do you even know?

According to the driver code and DT description, the example is also for
selecting the reference clock source according to the clock value of DT
properties.

Sure, we can also use the registered fix clock to identify the
reference clock source for CMN block.

> 
> 
> 
> Best regards,
> Krzysztof
>