On Thu, May 23, 2024 at 11:26:07AM +0900, きくちゃんさん wrote: > Hello Aleksandr, > > I had coincidentally developed a PWM driver for the device. > Based on my experience, I find that dynamically changing the coupled > DIV_M value is quite complex. > The current approach has limitations, especially with resolution > changes, which can be unpredictable for users. For example: > > 1. Enabling channel A automatically selects DIV_M. > 2. Enabling coupled channel B with a specific period may result in > poor resolution for channel B, as the DIV_M value depends on the > period of channel A. > 3. If channel B is enabled first, channel A may not be enabled if > its period doesn't fit the DIV_M range selected by channel B. > > Additionally, using APB as a clock source for the channels would > further complicate the process. > > To simplify this, I suggest (maybe for the future) specifying these > values directly in the Device Tree like this: > ``` > allwinner,pwm-coupled-channel-clock-sources="hosc", "apb", "hosc"; > allwinner,pwm-coupled-channel-clock-prescales=<0>, <3>, <8>; > ``` > This would delegate the complexity to the DT, making the resolution > predictable for users. > As a bonus, it introduces a way to select clock sources for each > coupled channels. > > For the meantime, I think it is enough to use fixed "hosc" and <0> for > regular use. > > Looking forward to your thoughts. > > Best regards, > kikuchan. I have a somewhat opposite opinion. I've developed a driver too and posted it on the u-boot mailing list that is deterministic and handles both channels: https://lore.kernel.org/all/20240518-pwm_d1-v1-0-311fc5fe2248@xxxxxxxxxx/ It does this by remembering the settings for channels and disabling then setting both channels at once whenever there's an update. I think this is a decent enough solution to the problem and just works automatically without people having to micromanage the controller. John.
