Re: [PATCH RFC 3/3] clk: meson: t7: Model PLL pre-divider as a divider clock

[Jian Hu <jian.hu@amlogic.com>]

On 9/23/2026 7:26 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] clk: meson: t7: PLL pre-dividers permanently stuck at 0 due to missing CLK_SET_RATE_PARENT
> - [Low] clk: meson: build failure in drivers after struct meson_clk_pll_data change
> --
>
> commit 03f0a62ef49427b4fecb0de15b51e80cb45be623
> Author: Jian Hu <jian.hu@amlogic.com>
>
> clk: meson: t7: Model PLL pre-divider as a divider clock
>
> This commit replaces the dedicated PLL pre-divider with a standalone
> divider clock in the Meson T7 clock driver. The PLL DCO clocks are
> updated to take the new pre-divider clock as their parent instead of
> the input clock directly.
>
>> diff --git a/drivers/clk/meson/t7-pll.c b/drivers/clk/meson/t7-pll.c
>> index 0a622f45fa368..049f0c879c458 100644
>> --- a/drivers/clk/meson/t7-pll.c
>> +++ b/drivers/clk/meson/t7-pll.c
>> @@ -71,6 +71,34 @@
>>   #define MCLK_PLL_CNTL4               0x10
>>   #define MCLK_PLL_STS         0x14
>>
>> +static const struct clk_div_table t7_prediv_div_table[] = {
>> +     { .val = 1, .div = 1 },
>> +     { /* sentinel */ }
>> +};
>> +
>> +static struct clk_regmap t7_gp0_prediv = {
>> +     .data = &(struct clk_regmap_div_data){
>> +             .offset = GP0PLL_CTRL0,
>> +             .shift = 10,
>> +             .width = 5,
>> +             .table = t7_prediv_div_table,
>> +             /*
>> +              * The hardware reset value is 0. Allow it during clock registration
>> +              * to avoid a warning from the common divider code.
>> +              * set_rate() will program the valid divider value (1).
>> +              */
>> +             .flags = CLK_DIVIDER_ALLOW_ZERO,
> [ ... ]
>
>> @@ -97,11 +125,6 @@ static struct clk_regmap t7_gp0_pll_dco = {
>>                        .shift   = 0,
>>                        .width   = 8,
>>                },
>> -             .n = {
>> -                     .reg_off = GP0PLL_CTRL0,
>> -                     .shift   = 10,
>> -                     .width   = 5,
>> -             },
> [Severity: Low]
> When removing the .n parameter initialization here (and presumably from
> struct meson_clk_pll_data in a prerequisite commit), does this patch series
> also update other Meson drivers like drivers/clk/meson/gxbb.c and
> drivers/clk/meson/axg.c?
>
> Looking at other drivers in the tree, they appear to still statically
> initialize .n in their declarations:
>
> drivers/clk/meson/gxbb.c:gxbb_fixed_pll_dco() {
>          ...
>          .n = {
>              .reg_off = HHI_MPLL_CNTL,
>              .shift   = 9,
>              .width   = 5,
>          },
>          ...
> }
>
> Will leaving these initializations in place cause a build failure with a
> 'has no member' error for those drivers?


  I will include clk-pll.c and all the PLLs in a single patch.

>>                .l = {
>>                        .reg_off = GP0PLL_STS,
>>                        .shift   = 31,
>> @@ -119,8 +142,8 @@ static struct clk_regmap t7_gp0_pll_dco = {
>>        .hw.init = &(struct clk_init_data){
>>                .name = "gp0_pll_dco",
>>                .ops = &meson_clk_pll_ops,
>> -             .parent_data = &(const struct clk_parent_data) {
>> -                     .fw_name = "in0",
>> +             .parent_hws = (const struct clk_hw *[]) {
>> +                     &t7_gp0_prediv.hw
>>                },
>>                .num_parents = 1,
>>        },
> [Severity: High]
> Since the pre-divider relies on .set_rate() to correct the invalid
> hardware reset value of 0, will the common clock framework ever actually
> call .set_rate() on the pre-divider?
>
> Without adding the CLK_SET_RATE_PARENT flag here on the DCO clock
> (t7_gp0_pll_dco), rate requests to the DCO won't propagate up to the
> pre-divider. Could this leave the pre-divider stuck at the invalid 0 state
> and permanently prevent the PLL from functioning?


Ok, I will add CLK_SET_RATE_PARENT flag for DCO clock in the next version.

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260923-meson_refactor_n-v1-0-3a8ce27121a2@amlogic.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic