On Wed, Jun 14, 2023 at 05:28:08PM -0700, Stephen Boyd wrote: > Quoting Christian Marangi (2023-05-29 05:34:57) > > On Mon, May 29, 2023 at 02:12:23PM +0200, Konrad Dybcio wrote: > > > On 28.05.2023 14:37, Christian Marangi wrote: > > > > On Sat, May 27, 2023 at 06:11:16PM +0200, Konrad Dybcio wrote: > > > >> On 27.04.2023 17:07, Christian Marangi wrote: > > > >>> + * Force the first conf if we can't find a correct config. > > > >>> + */ > > > >>> + if (unlikely(i == f->num_confs)) > > > >>> + best_conf = f->confs; > > > >> Is that a supported scenario or would it be a device driver / clock > > > >> driver error? > > > >> > > > > > > > > It's to handle case for the 2 continue in the loop and arriving in a > > > > situation where best_conf was never set? > > > > > > > > Should we return a warning and an ERR_PTR? Idea was to provide a best > > > > effort selection. > > > Hm.. I'm not sure what's the expected behavior here.. Stephen? > > > > > > > I have this implementation rady, if you want I can send this revision > > and discuss that in v5 directly. It's WARN and returning -EINVAL. > > I'd only have a WARN if you never expect to hit that case. Otherwise, it > should return -EINVAL and not warn. At a quick glance it sounds like > some sort of rounding policy, so just make sure the > round_rate/determine_rate implementation agrees with what set_rate() > will do and it should be good. Hi, in theory the WAN path should never happen, as it means no parent for the clk are present. So I guess with your logic printing a WARN is correct. About the rounding policy this is more or less problematic, it's a CLOSEST policy, so not a CEIL or FLOOR one. The determine_rate apply the very same selection logic of set_rate so also that should be good. I sent v5 some time ago with the concern here addressed so if you have time it would be good if you can check that. -- Ansuel
