Re: [PATCH 2/4] clk: qcom: cmnpll: Add IPQ5424 SoC support

[Jie Luo <quic_luoj@quicinc.com>]

On 3/25/2025 7:42 AM, Stephen Boyd wrote:
> Quoting Luo Jie (2025-03-21 05:49:53)
>> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
>> index 432d4c4b7aa6..ce5e83124c6d 100644
>> --- a/drivers/clk/qcom/ipq-cmn-pll.c
>> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   /*
>> @@ -16,6 +16,10 @@
>>    * are supplied to GCC (24 MHZ as XO and 32 KHZ as sleep clock), and to PCS
>>    * with 31.25 MHZ.
>>    *
>> + * On the IPQ5424 SoC, there is an output clock from CMN PLL to PPE at 375 MHZ,
>> + * and an output clock to NSS at 300 MHZ. The other output clocks from CMN PLL
>> + * on IPQ5424 are the same as IPQ9574.
>> + *
>>    *               +---------+
>>    *               |   GCC   |
>>    *               +--+---+--+
>> @@ -115,6 +119,20 @@ static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
>>          CLK_PLL_OUTPUT(ETH1_50MHZ_CLK, "eth1-50mhz", 50000000UL),
>>          CLK_PLL_OUTPUT(ETH2_50MHZ_CLK, "eth2-50mhz", 50000000UL),
>>          CLK_PLL_OUTPUT(ETH_25MHZ_CLK, "eth-25mhz", 25000000UL),
>> +       { /* Sentinel */ },
> 
> Nitpick: Drop the comma here so nothing can come after the sentinel.

Understand, I will remove it.

> 
>> +};
>> +
>> +static const struct cmn_pll_fixed_output_clk ipq5424_output_clks[] = {
>> +       CLK_PLL_OUTPUT(XO_24MHZ_CLK, "xo-24mhz", 24000000UL),
>> +       CLK_PLL_OUTPUT(SLEEP_32KHZ_CLK, "sleep-32khz", 32000UL),
>> +       CLK_PLL_OUTPUT(PCS_31P25MHZ_CLK, "pcs-31p25mhz", 31250000UL),
>> +       CLK_PLL_OUTPUT(NSS_300MHZ_CLK, "nss-300mhz", 300000000UL),
>> +       CLK_PLL_OUTPUT(PPE_375MHZ_CLK, "ppe-375mhz", 375000000UL),
>> +       CLK_PLL_OUTPUT(ETH0_50MHZ_CLK, "eth0-50mhz", 50000000UL),
>> +       CLK_PLL_OUTPUT(ETH1_50MHZ_CLK, "eth1-50mhz", 50000000UL),
>> +       CLK_PLL_OUTPUT(ETH2_50MHZ_CLK, "eth2-50mhz", 50000000UL),
>> +       CLK_PLL_OUTPUT(ETH_25MHZ_CLK, "eth-25mhz", 25000000UL),
>> +       { /* Sentinel */ },
> 
> Nitpick: Drop the comma here so nothing can come after the sentinel.
> 

OK.

>>   };
>>   
>>   /*
>