Messages in this thread | | | Date | Wed, 24 May 2023 03:52:06 +0300 | Subject | Re: [PATCH v14 6/6] clk: meson: a1: add Amlogic A1 Peripherals clock controller driver | From | George Stark <> |
| |
On 5/22/23 23:36, 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@sberdevices.ru> 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? > 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. > Hello Heiner
As I mentioned earlier I have some changes to take into account lo and hi regs incrementing.
But it's more convenient to base my patch on top on one of yours (https://lore.kernel.org/linux-amlogic/23fe625e-dc23-4db8-3dce-83167cd3b206@gmail.com/)
Is that ok if I resend your patch along with mine in series?
Best regards George
>>> >>>> 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@gmail.com/ >
| |