mbox series

[v2,0/4] Add CMN PLL clock controller driver for IPQ9574

Message ID 20240820-qcom_ipq_cmnpll-v2-0-b000dd335280@quicinc.com
Headers show
Series Add CMN PLL clock controller driver for IPQ9574 | expand

Message

Jie Luo Aug. 20, 2024, 2:02 p.m. UTC
The CMN PLL clock controller in Qualcomm IPQ chipsets provides
the clocks to the networking hardware blocks that are internal or
external to the SoC. This driver configures the CMN PLL clock
controller to enable the output clocks to such networking hardware
blocks. These networking blocks include the internal PPE (Packet
Process Engine), external connected Ethernet PHY, or external switch.
 
The controller expects the input reference clock from the internal
Wi-Fi block acting as the clock source. The output clocks supplied
by the controller are fixed rate clocks.

The CMN PLL hardware block does not include any other function other
than enabling the clocks to the networking hardware blocks.

The driver is being enabled to support IPQ9574 SoC initially, and
will be extended for other SoCs.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
Changes in v2:
- Rename the dt-binding file with the compatible.
- Remove property 'clock-output-names' from dt-bindings and define
  names in the driver. Add qcom,ipq-cmn-pll.h to export the output
  clock specifier.
- Alphanumeric ordering of 'cmn_pll_ref_clk' node in DTS.
- Fix allmodconfig error reported by test robot.
- Replace usage of "common" to "CMN" to match the name with the
  hardware specification.
- Clarify in commit message on scope of CMN PLL function.

- Link to v1: https://lore.kernel.org/r/20240808-qcom_ipq_cmnpll-v1-0-b0631dcbf785@quicinc.com

---
Luo Jie (4):
      dt-bindings: clock: qcom: Add CMN PLL clock controller for IPQ SoC
      clk: qcom: Add CMN PLL clock controller driver for IPQ SoC
      arm64: defconfig: Enable Qualcomm IPQ CMN PLL clock controller
      arm64: dts: qcom: Add CMN PLL node for IPQ9574 SoC

 .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       |  70 +++++++
 arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi   |   6 +-
 arch/arm64/boot/dts/qcom/ipq9574.dtsi              |  17 +-
 arch/arm64/configs/defconfig                       |   1 +
 drivers/clk/qcom/Kconfig                           |  10 +
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/clk-ipq-cmn-pll.c                 | 227 +++++++++++++++++++++
 include/dt-bindings/clock/qcom,ipq-cmn-pll.h       |  15 ++
 8 files changed, 345 insertions(+), 2 deletions(-)
---
base-commit: 222a3380f92b8791d4eeedf7cd750513ff428adf
change-id: 20240808-qcom_ipq_cmnpll-7c1119b25037

Best regards,

Comments

Jie Luo Aug. 21, 2024, 4:08 p.m. UTC | #1
On 8/21/2024 4:33 PM, Krzysztof Kozlowski wrote:
> On Tue, Aug 20, 2024 at 10:02:42PM +0800, Luo Jie wrote:
>> The CMN PLL controller provides clocks to networking hardware blocks
>> 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 output
>> clocks are supplied to the Ethernet hardware such as PPE (packet
>> process engine) and the externally connected switch or PHY device.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       | 70 ++++++++++++++++++++++
>>   include/dt-bindings/clock/qcom,ipq-cmn-pll.h       | 15 +++++
>>   2 files changed, 85 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..7ad04b58a698
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>> @@ -0,0 +1,70 @@
>> +# 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@kernel.org>
>> +  - Luo Jie <quic_luoj@quicinc.com>
>> +
>> +description:
>> +  The CMN PLL clock controller expects a reference input clock.
> 
> You did not explain what is CMN. Is this some sort of acronym?

CMN is short form for 'common'. Since it is referred to as 'CMN'
PLL in the hardware programming guides, we wanted the driver name
to include it as well. The description can be updated as below to
clarify the name and purpose of this hardware block. Hope this is
fine.

"The CMN PLL clock controller expects a reference input clock
from the on-board Wi-Fi, and supplies a number of fixed rate
output clocks to the Ethernet devices including PPE (packet
process engine) and the connected switch or PHY device. The
CMN (or 'common') PLL's only function is to enable clocks to
Ethernet hardware used with the IPQ SoC and does not include
any other function."

> 
> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski Aug. 22, 2024, 6:29 a.m. UTC | #2
On 21/08/2024 18:08, Jie Luo wrote:
> 
> 
> On 8/21/2024 4:33 PM, Krzysztof Kozlowski wrote:
>> On Tue, Aug 20, 2024 at 10:02:42PM +0800, Luo Jie wrote:
>>> The CMN PLL controller provides clocks to networking hardware blocks
>>> 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 output
>>> clocks are supplied to the Ethernet hardware such as PPE (packet
>>> process engine) and the externally connected switch or PHY device.
>>>
>>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>>> ---
>>>   .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       | 70 ++++++++++++++++++++++
>>>   include/dt-bindings/clock/qcom,ipq-cmn-pll.h       | 15 +++++
>>>   2 files changed, 85 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..7ad04b58a698
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>>> @@ -0,0 +1,70 @@
>>> +# 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@kernel.org>
>>> +  - Luo Jie <quic_luoj@quicinc.com>
>>> +
>>> +description:
>>> +  The CMN PLL clock controller expects a reference input clock.
>>
>> You did not explain what is CMN. Is this some sort of acronym?
> 
> CMN is short form for 'common'. Since it is referred to as 'CMN'
> PLL in the hardware programming guides, we wanted the driver name
> to include it as well. The description can be updated as below to
> clarify the name and purpose of this hardware block. Hope this is
> fine.
> 
> "The CMN PLL clock controller expects a reference input clock
> from the on-board Wi-Fi, and supplies a number of fixed rate
> output clocks to the Ethernet devices including PPE (packet
> process engine) and the connected switch or PHY device. The
> CMN (or 'common') PLL's only function is to enable clocks to
> Ethernet hardware used with the IPQ SoC and does not include
> any other function."

So the block is called "CMN" in hardware programming guide, without any
explanation of the acronym?

Best regards,
Krzysztof
Jie Luo Aug. 22, 2024, 1:52 p.m. UTC | #3
On 8/22/2024 2:29 PM, Krzysztof Kozlowski wrote:
> On 21/08/2024 18:08, Jie Luo wrote:
>>
>>
>> On 8/21/2024 4:33 PM, Krzysztof Kozlowski wrote:
>>> On Tue, Aug 20, 2024 at 10:02:42PM +0800, Luo Jie wrote:
>>>> The CMN PLL controller provides clocks to networking hardware blocks
>>>> 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 output
>>>> clocks are supplied to the Ethernet hardware such as PPE (packet
>>>> process engine) and the externally connected switch or PHY device.
>>>>
>>>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>>>> ---
>>>>    .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       | 70 ++++++++++++++++++++++
>>>>    include/dt-bindings/clock/qcom,ipq-cmn-pll.h       | 15 +++++
>>>>    2 files changed, 85 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..7ad04b58a698
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
>>>> @@ -0,0 +1,70 @@
>>>> +# 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@kernel.org>
>>>> +  - Luo Jie <quic_luoj@quicinc.com>
>>>> +
>>>> +description:
>>>> +  The CMN PLL clock controller expects a reference input clock.
>>>
>>> You did not explain what is CMN. Is this some sort of acronym?
>>
>> CMN is short form for 'common'. Since it is referred to as 'CMN'
>> PLL in the hardware programming guides, we wanted the driver name
>> to include it as well. The description can be updated as below to
>> clarify the name and purpose of this hardware block. Hope this is
>> fine.
>>
>> "The CMN PLL clock controller expects a reference input clock
>> from the on-board Wi-Fi, and supplies a number of fixed rate
>> output clocks to the Ethernet devices including PPE (packet
>> process engine) and the connected switch or PHY device. The
>> CMN (or 'common') PLL's only function is to enable clocks to
>> Ethernet hardware used with the IPQ SoC and does not include
>> any other function."
> 
> So the block is called "CMN" in hardware programming guide, without any
> explanation of the acronym?

Yes, I double checked again with our hardware team and the
documentation. CMN is just a short form of "common" with no additional
information in the guide.

Thanks for review.

> 
> Best regards,
> Krzysztof
>
Jie Luo Aug. 23, 2024, 3:15 p.m. UTC | #4
On 8/23/2024 12:12 AM, Ziyang Huang wrote:
>> Yes, I double checked again with our hardware team and the
>> documentation. CMN is just a short form of "common" with no additional
>> information in the guide.
> 
> Hi luo jie,
> 
> I'm a free developer who was trying to add the ethernet support for
> IPQ5018[1]. And I'm also trying to write the same driver in the V2 patch.
> 
> When I was trying to write this driver, I was also confused about the
> 'CMN' whcih I can't find any description.
> 
> But finally in WiFI documents, I found the same word explained as
> 'Component'. There may be different. But I think this is a better
> explanation than 'common'. So I named this driver to
> QCOM_ETH_CMN (Qualcomm Ethernet Component Driver).

Hi Ziyang,

We have confirmed from our SoC team that 'CMN' is used as a short
form of "common" for the CMN PLL block in the documentation. The
'component' reference that you may have found in the Wi-Fi document
should not apply to this Ethernet specific CMN PLL block. This CMN
PLL block provides a similar function on all IPQ SoCs including
IPQ5018.

Also, note that while this driver is initially enabled for IPQ9574
SoC, we plan to extend it to other SoC later once the driver is
accepted. Similarly we suggest enabling the IPQ5018 support for
CMN PLL on top of this driver.

> 
> Hope this can help something.
> 
> [1] https://lore.kernel.org/all/TYZPR01MB55563BD6A2B78402E4BB44D4C9762@TYZPR01MB5556.apcprd01.prod.exchangelabs.com/
>