Enabling the PWM clock at boot with Device Tree

[subspclr4]

Hello,

I'm looking to ship a Raspberry Pi HAT + EEPROM overlay that uses the PWM as part of a larger driver, but I am having some difficulties with it, most likely due to my newness to Device Tree.

I read here that @repk has patched the clock subsystem to enable the PWM clock from Device Tree. These patches appear to have been applied to the two RPi Foundation Linux 4.4 and 4.6 kernels I've experimented with, but I haven't dug too deeply to confirm this 100%. An example of making it work is provided with @repk's EPD driver along with a brief README mention that the PWM clock patch is applied to recent RPi Foundation kernels.

Unfortunately, I can't seem to get the clock to turn on with my Device Tree code unless I manually enable the PWM clock via direct register access on my Raspberry Pi 1 B+:

PWMCLK_DIV = 0x5A000000 | (5<<12);  // TODO:  switch to writel()
PWMCLK_CNTL = 0x5A000016;

After the enabling the clock with the above, calls to of_pwm_get(), pwm_config(), and pwm_enable() all function as expected and generate the correct PWM waveform.

Below is my Device Tree Overlay. Could someone point out where the mistake is that prevents the PWM clock from turning on at boot with this overlay? I'm very new to Device Tree, so it's very likely I'm doing something wrong.

I'm currently using the Raspbian Lite distribution released 2016-05-27 and a recent kernel release raspberrypi-kernel_1.20160506-1.tar.gz. As mentioned earlier, I've also tried the current git 4.6 branch as well. My kernel configuration is simply make bcmrpi_defconfig.

Thanks!

-Adam

Device Tree Overlay below. Note: I'm temporarily "borrowing" the i2s_pins node for the PWM out because I still need to fix an outstanding phandle reference issue if I overlay my own pwm_pins node within the gpio node.

/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2835", "bcrm,bcm2708";

    fragment@0 {
        target = <&i2s_pins>;
        __overlay__ {
            brcm,pins = <19>;
            brcm,function = <2>;
        };
    };

    fragment@1 {
        target = <&pwm>;
        __overlay__ {
            pinctrl-names = "default";
            pinctrl-0 = <&i2s_pins>;
            status = "okay";
        };
    };

    fragment@2 {
        target-path = "/";
        __overlay__ {
            corp_foo@0{
                compatible = "corp,foo";
                pwms = <&pwm 1 50000>;
                reset-gpios = <&gpio 13 0>;
                status = "okay";
            };
        };
    };
};

[repk]

Hi,

Are you sure you want to use pwmchip1 (ie your are using "pwms = <&pwm 1 50000>" in the overlay). I think you only have one pwm chip and should use "pwms = <&pwm 0 50000>" instead.

By the way, after a quick look, prefer 4.6.y over 4.4.y because 4.4.y does not seem to have my devicetree patches.

Remi

[subspclr4]

Thank you for the quick reply, Remi.

Yes, I do want PWM channel 1 (PWM1). It simplified some PCB layout, which is why I chose it over PWM0. PWM1 works fine as long as I turn on the PWM clock by directly setting PWMCLK_DIV and PWMCLK_CNTL like above.

I don't think the 1 in pwms = <&pwm 1 50000> references a pwmchip1 -- pwmchip1 doesn't exist. The (somewhat limited) Device Tree binding documentation says that the first cell after the &pwm phandle "typically encodes the chip-relative PWM number." I read that as PWM channel number 1 of the phandle referenced PWM chip node, in this case pwm@7e20c000.

I have evidence that it is PWM channel 1 of pwmchip0 that's referenced because if I try to export the channel with echo 1 > /sys/class/pwm/pwmchip0/export the stock PWM driver will not let me export PWM1 if my driver is running (as expected, it throws a device busy error). It will let me export PWM0 without a problem, and a mythical PWM2 throws a no such device error. So it seems to me the overlay snippet pwms = <&pwm 1 50000> is correctly allocating the PWM1 resource. Also, I don’t specifically request channel 1 anywhere else besidespwms = <&pwm 1 50000> -- my code only pulls PWM configuration information from Device Tree:

pwm = of_pwm_get(node, NULL);  // omitting error checking for brevity...
pwm_config(pwm, PWM_DUTY, PWM_PERIOD);
pwm_enable(pwm);

As suggested, I went back to the 4.6.y foundation kernel branch (specifically commit 6d2b7c3), but the issue is present in that code as well. To further try and isolate whether it is my code, I also tried using the stock dtoverlay=pwm,pin=19,func=2 in /boot/config.txt without my driver’s overlay. With that configuration, I can only get the PWM to work after calling userspace code that enables the clock, exporting PWM1 from /sys/class/pwm/pwmchip0, and writing to the {period,duty_cycle,enable} sysfs files. The userspace code I'm using to start the PWM clock is pwmclk from frasersdev/librpip.

So it really does seem like something else is not working since both my driver's overlay and the stock dtoverlay=pwm,pin=19,func=2 don't start the PWM clock.

Any other ideas for what could be preventing the clock's start at boot? I am using Raspbian Lite -- does regular Raspbian happen to turn on the PWM clock for audio?

-Adam

[pelwell]

I think you need to study the README in @repk's EPD Driver repo, in particular the part about having to run the pwmconf when using a Foundation kernel < 4.5. This utility enables the clock.

ARM-side clock control isn't currently enabled by default in Foundation kernels. You can optionally enable it using an overlay - see the vc4-kms-v3d as an example. Raspbian (even Lite) will turn on the clock for audio, but the audio jack must be selected (not the default behaviour if HDMI is connected - see the Advanced Options section of raspi-config) and you need to play something - we don't automatically start clocks that may not be needed.

[ghollingworth]

Either that or you can set it in the dt-blob.bin. an example of that would be to look in https://github.com/fiveninjas/OpenELEC.tv

[pelwell]

[ This is the bit that Gordon is referring to: https://github.com/FiveNinjas/OpenELEC.tv/blob/master/projects/Slice/bootloader/dt-blob.dts#L96 - he's out-of-office this afternoon ]

True, but that can be a bit of a support nightmare if you aren't a carefully managed end-user product like Slice. It's a shame the HAT EEPROM spec doesn't include a clock declaration section.

[subspclr4]

Thank you for the additional information. With regard to the README, I had tried the suggested foundation 4.6 kernel, but the PWM clock would not start, hence my confusion about how to do it. From userspace, I was using pwmclk instead of pwmconf because pwmconf does additional GPIO setup that isn’t relevant in my application since I’m not using PWM0.

As suggested, I tried disconnecting HDMI, but the PWM clock doesn’t immediately start, ostensibly because I haven't played audio yet. If I then go to raspi-config -> Advanced Setup -> Force 3.5mm ('headphone') jack, I get "There was an error running option A8 Audio" and on the console "amixer: Control default open error: No such file or directory." I imagine this is related to needing an overlay to enable audio? Either way, I can't rely on end users going through all those steps -- I'd really like an automatic EEPROM overlay method.

It's a shame the HAT EEPROM spec doesn't include a clock declaration section.

Okay, thank you for clarifying -- that answers my original question. How should I work around this? Is it recommended to start the clock in userspace or via the driver?

My preference would be in the driver since it already has elevated memory access permissions. If in the driver, should I continue to use direct register access with PWMCLK_DIV and PWMCLK_CNTL like above or is there a more elegant Device Tree approach I should take? I imagine Device Tree probably isn’t the right approach for right now since there’s no standard spot for me to put that configuration in my device's EEPROM overlay.

[pelwell]

Do you have dtparam=audio=on in your config.txt? Although that is now the default for the new installations, if you've upgraded then it might not be there.

I imagine Device Tree probably isn’t the right approach for right now since there’s no standard spot for me to put that configuration in my device's EEPROM overlay.

I don't understand that sentence.

I'd suggest enabling cprman (see the vc4-kms-v3d overlay) and patching the pwm node to use cprman as its clock provider. See here for how they do it upstream.

[subspclr4]

@pelwell Thanks for pointing out the vc4-kms-v3d overlay. I'm sorry I missed your earlier reference to vc4-kms-v3d, but thanks for further clarifying that I need to specifically enable cprman. My first quick pass at an overlay with cprman didn't work, but the kernel log at least says clk did attempt to start the PWM clock (but failed). I'll try some more debugging tonight and write up more specifics.

Once this is working, is the Device Tree approach with enabling cprman and patching the pwm node considered stable enough to ship embedded in hardware with an EEPROM?

Do you have dtparam=audio=on in your config.txt?

No, I do not. I'll try testing with the newest standard config.txt. Should I add Device Tree overlay code that disables audio drivers to avoid potential interference between my non-audio PWM HAT and the audio PWM driver?

I don't understand that sentence.

I had missed your earlier reference to vc4-kms-v3d (later clarified to be cprman) as the standard Device Tree method to turn the PWM clock on. The previous mention that the HAT EEPROM spec did not have a Device Tree clock capability led me to mistakenly think turning on clocks via Device Tree overlays wasn't provided -- many references (including the EPD driver) work around this with userspace code to turn on the clock. I was expressing concern that any clock related Device Tree properties I added to my driver's overlay would be non-standard.

[pelwell]

Once this is working, is the Device Tree approach with enabling cprman and patching the pwm node considered stable enough to ship embedded in hardware with an EEPROM?

The main reason upstreaming drivers and device tree bindings takes such a long time is that changes are resisted and backwards compatibility is paramount. Although I can't give you any guarantees, I'm pretty certain that an upstream change won't invalidate your overlay. If it does get broken, either because of an upstream change or as is more likely a Pi-specific change, there is a work around - putting "dtoverlay=" before any other DT directives will cause the overlay in the HAT EEPROM to be ignored, to be followed by another dtoverlay to load the replacement. Of course, being able to rewrite the EEPROM is better.

No, I do not. I'll try testing with the newest standard config.txt. Should I add Device Tree overlay code that disables audio drivers to avoid potential interference between my non-audio PWM HAT and the audio PWM driver?

Setting the audio parameter on will enable the ALSA audio driver, which when used will communicate with the VPU-side audio driver that turns on the clock. I only mentioned it because it might explain why any testing you'd done hadn't resulted in the clock being enabled.

If you really want to prevent the audio driver from interfering then your overlay could overwrite the "compatible" property of the audio node.

[subspclr4]

If you really want to prevent the audio driver from interfering then your overlay could overwrite the "compatible" property of the audio node.

Thanks, I'll try that.

Regarding the other examples provided, I’m still having issues starting the PWM clock under 4.6.y. Here’s the relevant section of the kernel log:

[   10.032735] clk: couldn't get clock 0 for /soc/pwm@7e20c000
[   10.032828] bcm2835-pwm: probe of 2020c000.pwm failed with error -17

And here’s my current overlay:

/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2835", "bcrm,bcm2708";

    fragment@0 {
        target = <&i2s_pins>;
        __overlay__ {
            brcm,pins = <19>;
            brcm,function = <2>;
        };
    };

    fragment@1 {
        target = <&pwm>;
        __overlay__ {
            #clock-cells = <1>;
            clocks = <&cprman 30>;  /* 30 should be BCM2835_CLOCK_PWM */
            assigned-clocks = <&cprman 30>;
            assigned-clock-rates = <10000000>;
            pinctrl-names = "default";
            pinctrl-0 = <&i2s_pins>;
            status = "okay";
        };
    };

    fragment@2 {
        target = <&cprman>;
        __overlay__  {
            status = "okay";
        };
    };

    fragment@3 {
        target-path = "/";
        __overlay__ {
            corp_foo@0{
                compatible = "corp,foo";
                pwms = <&pwm 1 50000>;
                reset-gpios = <&gpio 13 0>;
                status = "okay";
            };
        };
    };
};

Do you know what step I'm missing?

Thanks again, I really appreciate the help.

[pelwell]

The failure is caused by the fact that cprman attempts to register a clock called "pwm", but the base DTB has already created a fixed clock with the same name so cprman fails. I've modified the overlay to rename the fixed clock to "fake_pwm" and it seems happy. I've also created a proper "pwm_pins" group - let me know if this works. You may also want to allocate pin 18 to PWM - it's up to you.

This overlay doesn't solve the problem of PWM0 already being mapped on either 40 & 41 or 40 & 45. It would be nice to find an elegant solution to that; making it dependent on the audio node being enabled would be logical, whoever ends up making that assignment - firmware or pinctrl.

Anyway, here's the overlay:

/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2835", "bcrm,bcm2708";

    fragment@0 {
        target = <&gpio>;
        __overlay__ {
            pwm_pins: pwm_pins {
                brcm,pins = <19>;
                brcm,function = <2>;
            };
        };
    };

    fragment@1 {
        target = <&clk_pwm>;
        __overlay__ {
            // Rename the fixed "pwm" clock to avoid a clash
            clock-output-names = "fake_pwm";
        };
    };

    fragment@2 {
        target = <&pwm>;
        __overlay__ {
            #clock-cells = <1>;
            clocks = <&cprman 30>;  /* 30 should be BCM2835_CLOCK_PWM */
            assigned-clocks = <&cprman 30>;
            assigned-clock-rates = <10000000>;
            pinctrl-names = "default";
            pinctrl-0 = <&pwm_pins>;
            status = "okay";
        };
    };

    fragment@3 {
        target = <&cprman>;
        __overlay__  {
            status = "okay";
        };
    };

    fragment@4 {
        target-path = "/";
        __overlay__ {
            corp_foo@0{
                compatible = "corp,foo";
                pwms = <&pwm 1 50000>;
                reset-gpios = <&gpio 13 0>;
                status = "okay";
            };
        };
    };
};

[repk]

Hi,

Oh I have forgotten that foundation repo uses bcm2708 device tree that is why I didn't understand why setting cprman status to "okay" was needed at all.

So your overlay looks almost correct, I just tried it real quick and get also an error enabling PWM clock (EEXIST).

I think you have to disable clk_pwm also in your device tree.

Unfortunately I cannot test it right now because I don't have any access to a RPI.

Remi

[subspclr4]

@pelwell's overlay works fine for me on the 4.6.y Foundation kernel (commit 6d2b7c3):

Thanks so much @pelwell! I hope the other EEXIST issue @repk is reporting can be resolved -- what kernel is it with?

Regarding overwriting the audio compatible property to completely disable audio, do you have any suggestions on what I should I overwrite it with? Just a compatible; boolean or something more descriptive like compatible = "disabled,bcm2835-audio";?

Also, one other Device Tree question (unrelated to PWM). It looks like the GPIO driver can't be easily disabled in newer kernels. I have some FIQ code that triggers off a GPIO interrupt line, which is already allocated by the GPIO driver. My tentative (untested) workaround is to overwrite interrupts = <0x2 0x11 0x2 0x12>; with a plain boolean overlay interrupts;. Do you think this is the right approach?

[pelwell]

I think an empty compatible string is cleanest - it should do the job. You can test it by adding dtparam=audio=on in your config.txt then confirming that the bcm2835 sound driver module isn't loaded with your overlay applied.

What you are proposing for the GPIO interrupts sounds ugly, and I'm not even sure it can work since AFAIK only one FIQ interrupt source is allowed and USB needs it to get reasonable performance.

Giving the GPIO driver fewer interrupts than it expects (2) will stop it initialising. It would be better to give it some other interrupts that will never trigger. The list of available interrupt sources is here - SMI and SLIMBUS might be OK, or perhaps the MULTICORESYNC interrupts. The existing claimed interrupts are GPIO0 and GPIO1.

[subspclr4]

Yes, overlaying compatible = ""; prevents dtparam=audio=on from loading the audio drivers at boot. Thank you!

I am able to boot with a GPIO node interrupts; overlay. The kernel log doesn’t show any errors and cat /proc/interrupts doesn’t show any registered GPIO interrupts. That said, I agree, it’s far from ideal. I will look into the other non-firing sources you suggest and further examine the GPIO interrupt code. It would be great if there was a clean way to disable GPIO interrupts since the driver is now required (in the past it wasn't required).

Regarding the FIQ, I have been using it successfully for a couple years now by disabling the USB driver’s FIQ with dwc_otg.fiq_enable=0 dwc_otg.fiq_fsm_enable=0 in cmdline.txt. There were different cmdline.txt parameters used in previous years and the USB driver used to require patching to remove calls to local_fiq_disable() and friends while running in IRQ mode, but those have been fixed in recent USB drivers.

It does slow the USB, but it gives microsecond-level real-time determinacy to my code with Linux running in the background. Some more details with scope traces are available in raspberrypi/firmware#497.

I haven't looked into it in a while, but I think it is possible to have more than one FIQ source on a multi-core RPi 2+ machine, but it's messy to share the FIQ between different driver code bases. The furthest I got back in November was sending a GPIO FIQ to all four cores and putting three of the cores in a busy-waiting loop (based off the mpidr CPU ID) while one core executed my code. That was part of my debug in raspberrypi/firmware#497.

[pelwell]

I think the original question has been answered. Can we close this issue?

[subspclr4]

Yes. Thank you again!