Re: [PATCH 1/4] dt-bindings: clock: qcom: Add common PLL clock controller for IPQ SoC

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On 08/08/2024 16:03, Luo Jie wrote:
> The common PLL controller provides clocks to networking hardware
> blocks on Qualcomm IPQ 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
> output clocks are supplied to the Ethernet hardware such as PPE
> (packet process engine) and the externally connected switch or PHY
> device.
> 
> The common PLL driver is initially being supported for IPQ9574 SoC.

Drop references to driver and explain the hardware.

Above with the usage of "common" looks like this is all for some common
driver, not for particular hardware.

> 
> Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
> ---
>  .../bindings/clock/qcom,ipq-cmn-pll.yaml           | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq-cmn-pll.yaml
> new file mode 100644
> index 000000000000..c45b3a201751
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq-cmn-pll.yaml

Use compatible as filename.

> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,ipq-cmn-pll.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Common PLL Clock Controller on IPQ SoC
> +
> +maintainers:
> +  - Bjorn Andersson <andersson@xxxxxxxxxx>
> +  - Luo Jie <quic_luoj@xxxxxxxxxxx>
> +
> +description:
> +  The 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 Ethernet
> +  devices including PPE (packet process engine) and the connected
> +  switch or PHY device.
> +
> +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 common PLL
> +      clock registers.
> +
> +  clock-names:
> +    items:
> +      - const: ref
> +      - const: ahb
> +      - const: sys
> +
> +  clock-output-names:
> +    items:
> +      - const: ppe-353mhz
> +      - const: eth0-50mhz
> +      - const: eth1-50mhz
> +      - const: eth2-50mhz
> +      - const: eth-25mhz

Drop entire property. If the names are fixed, what's the point of having
it in DTS? There is no.

Best regards,
Krzysztof