Messages in this thread | | | From | Tomer Maimon <> | Date | Wed, 25 Jan 2023 18:18:47 +0200 | Subject | Re: [PATCH v14 1/1] clk: npcm8xx: add clock controller |
| |
Hi Stephen,
Kind reminder regarding the NPCM clock questions
Thanks,
Tomer
On Tue, 17 Jan 2023 at 19:35, Tomer Maimon <tmaimon77@gmail.com> wrote: > > Hi Stephen, > > Very sorry for the late reply. > > On Fri, 16 Dec 2022 at 20:44, Stephen Boyd <sboyd@kernel.org> wrote: > > > > 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 > What do you mean by associated DT binding? does the.fw_name, for > example, NPCM8XX_CLK_S_PLL0 need to represent in the device tree? > > 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 does the > yes the PLL clocks are internal. > > 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. > but still, I have used devm_clk_hw_register_mux_parent_data_table > function to register the clock mux, > should I use devm_clk_hw_register_mux_parent_hws function instead? > Does this modification need to be done in all the mux parent struct? > could you point me to some example in the Linux kernel how do you > think that I should represent the mux clock in the NPCM8XX clock > driver? > > > > > + { .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. > O.K. > > Thanks, > > Tomer
| |