Re: [PATCH v2 1/8] dt-bindings: PCI: Add binding for qps615

[Krzysztof Kozlowski <krzk@kernel.org>]

On 08/08/2024 14:01, Manivannan Sadhasivam wrote:
> On Mon, Aug 05, 2024 at 07:18:04PM +0200, Krzysztof Kozlowski wrote:
>> On 05/08/2024 19:07, Bjorn Andersson wrote:
>>> On Mon, Aug 05, 2024 at 09:41:26AM +0530, Krishna Chaitanya Chundru wrote:
>>>> On 8/4/2024 2:23 PM, Krzysztof Kozlowski wrote:
>>>>> On 03/08/2024 05:22, Krishna chaitanya chundru wrote:
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,qps615.yaml b/Documentation/devicetree/bindings/pci/qcom,qps615.yaml
>>> [..]
>>>>>> +  qps615,axi-clk-freq-hz:
>>>>>> +    description:
>>>>>> +      AXI clock which internal bus of the switch.
>>>>>
>>>>> No need, use CCF.
>>>>>
>>>> ack
>>>
>>> This is a clock that's internal to the QPS615, so there's no clock
>>> controller involved and hence I don't think CCF is applicable.
>>
>> AXI does not sound that internal.
> 
> Well, AXI is applicable to whatever entity that implements it. We mostly seen it
> in ARM SoCs (host), but in this case the PCIe switch also has a microcontroller
> /processor of some sort, so AXI is indeed relevant for it. The naming actually
> comes from the switch's i2c register name that is being configured in the driver
> based on this property value.
> 
>> DT rarely needs to specify internal
>> clock rates. What if you want to define rates for 20 clocks? Even
>> clock-frequency is deprecated, so why this would be allowed?
>> bus-frequency is allowed for buses, but that's not the case here, I guess?
>>
> 
> This clock frequency is for the switch's internal AXI bus that runs at default
> 200MHz. And this property is used to specify a frequency that is configured over
> the i2c interface so that the switch's AXI bus can operate in a low frequency
> there by reducing the power consumption of the switch.
> 
> It is not strictly needed for the switch operation, but for power optimization.
> So this property can also be dropped for the initial submission and added later
> if you prefer.

So if the clock rate can change, why this is static in DTB? Or why this
is configurable per-board?

There is a reason why clock-frequency property is not welcomed and you
are re-implementing it.

Best regards,
Krzysztof