Messages in this thread |  | | Subject | Re: [PATCH v14 1/1] clk: npcm8xx: add clock controller | From | Stephen Boyd <> | Date | Fri, 16 Dec 2022 10:44:00 -0800 |
| |
Quoting Tomer Maimon (2022-12-11 12:43:24) > diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c > new file mode 100644 > index 000000000000..08ee7bea6f3a > --- /dev/null > +++ b/drivers/clk/clk-npcm8xx.c > @@ -0,0 +1,650 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* [...] > +#define NPCM8XX_CLK_S_RCP "rcp" > + > +static const u32 pll_mux_table[] = { 0, 1, 2, 3 }; > +static const struct clk_parent_data pll_mux_parents[] = { > + { .fw_name = NPCM8XX_CLK_S_PLL0, .name = NPCM8XX_CLK_S_PLL0 },
As this is a new driver either you should only have .fw_name here. The .name field is a backup to migrate code over to a new binding. When .fw_name is used there should be an associated DT binding update. I doubt the usage of .fw_name is correct though, because aren't these clks internal to the controller? The .fw_name field is about describing parents that are an input to the clk controller node in DT (because the controller is a consumer of these clks that are external to the device).
So can you use the .hw field for these internal clks? Check out CLK_HW_INIT_HWS() macro and friends for a possible way to initialize this.
> + { .fw_name = NPCM8XX_CLK_S_PLL1, .name = NPCM8XX_CLK_S_PLL1 }, > + { .fw_name = NPCM8XX_CLK_S_REFCLK, .name = NPCM8XX_CLK_S_REFCLK },
Maybe this is external? If so, it would be great to have this in the binding as a `clocks` property.
|  |