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

[Jie Luo]

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.

> >   };
> >   
> >   /*