On Mon, Sep 02, 2024 at 11:33:57PM GMT, Jie Luo wrote: > > > On 8/31/2024 6:24 AM, Stephen Boyd wrote: > > Quoting Jie Luo (2024-08-30 09:14:28) > > > Hi Stephen, > > > Please find below a minor update to my earlier message on clk_ops usage. > > > > Ok. Next time you can trim the reply to save me time. > > OK. > > > > > > On 8/28/2024 1:44 PM, Jie Luo wrote: > > > > On 8/28/2024 7:50 AM, Stephen Boyd wrote: > > > > > Quoting Luo Jie (2024-08-27 05:46:00) > > > > > > + case 48000000: > > > > > > + val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7); > > > > > > + break; > > > > > > + case 50000000: > > > > > > + val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 8); > > > > > > + break; > > > > > > + case 96000000: > > > > > > + val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7); > > > > > > + val &= ~CMN_PLL_REFCLK_DIV; > > > > > > + val |= FIELD_PREP(CMN_PLL_REFCLK_DIV, 2); > > > > > > + break; > > > > > > + default: > > > > > > + return -EINVAL; > > > > > > + } > > > > > > > > > > Why isn't this done with struct clk_ops::set_rate() or clk_ops::init()? > > > > > > > > OK, I will move this code into the clk_ops::init(). > > > > > > This code is expected to be executed once for initializing the CMN PLL > > > to enable output clocks, and requires the parent clock rate to be > > > available. However the parent clock rate is not available in the > > > clk_ops::init(). Hence clk_ops::set_rate() seems to be the right option > > > for this. Please let us know if this approach is fine. Thanks. > > > > Sure. It actually sounds like the PLL has a mux to select different > > reference clks. Is that right? If so, it seems like there should be > > multiple 'clocks' for the DT property and many parents possible. If > > that's the case then it should be possible to have something like > > > > clocks = <0>, <&refclk>, <0>; > > > > in the DT node and then have clk_set_rate() from the consumer actually > > set the parent index in hardware. If that's all static then it can be > > done with assigned-clock-parents or assigned-clock-rates. > > Thanks Stephen. The CMN PLL block always uses a single input reference > clock pin on any given IPQ SoC, however its rate may be different on > different IPQ SoC. For example, its rate is 48MHZ on IPQ9574 and 96MHZ > on IPQ5018. > > Your second suggestion seems more apt for this device. I can define the > DT property 'assigned-clock-parents' to configure the clock parent of > CMN PLL. The code for reference clock selection will be added in > clk_ops::set_parent(). Please let us know if this approach is fine. What is the source of this clock? Can you call clk_get_rate() on this input? -- With best wishes Dmitry
