[PATCH v7 1/5] dt-bindings: clock: qcom: Add CMN PLL clock controller for IPQ SoC

[Jie Luo]

On 12/22/2024 4:19 PM, Krzysztof Kozlowski wrote:
> On Fri, Dec 20, 2024 at 09:22:42PM +0800, Luo Jie wrote:
>> The CMN PLL controller provides clocks to networking hardware blocks
>> and to GCC on Qualcomm IPQ9574 SoC. It receives input clock from the
>> on-chip Wi-Fi, and produces output clocks at fixed rates. These output
>> rates are predetermined, and are unrelated to the input clock rate.
>> The primary purpose of CMN PLL is to supply clocks to the networking
>> hardware such as PPE (packet process engine), PCS and the externally
>> connected switch or PHY device. The CMN PLL block also outputs fixed
>> rate clocks to GCC, such as 24 MHZ as XO clock and 32 KHZ as sleep
>> clock supplied to GCC.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
>> Signed-off-by: Luo Jie <quic_luoj at quicinc.com>
>> ---
>>   .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       | 85 ++++++++++++++++++++++
>>   include/dt-bindings/clock/qcom,ipq-cmn-pll.h       | 22 ++++++
>>   2 files changed, 107 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> new file mode 100644
>> index 000000000000..db8a3ee56067
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> @@ -0,0 +1,85 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,ipq9574-cmn-pll.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm CMN PLL Clock Controller on IPQ SoC
>> +
>> +maintainers:
>> +  - Bjorn Andersson <andersson at kernel.org>
>> +  - Luo Jie <quic_luoj at quicinc.com>
>> +
>> +description:
>> +  The CMN (or common) PLL clock controller expects a reference
>> +  input clock. This reference clock is from the on-board Wi-Fi.
>> +  The CMN PLL supplies a number of fixed rate output clocks to
>> +  the devices providing networking functions and to GCC. These
>> +  networking hardware include PPE (packet process engine), PCS
>> +  and the externally connected switch or PHY devices. The CMN
>> +  PLL block also outputs fixed rate clocks to GCC. The PLL's
>> +  primary function is to enable fixed rate output clocks for
>> +  networking hardware functions used with the IPQ SoC.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,ipq9574-cmn-pll
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: The reference clock. The supported clock rates include
>> +          25000000, 31250000, 40000000, 48000000, 50000000 and 96000000 HZ.
>> +      - description: The AHB clock
>> +      - description: The SYS clock
>> +    description:
>> +      The reference clock is the source clock of CMN PLL, which is from the
>> +      Wi-Fi. The AHB and SYS clocks must be enabled to access CMN PLL
>> +      clock registers.
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ref
>> +      - const: ahb
>> +      - const: sys
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +
>> +  assigned-clocks:
> 
> Drop

OK.

> 
>> +    maxItems: 1
>> +
>> +  assigned-clock-rates-u64:
>> +    maxItems: 1
> 
> These wasn't here when you received review. Adding new properties always
> invalidates review.
> 
> No, drop them.

OK.

I will remove the reviewed-by tag next time when such modifications are
made.

> 
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - "#clock-cells"
>> +  - assigned-clocks
> 
> Drop

OK.

> 
>> +  - assigned-clock-rates-u64
> 
> Drop... or explain

I will drop it as the core DTS schema allows it for the clock
provider.

The DT property 'assigned-clock-rates-u64' is used for configuring the
clock rate of CMN PLL to 12 GHZ, which is the working clock rate of CMN
PLL on IPQ9574.

> 
> Drop all review tags after making significant changes like that.

Sure I will.

> 
> Best regards,
> Krzysztof
>