Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <18ae0876-2d37-cde9-183f-894dfcdf4be9@linaro.org>
Date:   Thu, 27 Oct 2022 14:49:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
From:   neil.armstrong@linaro.org
Reply-To: neil.armstrong@linaro.org
Subject: Re: [PATCHv3] arm64: dts: meson: Enable active coling using gpio-fan
 on Odroid N2/N2+
Content-Language: en-US
To:     Anand Moon <linux.amoon@gmail.com>
Cc:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Kevin Hilman <khilman@baylibre.com>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Dan Johansen <strit@manjaro.org>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20221022084737.1028-1-linux.amoon@gmail.com>
 <CAFBinCBu_-0m9JeAr5kd-v7Z5LTi7w0WmmUYybW_kL4KJXXpOQ@mail.gmail.com>
 <CANAwSgRzdD0FWg+z6hTFs7KvpsD64bChX-k0dPXJfACXZH2zbQ@mail.gmail.com>
 <CAFBinCCNJiL-ZKRYesQAwys6bBMpYHJbUDK-Zi_VhGDVSvF7uQ@mail.gmail.com>
 <CANAwSgTLTCA9=WvWXcR-40baauN3kAk2qx9k4FQOPCa1+_O0Yw@mail.gmail.com>
 <5d2ffeca-b41c-20b4-454b-703c39bbb29e@linaro.org>
 <CANAwSgR1CN=Gho6NGMoM1bH92KyeGqoAphmT6NqtnL=+3Zg_jQ@mail.gmail.com>
Organization: Linaro Developer Services
In-Reply-To: <CANAwSgR1CN=Gho6NGMoM1bH92KyeGqoAphmT6NqtnL=+3Zg_jQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 26/10/2022 18:02, Anand Moon wrote:
> Hi Neil,

<snip>

> 
> When I try to use pwm_AO_cd,,
> Either one of the PWM binds will fail to get the following error.
> 
>   &pwm_AO_cd {
> -       pinctrl-0 = <&pwm_ao_d_e_pins>;
> +       pinctrl-0 = <&pwm_ao_d_e_pins>, <&pwm_ao_d_10_pins>;
>          pinctrl-names = "default";
>          clocks = <&xtal>;
>          clock-names = "clkin1";
> 
> [    3.941700] pwm-regulator regulator-vddcpu-b: error -EBUSY: Failed to get PWM
> [    3.943198] pwm-regulator: probe of regulator-vddcpu-b failed with error -16
> 
> [    3.956356] pwm-fan pwm-fan: error -EBUSY: Could not get PWM
> [    3.956396] pwm-fan: probe of pwm-fan failed with error -16

Yeah because PWM "D" is already used by the "pwm_AO_ab" controller, so you can't use it for the FAN.

> 
> Below are my changes with  pwm_AO_ab
> ---------------------------------------------------------------------------------------------
> alarm@odroid-n2:~/linux-amlogic-5.y-devel$ git diff
> arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
> index fd3fa82e4c33..d038ba1e2453 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
> @@ -39,6 +39,14 @@ emmc_pwrseq: emmc-pwrseq {
>                  reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
>          };
> 
> +       fan: pwm-fan {
> +               compatible = "pwm-fan";
> +               pwms = <&pwm_AO_ab 1 1250 0>;

Here you use the "B" PWM signal, not the D.

> +               fan-supply = <&vcc_5v>;
> +               #cooling-cells = <2>;
> +               cooling-levels = <0 100 170 230>;
> +       };
> +
>          leds {

<snip>

> 
> +&pwm_AO_ab {
> +       pinctrl-0 = <&pwm_ao_d_10_pins>;

The "pwm_AO_ab" controller only controls the PWM "A" & "B signals, not the "D" !

This basically enables the PWM "D" pin function to GPIOAO_10, it doesn't assign it to the "pwm_AO_ab" controller.

So by enabling this pinctrl, it will duplicate the pwm_ao_d_e_pins signal to pwm_ao_d_10_pins, this is why the FAN spins non-stop.

> +       pinctrl-names = "default";
> +       clocks = <&xtal>;
> +       clock-names = "clkin1";
> +       status = "okay";
> +};
> +
>   &pwm_AO_cd {
>          pinctrl-0 = <&pwm_ao_d_e_pins>;
>          pinctrl-names = "default";
> -------------------------------------------------------------------------------------------
>>>
>>> Thanks
>>>
>>> -Anand
>>
>> Neil

Neil