On 23.05.2023 12:23, Dmitry Rokosov wrote: > Heiner, > > On Mon, May 22, 2023 at 10:35:59PM +0200, Heiner Kallweit wrote: >> On 22.05.2023 15:44, Dmitry Rokosov wrote: >>> Heiner, >>> >>> On Fri, May 19, 2023 at 06:10:50PM +0200, Heiner Kallweit wrote: >>>> On 18.05.2023 22:04, Martin Blumenstingl wrote: >>>>> Hi Dmitry, >>>>> >>>>> On Wed, May 17, 2023 at 12:34 PM Dmitry Rokosov >>>>> <ddrokosov@xxxxxxxxxxxxxx> wrote: >>>>> [...] >>>>>>>> Additionally, the CCF determines the best ancestor based on how close >>>>>>>> its rate is to the given one, based on arithmetic calculations. However, >>>>>>>> we have independent knowledge that a certain clock would be better, with >>>>>>>> less jitter and fewer intermediaries, which will likely improve energy >>>>>>>> efficiency. Sadly, the CCF cannot take this into account. >>>>>>> I agree that the implementation in CCF is fairly simple. There's ways >>>>>>> to trick it though: IIRC if there are multiple equally suitable clocks >>>>>>> it picks the first one. For me all of this has worked so far which is >>>>>>> what makes me curious in this case (not saying that anything is wrong >>>>>>> with your approach). >>>>>>> >>>>>>> Do you have a (real world) example where the RTC clock should be >>>>>>> preferred over another clock? >>>>>>> >>>>>> >>>>>> Yes, a real-life example is the need for a 32Khz clock for an external >>>>>> wifi chip. There is one option to provide this clock with high >>>>>> precision, which is RTC + GENCLK. >>>>>> >>>>>>> I'm thinking about the following scenario. >>>>>>> PWM parents: >>>>>>> - XTAL: 24MHz >>>>>>> - sys: not sure - let's say 166.67MHz >>>>>>> - RTC: 32kHz >>>>>>> >>>>>>> Then after that there's a divider and a gate. >>>>>>> >>>>>>> Let's say the PWM controller needs a 1MHz clock: it can take that from >>>>>>> XTAL or sys. Since XTAL is evenly divisible to 1MHz CCF will pick that >>>>>>> and use the divider. >>>>>>> But let's say the PWM controller needs a 32kHz clock: CCF would >>>>>>> automatically pick the RTC clock. >>>>>>> So is your implementation there to cover let's say 1kHz where >>>>>>> mathematically 24MHz can be divided evenly to 1kHz (and thus should >>>>>>> not result in any jitter) but RTC gives better precision in the real >>>>>>> world (even though it's off by 24Hz)? >>>>>>> >>>>>> >>>>>> I don't think so. The highest precision that RTC can provide is from a >>>>>> 32KHz rate only. However, I believe that a 1kHz frequency can also be >>>>>> achieved by using xtal 24MHz with a divider, which can provide high >>>>>> precision as well. >>>>> Thank you again for the great discussion on IRC today. >>>>> Here's my short summary so I don't forget before you'll follow up on this. >>>>> >>>>> In general there's two known cases where the RTC clock needs to be used: >>>>> a) When using the GENCLK output of the SoC to output the 32kHz RTC >>>>> clock and connect that to an SDIO WiFi chip clock input (this seems >>>>> useful in my understanding because the RTC clock provides high >>>>> precision) >>>>> b) When using the PWM controller to output a 32kHz clock signal. In >>>>> this case my understanding is that using the RTC clock as input to the >>>>> PWM controller results in the best possible signal >>>>> >>>>> The second case won't be supported with Heiner's patches [0] that use >>>>> CCF (common clock framework) in the PWM controller driver. >>>>> In this series the parent clock is calculated using: >>>>> freq = div64_u64(NSEC_PER_SEC * (u64)0xffff, period); >>>>> >>>>> A 32kHz clock means a PWM period of 30518ns. So with the above >>>> >>>> To be precise: 30517,578125ns >>>> What means that the PWM framework can't say "I want 32768Hz", >>>> but just "I want something being very close to 32768Hz". >>>> So what you need is some simple heuristic to interpret the >>>> PWM request -> "PWM requests 30518ns, but supposedly it wants >>>> 32768Hz" >>>> >>>> NSEC_PER_SEC / 30518 = 32767 (rounded down from 32767,547) >>>> clk_round_rate(channel->clk, 32767) would return 0 (I *think*), >>>> because it tries to find the next lower clock. >>>> >>>> The SoC families I'm familiar with have fclkin2 as PWM parent. >>>> That's 1 GHz in my case, what results in a frequency of 32.767,547Hz >>>> for period = 30518n. >>>> What you're saying is that newer generations don't have PWM parents >>>>> 24MHz any longer? >>> >>> No, of course not. For example, a fixed PLL (with all fclk_divX >>> settings) has rates higher than 24MHz. However, we need to consider the >>> 'heavy' background of such PWM. >>> >>> However, we have a "lightweight" clkin (special rtc32k) with a rate of >>> 32kHz that we could potentially use as an input to produce a 32kHz >>> output on the PWM lines. I don't see any reason why we should not >>> support such special cases. >>> >> >> Two more things to consider: >> 1. When wanting a 32kHz (well, 32768Hz) output with a 50% duty cycle, >> then we need hi=0 and lo=0 with a 64kHz input clock. >> See point 2 for an explanation of why 0 and not 1. >> Means we couldn't use the RTC input clock. Did you consider this? >> Or do I miss something? > > Nope, that's my fault. Using a 32kHz base clock for rates of 16kHz or > lower that can be divided evenly is a good choice. However, as you > mentioned in point 2, the problem with autoincrements should be fixed > first. > >> 2. Seems the PWM block internally increments hi and lo, except the >> constant_en bit is set on newer PWM block versions. >> For bigger cnt values the impact is negligible, but for very small >> values it's something we have to consider. >> This was one additional motivation for me to choose an input >> frequency that creates big cnt values. > > George has a patchset for this and he will send it very soon as he > mentioned in another email. Let's assume the code is already fixed and > no longer has any issues. In my opinion, if we want to generate rates of > 16kHz, 8kHz, 4kHz, etc., the best parent clock to use is a 32kHz RTC. > High-rate clocks may not divide evenly and may not generate an accurate > output rate. What do you think about it? > With the logic in the CCF conversion patch set for these rates the 24MHz xtal mux input would be used, creating exactly the requested rates. So I see no difference here. What I don't know is whether power consumption may be higher with a higher input rate, or whether quality of the mux parent clocks differs. >> >>>> >>>> >>>>> calculation the PWM driver is asking for a clock rate of >=2GHz. >>>>> We concluded that letting the common clock framework choose the best >>>>> possible parent (meaning: removing CLK_SET_RATE_NO_REPARENT here) can >>>>> be a way forward. >>>>> But this means that the PWM controller driver must try to find the >>>>> best possible parent somehow. The easiest way we came up with >>>>> (pseudo-code): >>>>> freq = NSEC_PER_SEC / period; >>>>> fin_freq = clk_round_rate(channel->clk, freq); >>>>> if (fin_freq != freq) { >>>>> freq = div64_u64(NSEC_PER_SEC * (u64)0xffff, period); >>>>> fin_freq = clk_round_rate(channel->clk, freq); >>>>> } >>>>> >>>>> The idea is: for a requested 32kHz signal the PWM period is 30518ns. >>>>> The updated logic would find that there's a matching clock input and >>>>> use that directly. If not: use the original logic as suggested by >>>>> Heiner. >>>>> >>>>> >>>>> Best regards, >>>>> Martin >>>>> >>>>> >>>>> [0] https://lore.kernel.org/linux-amlogic/9faca2e6-b7a1-4748-7eb0-48f8064e323e@xxxxxxxxx/ >>>> >>> >> >