| Message ID | 20220413071318.244912-1-codrin.ciubotariu@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: at91: generated: consider range when calculating best rate | expand |
On 13.04.2022 10:13, Codrin Ciubotariu wrote: > clk_generated_best_diff() helps in finding the parent and the divisor to > compute a rate closest to the required one. However, it doesn't take into > account the request's range for the new rate. Make sure the new rate > is within the required range. > > Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") > Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/clk/at91/clk-generated.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/clk/at91/clk-generated.c b/drivers/clk/at91/clk-generated.c > index 23cc8297ec4c..d429ba52a719 100644 > --- a/drivers/clk/at91/clk-generated.c > +++ b/drivers/clk/at91/clk-generated.c > @@ -117,6 +117,10 @@ static void clk_generated_best_diff(struct clk_rate_request *req, > tmp_rate = parent_rate; > else > tmp_rate = parent_rate / div; > + > + if (tmp_rate < req->min_rate || tmp_rate > req->max_rate) > + return; > + > tmp_diff = abs(req->rate - tmp_rate); > > if (*best_diff < 0 || *best_diff >= tmp_diff) { > -- > 2.32.0 >
Quoting Codrin Ciubotariu (2022-04-13 00:13:18) > clk_generated_best_diff() helps in finding the parent and the divisor to > compute a rate closest to the required one. However, it doesn't take into > account the request's range for the new rate. Make sure the new rate > is within the required range. > > Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") > Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> > --- Is this fixing anything real or it's just a thing that you noticed and sent a patch to fix? > drivers/clk/at91/clk-generated.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/clk/at91/clk-generated.c b/drivers/clk/at91/clk-generated.c > index 23cc8297ec4c..d429ba52a719 100644 > --- a/drivers/clk/at91/clk-generated.c > +++ b/drivers/clk/at91/clk-generated.c > @@ -117,6 +117,10 @@ static void clk_generated_best_diff(struct clk_rate_request *req, > tmp_rate = parent_rate; > else > tmp_rate = parent_rate / div; > + > + if (tmp_rate < req->min_rate || tmp_rate > req->max_rate) > + return; > + > tmp_diff = abs(req->rate - tmp_rate); > > if (*best_diff < 0 || *best_diff >= tmp_diff) {
On 22.04.2022 04:12, Stephen Boyd wrote: > Quoting Codrin Ciubotariu (2022-04-13 00:13:18) >> clk_generated_best_diff() helps in finding the parent and the divisor to >> compute a rate closest to the required one. However, it doesn't take into >> account the request's range for the new rate. Make sure the new rate >> is within the required range. >> >> Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") >> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> >> --- > > Is this fixing anything real or it's just a thing that you noticed and > sent a patch to fix? It fixes the clk_set_min/max_rate() calls to a generated clock. Do you want me to add this fact in the commit description? > >> drivers/clk/at91/clk-generated.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/clk/at91/clk-generated.c b/drivers/clk/at91/clk-generated.c >> index 23cc8297ec4c..d429ba52a719 100644 >> --- a/drivers/clk/at91/clk-generated.c >> +++ b/drivers/clk/at91/clk-generated.c >> @@ -117,6 +117,10 @@ static void clk_generated_best_diff(struct clk_rate_request *req, >> tmp_rate = parent_rate; >> else >> tmp_rate = parent_rate / div; >> + >> + if (tmp_rate < req->min_rate || tmp_rate > req->max_rate) >> + return; >> + >> tmp_diff = abs(req->rate - tmp_rate); >> >> if (*best_diff < 0 || *best_diff >= tmp_diff) {
Quoting Codrin.Ciubotariu@microchip.com (2022-04-26 00:24:15) > On 22.04.2022 04:12, Stephen Boyd wrote: > > Quoting Codrin Ciubotariu (2022-04-13 00:13:18) > >> clk_generated_best_diff() helps in finding the parent and the divisor to > >> compute a rate closest to the required one. However, it doesn't take into > >> account the request's range for the new rate. Make sure the new rate > >> is within the required range. > >> > >> Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") > >> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> > >> --- > > > > Is this fixing anything real or it's just a thing that you noticed and > > sent a patch to fix? > > It fixes the clk_set_min/max_rate() calls to a generated clock. Do you > want me to add this fact in the commit description? > I wanted to know if there are clk_set_min/max_rate() calls on this clk. Are there?
On 17.05.2022 10:14, Stephen Boyd wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Quoting Codrin.Ciubotariu@microchip.com (2022-04-26 00:24:15) >> On 22.04.2022 04:12, Stephen Boyd wrote: >>> Quoting Codrin Ciubotariu (2022-04-13 00:13:18) >>>> clk_generated_best_diff() helps in finding the parent and the divisor to >>>> compute a rate closest to the required one. However, it doesn't take into >>>> account the request's range for the new rate. Make sure the new rate >>>> is within the required range. >>>> >>>> Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") >>>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> >>>> --- >>> >>> Is this fixing anything real or it's just a thing that you noticed and >>> sent a patch to fix? >> >> It fixes the clk_set_min/max_rate() calls to a generated clock. Do you >> want me to add this fact in the commit description? >> > > I wanted to know if there are clk_set_min/max_rate() calls on this clk. > Are there? Yes, there are: https://elixir.bootlin.com/linux/latest/source/sound/soc/atmel/mchp-spdifrx.c#L450 Best regards, Codrin
Quoting Codrin Ciubotariu (2022-04-13 00:13:18) > clk_generated_best_diff() helps in finding the parent and the divisor to > compute a rate closest to the required one. However, it doesn't take into > account the request's range for the new rate. Make sure the new rate > is within the required range. > > Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") > Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> > --- Applied to clk-fixes
diff --git a/drivers/clk/at91/clk-generated.c b/drivers/clk/at91/clk-generated.c index 23cc8297ec4c..d429ba52a719 100644 --- a/drivers/clk/at91/clk-generated.c +++ b/drivers/clk/at91/clk-generated.c @@ -117,6 +117,10 @@ static void clk_generated_best_diff(struct clk_rate_request *req, tmp_rate = parent_rate; else tmp_rate = parent_rate / div; + + if (tmp_rate < req->min_rate || tmp_rate > req->max_rate) + return; + tmp_diff = abs(req->rate - tmp_rate); if (*best_diff < 0 || *best_diff >= tmp_diff) {
clk_generated_best_diff() helps in finding the parent and the divisor to compute a rate closest to the required one. However, it doesn't take into account the request's range for the new rate. Make sure the new rate is within the required range. Fixes: 8a8f4bf0c480 ("clk: at91: clk-generated: create function to find best_diff") Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> --- drivers/clk/at91/clk-generated.c | 4 ++++ 1 file changed, 4 insertions(+)