Messages in this thread | | | Date | Fri, 4 Nov 2022 11:49:34 -0500 | Subject | Re: [PATCH v2 54/65] clk: da8xx: clk48: Switch to determine_rate | From | David Lechner <> |
| |
On 11/4/22 8:18 AM, Maxime Ripard wrote: > The TI DA8xx USB0 clk48 clocks implements a mux with a set_parent > hook, but doesn't provide a determine_rate implementation. > > This is a bit odd, since set_parent() is there to, as its name implies, > change the parent of a clock. However, the most likely candidate to > trigger that parent change is a call to clk_set_rate(), with > determine_rate() figuring out which parent is the best suited for a > given rate. > > The other trigger would be a call to clk_set_parent(), but it's far less > used, and it doesn't look like there's any obvious user for that clock. > > So, the set_parent hook is effectively unused, possibly because of an > oversight. However, it could also be an explicit decision by the > original author to avoid any reparenting but through an explicit call to > clk_set_parent(). > > The driver does implement round_rate() though, which means that we can > change the rate of the clock, but we will never get to change the > parent. > > However, It's hard to tell whether it's been done on purpose or not. > > Since we'll start mandating a determine_rate() implementation, let's > convert the round_rate() implementation to a determine_rate(), which > will also make the current behavior explicit. And if it was an > oversight, the clock behaviour can be adjusted later on.
I think this one should be the same as the clk:davinci changes and not allow re-parenting. Since this is a USB 48MHz PHY clock, a rate change will never be requested.
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- > drivers/clk/davinci/da8xx-cfgchip.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/davinci/da8xx-cfgchip.c b/drivers/clk/davinci/da8xx-cfgchip.c > index 4c1cc59bba53..f60c97091818 100644 > --- a/drivers/clk/davinci/da8xx-cfgchip.c > +++ b/drivers/clk/davinci/da8xx-cfgchip.c > @@ -462,10 +462,12 @@ static unsigned long da8xx_usb0_clk48_recalc_rate(struct clk_hw *hw, > return 48000000; > } > > -static long da8xx_usb0_clk48_round_rate(struct clk_hw *hw, unsigned long rate, > - unsigned long *parent_rate) > +static int da8xx_usb0_clk48_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > { > - return 48000000; > + req->rate = 48000000; > + > + return 0; > } > > static int da8xx_usb0_clk48_set_parent(struct clk_hw *hw, u8 index) > @@ -494,7 +496,7 @@ static const struct clk_ops da8xx_usb0_clk48_ops = { > .disable = da8xx_usb0_clk48_disable, > .is_enabled = da8xx_usb0_clk48_is_enabled, > .recalc_rate = da8xx_usb0_clk48_recalc_rate, > - .round_rate = da8xx_usb0_clk48_round_rate, > + .determine_rate = da8xx_usb0_clk48_determine_rate, > .set_parent = da8xx_usb0_clk48_set_parent, > .get_parent = da8xx_usb0_clk48_get_parent, > }; >
| |