Hi Peter, Le ven. 19 avr. 2024 à 16:08, Peter Maydell <peter.mayd...@linaro.org> a écrit : > > 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.
Alright, that makes sense, is that OK if I send a patch removing the assert ? Thanks, > > thanks > -- PMM