[PATCH RFC/RFT] pwm: meson: make full use of common clock framework
Heiner Kallweit
hkallweit1 at gmail.com
Wed Apr 5 13:43:54 PDT 2023
On 03.04.2023 23:01, Martin Blumenstingl wrote:
> Hello Heiner,
>
> On Tue, Mar 28, 2023 at 10:59 PM Heiner Kallweit <hkallweit1 at gmail.com> wrote:
>>
>> Newer versions of the PWM block use a core clock with external mux,
>> divider, and gate. These components either don't exist any longer in
>> the PWM block, or they are bypassed.
>> To minimize needed changes for supporting the new version, the internal
>> divider and gate should be handled by CCF too.
> That sounds like a good way forward to me
>
>> I didn't see a good way to split the patch, therefore it's somewhat
>> bigger. What it does:
>>
>> - The internal mux is handled by CCF already. Register also internal
>> divider and gate with CCF, so that we have one representation of the
>> input clock: [mux] parent of [divider] parent of [gate]
>>
>> - Now that CCF selects an appropriate mux parent, we don't need the
>> DT-provided default parent any longer. Accordingly we can also omit
>> setting the mux parent directly in the driver.
>>
>> - Instead of manually handling the pre-div divider value, let CCF
>> set the input clock. Targeted input clock frequency is
>> 0xffff * 1/period for best precision.
>>
>> - For the "inverted pwm disabled" scenario target an input clock
>> frequency of 1GHz. This ensures that the remaining low pulses
>> have minimum length.
> Unfortunately I didn't have much time today so I didn't get to reviewing this.
>
>> I don't have hw with the old PWM block, therefore I couldn't test this
>> patch. With the not yet included extension for the new PWM block
>> (channel->clock directly coming from get_clk(external_clk)) I didn't
>> notice any problem. My system uses PWM for the CPU voltage regulator
>> and for the SDIO 32kHz clock.
>>
>> Note: The clock gate in the old PWM block is permanently disabled.
>> This seems to indicate that it's not used by the new PWM block.
>>
>> I'd appreciate testing on the different platforms using the old
>> PWM block.
> I have tested basic functionality on a X96 Air (SM1 SoC, the version
> with Gbit/s PHY) where the VDDCPU regulator is PWM based and the 32kHz
> clock for wifi is generated by the PWM controller.
> The RTL8822CS SDIO wifi card is still working (firmware download,
> basic connectivity and connecting to an AP) and the system survived a
> minute of 100% CPU usage without hanging.
>
> For reference:
> # cat /sys/kernel/debug/pwm
> platform/ffd19000.pwm, 2 PWM devices
> pwm-0 (wifi32k ): requested enabled period: 30518 ns
> duty: 15259 ns polarity: normal
> pwm-1 ((null) ): period: 0 ns duty: 0 ns polarity: normal
>
> platform/ff807000.pwm, 2 PWM devices
> pwm-0 ((null) ): period: 0 ns duty: 0 ns polarity: normal
> pwm-1 ((null) ): period: 0 ns duty: 0 ns polarity: normal
>
> platform/ff802000.pwm, 2 PWM devices
> pwm-0 ((null) ): period: 0 ns duty: 0 ns polarity: normal
> pwm-1 (regulator-vddcpu ): requested enabled period: 1500 ns
> duty: 1125 ns polarity: normal
>
> # grep \.pwm /sys/kernel/debug/clk/clk_summary
> ffd19000.pwm#mux0 1 1 0 648999985
> 0 0 50000 Y
> ffd19000.pwm#div0 1 1 0
> 648999985 0 0 50000 Y
> ffd19000.pwm#gate0 1 1 0
> 648999985 0 0 50000 Y
> ffd19000.pwm#mux1 0 0 0 24000000
> 0 0 50000 Y
> ffd19000.pwm#div1 0 0 0 24000000
> 0 0 50000 Y
> ffd19000.pwm#gate1 0 0 0 24000000
> 0 0 50000 N
> ff807000.pwm#mux1 0 0 0 24000000
> 0 0 50000 Y
> ff807000.pwm#div1 0 0 0 24000000
> 0 0 50000 Y
> ff807000.pwm#gate1 0 0 0 24000000
> 0 0 50000 N
> ff807000.pwm#mux0 0 0 0 24000000
> 0 0 50000 Y
> ff807000.pwm#div0 0 0 0 24000000
> 0 0 50000 Y
> ff807000.pwm#gate0 0 0 0 24000000
> 0 0 50000 N
> ff802000.pwm#mux1 1 1 0 24000000
> 0 0 50000 Y
> ff802000.pwm#div1 1 1 0 24000000
> 0 0 50000 Y
> ff802000.pwm#gate1 1 1 0 24000000
> 0 0 50000 Y
> ff802000.pwm#mux0 0 0 0 24000000
> 0 0 50000 Y
> ff802000.pwm#div0 0 0 0 24000000
> 0 0 50000 Y
> ff802000.pwm#gate0 0 0 0 24000000
> 0 0 50000 N
>
> hdmi_pll is the parent of ffd19000.pwm#mux0 - before it was using the
> 24MHz XTAL.
> I haven't tested what happens when I change the video mode (that board
> is currently not connected to any HDMI screen).
>
That's a good point. AFAICS drivers/gpu/drm/meson/meson_vclk.c fiddles
with the hdmi clock registers. So we may want to avoid using hdmi_pll
or vid_pll as pwm parent. Below is a quick (and hopefully not too dirty)
follow-up patch disabling the hdmi/video clock parent.
Would be great if you can test this patch on top.
> Later this week I can also try this e.g. on my Odroid-C1 (with 32-bit
> Meson8b SoC) to verify that we don't have any 32-bit compatibility
> issues.
>
>
> Best regards,
> Martin
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 2b1debda4..81900e03a 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -348,7 +348,7 @@ static const struct pwm_ops meson_pwm_ops = {
};
static const char * const pwm_meson8b_parent_names[] = {
- "xtal", "vid_pll", "fclk_div4", "fclk_div3"
+ "xtal", "fclk_div4", "fclk_div3"
};
static const struct meson_pwm_data pwm_meson8b_data = {
@@ -357,7 +357,7 @@ static const struct meson_pwm_data pwm_meson8b_data = {
};
static const char * const pwm_gxbb_parent_names[] = {
- "xtal", "hdmi_pll", "fclk_div4", "fclk_div3"
+ "xtal", "fclk_div4", "fclk_div3"
};
static const struct meson_pwm_data pwm_gxbb_data = {
@@ -415,7 +415,7 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
};
static const char * const pwm_g12a_ee_parent_names[] = {
- "xtal", "hdmi_pll", "fclk_div4", "fclk_div3"
+ "xtal", "fclk_div4", "fclk_div3"
};
static const struct meson_pwm_data pwm_g12a_ee_data = {
@@ -470,6 +470,7 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
for (i = 0; i < meson->chip.npwm; i++) {
struct meson_pwm_channel *channel = &meson->channels[i];
+ static const u32 mux_parents_wo_vid[] = {0, 2, 3};
const char *clk_parent[1];
struct clk *mux_clk, *div_clk;
@@ -490,6 +491,10 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
channel->mux.table = NULL;
channel->mux.hw.init = &init;
+ /* 3 parents indicates that video clock parent should be omitted */
+ if (init.num_parents == 3)
+ channel->mux.table = mux_parents_wo_vid;
+
mux_clk = devm_clk_register(dev, &channel->mux.hw);
if (IS_ERR(mux_clk)) {
err = PTR_ERR(mux_clk);
--
2.40.0
More information about the linux-arm-kernel
mailing list