On Thu, 18 Apr 2024 at 21:39, Raphael Poggi <raphael.po...@lynxleap.co.uk> wrote: > > Hi Philippe, > > Le jeu. 18 avr. 2024 à 20:43, Philippe Mathieu-Daudé > <phi...@linaro.org> a écrit : > > > > Hi Raphael, > > > > On 18/4/24 21:16, Raphael Poggi wrote: > > > When dealing with few clocks depending with each others, sometimes > > > we might only want to update the multiplier/diviser on a specific clock > > > (cf clockB in drawing below) and call "clock_propagate(clockA)" to > > > update the childs period according to the potential new > > > multiplier/diviser values. > > > > > > +--------+ +--------+ +--------+ > > > | clockA | --> | clockB | --> | clockC | > > > +--------+ +--------+ +--------+ > > > > > > The actual code would not allow that because, since we cannot call > > > "clock_propagate" directly on a child, it would exit on the > > > first child has the period has not changed for clockB, only clockC is > > > > Typo "as the period has not changed"? > > That's a typo indeed, thanks! > > > > > Why can't you call clock_propagate() on a child? > > There is an assert "assert(clk->source == NULL);" in clock_propagate(). > If I am not wrong, clk->source is set when the clock has a parent.
I think that assertion is probably there because we didn't originally have the idea of a clock having a multiplier/divider setting. So the idea was that calling clock_propagate() on a clock with a parent would always be wrong, because the only reason for its period to change would be if the parent had changed, and if the parent changes then clock_propagate() should be called on the parent. We added mul/div later, and we (I) didn't think through all the consequences. If you change the mul/div settings on clockB in this example then you need to call clock_propagate() on it, so we should remove that assert(). Then when you change the mul/div on clockB you can directly clock_propagate(clockB), and I don't think you need this patch at that point. thanks -- PMM
