diff mbox series

[PATCHv3] arm64: dts: meson: Enable active coling using gpio-fan on Odroid N2/N2+

Message ID 20221022084737.1028-1-linux.amoon@gmail.com (mailing list archive)
State New, archived
Headers show
Series [PATCHv3] arm64: dts: meson: Enable active coling using gpio-fan on Odroid N2/N2+ | expand

Commit Message

Anand Moon Oct. 22, 2022, 8:47 a.m. UTC
Odroid N2/N2+ support active cooling via gpio-fan controller.
Add fan controls and tip point for cpu and ddr thermal sensor
on this boards. Drop bias-disable from set pwm_ao_d_10 the pin
as to allow transition from say pull-up to pull-down for on/off
of the fan.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: changes tip name cpu-active --> ddr-active
v3: drop bias-disable for pwm_ao_d_10 pine.
---
 .../boot/dts/amlogic/meson-g12-common.dtsi    |  1 -
 .../dts/amlogic/meson-g12b-odroid-n2.dtsi     | 42 +++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)


base-commit: 4da34b7d175dc99b8befebd69e96546c960d526c

Comments

Martin Blumenstingl Oct. 22, 2022, 10:17 a.m. UTC | #1
Hi Anand,

On Sat, Oct 22, 2022 at 10:48 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Odroid N2/N2+ support active cooling via gpio-fan controller.
> Add fan controls and tip point for cpu and ddr thermal sensor
> on this boards. Drop bias-disable from set pwm_ao_d_10 the pin
either use "on these boards" or "on this board"

[...]
> @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
>                                                 mux {
>                                                         groups = "pwm_ao_d_10";
>                                                         function = "pwm_ao_d";
> -                                                       bias-disable;
&pwm_ao_d_10_pins is not referenced anywhere so it seems that this
change has no impact on controlling the fan on Odroid-N2(+).
How did you test this change?


Best regards,
Martin
Anand Moon Oct. 22, 2022, 11:27 a.m. UTC | #2
Hi Martin,

Thanks for your review comments..

On Sat, 22 Oct 2022 at 15:47, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Sat, Oct 22, 2022 at 10:48 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Odroid N2/N2+ support active cooling via gpio-fan controller.
> > Add fan controls and tip point for cpu and ddr thermal sensor
> > on this boards. Drop bias-disable from set pwm_ao_d_10 the pin
> either use "on these boards" or "on this board"
>
> [...]
> > @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
> >                                                 mux {
> >                                                         groups = "pwm_ao_d_10";
> >                                                         function = "pwm_ao_d";
> > -                                                       bias-disable;
> &pwm_ao_d_10_pins is not referenced anywhere so it seems that this
> change has no impact on controlling the fan on Odroid-N2(+).
> How did you test this change?
>
Ok I felt these changes affect the behavior of the pinctrl

  * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a
 *  transition from say pull-up to pull-down implies that you disable
 *  pull-up in the process, this setting disables all biasing.

I mapped this is linked in pinctrl driver, pwm_ao_d_10_pins GPIOAO_10 see below

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/meson/pinctrl-meson-g12a.c?h=v6.1-rc1#n825

For testing, I tried to libgpio command to verify shown below, with
this patch applied.
$ gpioinfo
....
gpiochip1 - 15 lines:
        line   0:      unnamed       unused   input  active-high
        line   1:      unnamed       unused   input  active-high
        line   2:      unnamed     "enable"  output  active-high [used]
        line   3:      unnamed       unused   input  active-high
        line   4:      unnamed       unused  output  active-high
        line   5:      unnamed       unused   input  active-high
        line   6:      unnamed       unused   input  active-high
        line   7:      unnamed       unused   input  active-high
        line   8:      unnamed "regulator-tflash_vdd" output active-high [used]
        line   9:      unnamed      "TF_IO"  output  active-high [used]
        line  10:      unnamed   "gpio-fan"  output  active-high [used]
        line  11:      unnamed    "n2:blue"  output  active-high [used]
        line  12:      unnamed       unused   input  active-high
        line  13:      unnamed       unused   input  active-high
        line  14:      unnamed       unused   input  active-high

$ cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinmux-pins
Pinmux settings per pin
Format: pin (name): mux_owner gpio_owner hog?
pin 0 (GPIOAO_0): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
group uart_ao_a_tx
pin 1 (GPIOAO_1): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
group uart_ao_a_rx
pin 2 (GPIOAO_2): (MUX UNCLAIMED) aobus-banks:1950
pin 3 (GPIOAO_3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 4 (GPIOAO_4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 5 (GPIOAO_5): ff808000.ir (GPIO UNCLAIMED) function
remote_ao_input group remote_ao_input
pin 6 (GPIOAO_6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 7 (GPIOAO_7): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 8 (GPIOAO_8): (MUX UNCLAIMED) aobus-banks:1956
pin 9 (GPIOAO_9): (MUX UNCLAIMED) aobus-banks:1957
pin 10 (GPIOAO_10): (MUX UNCLAIMED) aobus-banks:1958
pin 11 (GPIOAO_11): (MUX UNCLAIMED) aobus-banks:1959
pin 12 (GPIOE_0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 13 (GPIOE_1): ff802000.pwm (GPIO UNCLAIMED) function pwm_ao_d
group pwm_ao_d_e
pin 14 (GPIOE_2): ffd1b000.pwm (GPIO UNCLAIMED) function pwm_a_e group pwm_a_e

$ sudo gpiomon  gpiochip1 10
gpiomon: error waiting for events: Device or resource busy

$ sudo gpioget  gpiochip1 10
gpioget: error reading GPIO values: Device or resource busy

If I am wrong I have previously sent it with a typo correction below.

[1] https://lore.kernel.org/all/20221021050906.1158-1-linux.amoon@gmail.com/

Thanks
-Anand
>
> Best regards,
> Martin
Martin Blumenstingl Oct. 22, 2022, 11:52 a.m. UTC | #3
Hi Anand,

On Sat, Oct 22, 2022 at 1:27 PM Anand Moon <linux.amoon@gmail.com> wrote:
[...]
> > > @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
> > >                                                 mux {
> > >                                                         groups = "pwm_ao_d_10";
> > >                                                         function = "pwm_ao_d";
> > > -                                                       bias-disable;
> > &pwm_ao_d_10_pins is not referenced anywhere so it seems that this
> > change has no impact on controlling the fan on Odroid-N2(+).
> > How did you test this change?
> >
> Ok I felt these changes affect the behavior of the pinctrl
>
>   * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a
>  *  transition from say pull-up to pull-down implies that you disable
>  *  pull-up in the process, this setting disables all biasing.
>
> I mapped this is linked in pinctrl driver, pwm_ao_d_10_pins GPIOAO_10 see below
Yes, I understand this part.
My concern is: &pwm_ao_d_10_pins settings only become active when this
node is actively referenced. You can even see it in your output
below...

[...]
> pin 10 (GPIOAO_10): (MUX UNCLAIMED) aobus-banks:1958
This shows that it's used as a GPIO. If the &pwm_ao_d_10_pins setting
was used then it would show "function pwm_ao_d group pwm_ao_d_10"
(similar to what GPIOE_1 shows in your output)

If you want to know if a pull-up/down is enabled you can look at the output of:
$ cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinconf-pins
(I'm sure this can also be retrieved from some userspace tools, but I
don't know how)


Best regards,
Martin
Anand Moon Oct. 25, 2022, 6:06 p.m. UTC | #4
Hi Martin,

On Sat, 22 Oct 2022 at 17:22, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Sat, Oct 22, 2022 at 1:27 PM Anand Moon <linux.amoon@gmail.com> wrote:
> [...]
> > > > @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
> > > >                                                 mux {
> > > >                                                         groups = "pwm_ao_d_10";
> > > >                                                         function = "pwm_ao_d";
> > > > -                                                       bias-disable;
> > > &pwm_ao_d_10_pins is not referenced anywhere so it seems that this
> > > change has no impact on controlling the fan on Odroid-N2(+).
> > > How did you test this change?
> > >
> > Ok I felt these changes affect the behavior of the pinctrl
> >
> >   * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a
> >  *  transition from say pull-up to pull-down implies that you disable
> >  *  pull-up in the process, this setting disables all biasing.
> >
> > I mapped this is linked in pinctrl driver, pwm_ao_d_10_pins GPIOAO_10 see below
> Yes, I understand this part.
> My concern is: &pwm_ao_d_10_pins settings only become active when this
> node is actively referenced. You can even see it in your output
> below...
>
> [...]
> > pin 10 (GPIOAO_10): (MUX UNCLAIMED) aobus-banks:1958
> This shows that it's used as a GPIO. If the &pwm_ao_d_10_pins setting
> was used then it would show "function pwm_ao_d group pwm_ao_d_10"
> (similar to what GPIOE_1 shows in your output)
>
> If you want to know if a pull-up/down is enabled you can look at the output of:
> $ cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinconf-pins
> (I'm sure this can also be retrieved from some userspace tools, but I
> don't know how)
>

I now switch using pwm-fan with the local changes I am able to link
pwm_ao_d_10_pins
but now the issue is fan keeps on spinning on boot-up and stays on.

I can manually turn on off by using
$ sudo gpioset gpiochip1 10=1   // fan on
$ sudo gpioset gpiochip1 10=0   // fan off

It is not controlled by the thermal tip as expected.
I feel some configuration is missing in pwm-meson driver.
Any input for me?

$ sudo cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinmux-pins
[sudo] password for alarm:
Pinmux settings per pin
Format: pin (name): mux_owner gpio_owner hog?
pin 0 (GPIOAO_0): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
group uart_ao_a_tx
pin 1 (GPIOAO_1): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
group uart_ao_a_rx
pin 2 (GPIOAO_2): (MUX UNCLAIMED) aobus-banks:1950
pin 3 (GPIOAO_3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 4 (GPIOAO_4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 5 (GPIOAO_5): ff808000.ir (GPIO UNCLAIMED) function
remote_ao_input group remote_ao_input
pin 6 (GPIOAO_6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 7 (GPIOAO_7): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 8 (GPIOAO_8): (MUX UNCLAIMED) aobus-banks:1956
pin 9 (GPIOAO_9): (MUX UNCLAIMED) aobus-banks:1957
pin 10 (GPIOAO_10): ff807000.pwm (GPIO UNCLAIMED) function pwm_ao_d
group pwm_ao_d_10
pin 11 (GPIOAO_11): (MUX UNCLAIMED) aobus-banks:1959
pin 12 (GPIOE_0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 13 (GPIOE_1): ff802000.pwm (GPIO UNCLAIMED) function pwm_ao_d
group pwm_ao_d_e
pin 14 (GPIOE_2): ffd1b000.pwm (GPIO UNCLAIMED) function pwm_a_e group pwm_a_e

$ sudo cat /sys/kernel/debug/pwm
platform/ffd1b000.pwm, 2 PWM devices
 pwm-0   (regulator-vddcpu-a  ): requested enabled period: 1250 ns
duty: 838 ns polarity: normal
 pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal

platform/ff807000.pwm, 2 PWM devices
 pwm-0   (pwm-fan             ): requested period: 1250 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-b  ): requested enabled period: 1250 ns
duty: 1213 ns polarity: normal

I could observe a change in duty when we have stress testing the CPU.

Thanks

-Anand
Neil Armstrong Oct. 26, 2022, 8:02 a.m. UTC | #5
Hi,

On 25/10/2022 20:06, Anand Moon wrote:
> Hi Martin,
> 
> On Sat, 22 Oct 2022 at 17:22, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> Hi Anand,
>>
>> On Sat, Oct 22, 2022 at 1:27 PM Anand Moon <linux.amoon@gmail.com> wrote:
>> [...]
>>>>> @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
>>>>>                                                  mux {
>>>>>                                                          groups = "pwm_ao_d_10";
>>>>>                                                          function = "pwm_ao_d";
>>>>> -                                                       bias-disable;
>>>> &pwm_ao_d_10_pins is not referenced anywhere so it seems that this
>>>> change has no impact on controlling the fan on Odroid-N2(+).
>>>> How did you test this change?
>>>>
>>> Ok I felt these changes affect the behavior of the pinctrl
>>>
>>>    * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a
>>>   *  transition from say pull-up to pull-down implies that you disable
>>>   *  pull-up in the process, this setting disables all biasing.
>>>
>>> I mapped this is linked in pinctrl driver, pwm_ao_d_10_pins GPIOAO_10 see below
>> Yes, I understand this part.
>> My concern is: &pwm_ao_d_10_pins settings only become active when this
>> node is actively referenced. You can even see it in your output
>> below...
>>
>> [...]
>>> pin 10 (GPIOAO_10): (MUX UNCLAIMED) aobus-banks:1958
>> This shows that it's used as a GPIO. If the &pwm_ao_d_10_pins setting
>> was used then it would show "function pwm_ao_d group pwm_ao_d_10"
>> (similar to what GPIOE_1 shows in your output)
>>
>> If you want to know if a pull-up/down is enabled you can look at the output of:
>> $ cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinconf-pins
>> (I'm sure this can also be retrieved from some userspace tools, but I
>> don't know how)
>>
> 
> I now switch using pwm-fan with the local changes I am able to link
> pwm_ao_d_10_pins
> but now the issue is fan keeps on spinning on boot-up and stays on.
> 
> I can manually turn on off by using
> $ sudo gpioset gpiochip1 10=1   // fan on
> $ sudo gpioset gpiochip1 10=0   // fan off

By doing that actually override the PWM function of the pin and set it as a GPIO.

> 
> It is not controlled by the thermal tip as expected.
> I feel some configuration is missing in pwm-meson driver.
> Any input for me?
> 
> $ sudo cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinmux-pins
> [sudo] password for alarm:
> Pinmux settings per pin
> Format: pin (name): mux_owner gpio_owner hog?
> pin 0 (GPIOAO_0): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
> group uart_ao_a_tx
> pin 1 (GPIOAO_1): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
> group uart_ao_a_rx
> pin 2 (GPIOAO_2): (MUX UNCLAIMED) aobus-banks:1950
> pin 3 (GPIOAO_3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 4 (GPIOAO_4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 5 (GPIOAO_5): ff808000.ir (GPIO UNCLAIMED) function
> remote_ao_input group remote_ao_input
> pin 6 (GPIOAO_6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 7 (GPIOAO_7): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 8 (GPIOAO_8): (MUX UNCLAIMED) aobus-banks:1956
> pin 9 (GPIOAO_9): (MUX UNCLAIMED) aobus-banks:1957
> pin 10 (GPIOAO_10): ff807000.pwm (GPIO UNCLAIMED) function pwm_ao_d
> group pwm_ao_d_10
> pin 11 (GPIOAO_11): (MUX UNCLAIMED) aobus-banks:1959
> pin 12 (GPIOE_0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 13 (GPIOE_1): ff802000.pwm (GPIO UNCLAIMED) function pwm_ao_d
> group pwm_ao_d_e
> pin 14 (GPIOE_2): ffd1b000.pwm (GPIO UNCLAIMED) function pwm_a_e group pwm_a_e
> 
> $ sudo cat /sys/kernel/debug/pwm
> platform/ffd1b000.pwm, 2 PWM devices
>   pwm-0   (regulator-vddcpu-a  ): requested enabled period: 1250 ns
> duty: 838 ns polarity: normal
>   pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
> 
> platform/ff807000.pwm, 2 PWM devices
>   pwm-0   (pwm-fan             ): requested period: 1250 ns duty: 0 ns
> polarity: normal
>   pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal

This should be on the pwm-1, hence the "pwm_AO_cd" name, "c" and "d" and the
names of the outputs.

So you need to use 1 as first PWM phandle argument instead of 0.

> 
> platform/ff802000.pwm, 2 PWM devices
>   pwm-0   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
>   pwm-1   (regulator-vddcpu-b  ): requested enabled period: 1250 ns
> duty: 1213 ns polarity: normal
> 
> I could observe a change in duty when we have stress testing the CPU.

Can you share the complete change you did here ?

> 
> Thanks
> 
> -Anand

Neil
Anand Moon Oct. 26, 2022, 4:02 p.m. UTC | #6
Hi Neil,

On Wed, 26 Oct 2022 at 13:32, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Hi,
>
> On 25/10/2022 20:06, Anand Moon wrote:
> > Hi Martin,
> >
> > On Sat, 22 Oct 2022 at 17:22, Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> >>
> >> Hi Anand,
> >>
> >> On Sat, Oct 22, 2022 at 1:27 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >> [...]
> >>>>> @@ -1982,7 +1982,6 @@ pwm_ao_d_10_pins: pwm-ao-d-10 {
> >>>>>                                                  mux {
> >>>>>                                                          groups = "pwm_ao_d_10";
> >>>>>                                                          function = "pwm_ao_d";
> >>>>> -                                                       bias-disable;
> >>>> &pwm_ao_d_10_pins is not referenced anywhere so it seems that this
> >>>> change has no impact on controlling the fan on Odroid-N2(+).
> >>>> How did you test this change?
> >>>>
> >>> Ok I felt these changes affect the behavior of the pinctrl
> >>>
> >>>    * @PIN_CONFIG_BIAS_DISABLE: disable any pin bias on the pin, a
> >>>   *  transition from say pull-up to pull-down implies that you disable
> >>>   *  pull-up in the process, this setting disables all biasing.
> >>>
> >>> I mapped this is linked in pinctrl driver, pwm_ao_d_10_pins GPIOAO_10 see below
> >> Yes, I understand this part.
> >> My concern is: &pwm_ao_d_10_pins settings only become active when this
> >> node is actively referenced. You can even see it in your output
> >> below...
> >>
> >> [...]
> >>> pin 10 (GPIOAO_10): (MUX UNCLAIMED) aobus-banks:1958
> >> This shows that it's used as a GPIO. If the &pwm_ao_d_10_pins setting
> >> was used then it would show "function pwm_ao_d group pwm_ao_d_10"
> >> (similar to what GPIOE_1 shows in your output)
> >>
> >> If you want to know if a pull-up/down is enabled you can look at the output of:
> >> $ cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinconf-pins
> >> (I'm sure this can also be retrieved from some userspace tools, but I
> >> don't know how)
> >>
> >
> > I now switch using pwm-fan with the local changes I am able to link
> > pwm_ao_d_10_pins
> > but now the issue is fan keeps on spinning on boot-up and stays on.
> >
> > I can manually turn on off by using
> > $ sudo gpioset gpiochip1 10=1   // fan on
> > $ sudo gpioset gpiochip1 10=0   // fan off
>
> By doing that actually override the PWM function of the pin and set it as a GPIO.

Yes, I just want to test if this pin is working.
>
> >
> > It is not controlled by the thermal tip as expected.
> > I feel some configuration is missing in pwm-meson driver.
> > Any input for me?
> >
> > $ sudo cat /sys/kernel/debug/pinctrl/ff800000.sys-ctrl\:pinctrl@14-pinctrl-meson/pinmux-pins
> > [sudo] password for alarm:
> > Pinmux settings per pin
> > Format: pin (name): mux_owner gpio_owner hog?
> > pin 0 (GPIOAO_0): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
> > group uart_ao_a_tx
> > pin 1 (GPIOAO_1): ff803000.serial (GPIO UNCLAIMED) function uart_ao_a
> > group uart_ao_a_rx
> > pin 2 (GPIOAO_2): (MUX UNCLAIMED) aobus-banks:1950
> > pin 3 (GPIOAO_3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> > pin 4 (GPIOAO_4): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> > pin 5 (GPIOAO_5): ff808000.ir (GPIO UNCLAIMED) function
> > remote_ao_input group remote_ao_input
> > pin 6 (GPIOAO_6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> > pin 7 (GPIOAO_7): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> > pin 8 (GPIOAO_8): (MUX UNCLAIMED) aobus-banks:1956
> > pin 9 (GPIOAO_9): (MUX UNCLAIMED) aobus-banks:1957
> > pin 10 (GPIOAO_10): ff807000.pwm (GPIO UNCLAIMED) function pwm_ao_d
> > group pwm_ao_d_10
> > pin 11 (GPIOAO_11): (MUX UNCLAIMED) aobus-banks:1959
> > pin 12 (GPIOE_0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> > pin 13 (GPIOE_1): ff802000.pwm (GPIO UNCLAIMED) function pwm_ao_d
> > group pwm_ao_d_e
> > pin 14 (GPIOE_2): ffd1b000.pwm (GPIO UNCLAIMED) function pwm_a_e group pwm_a_e
> >
> > $ sudo cat /sys/kernel/debug/pwm
> > platform/ffd1b000.pwm, 2 PWM devices
> >   pwm-0   (regulator-vddcpu-a  ): requested enabled period: 1250 ns
> > duty: 838 ns polarity: normal
> >   pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
> >
> > platform/ff807000.pwm, 2 PWM devices
> >   pwm-0   (pwm-fan             ): requested period: 1250 ns duty: 0 ns
> > polarity: normal
> >   pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
>
> This should be on the pwm-1, hence the "pwm_AO_cd" name, "c" and "d" and the
> names of the outputs.
>
> So you need to use 1 as first PWM phandle argument instead of 0.
>
> >
> > platform/ff802000.pwm, 2 PWM devices
> >   pwm-0   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
> >   pwm-1   (regulator-vddcpu-b  ): requested enabled period: 1250 ns
> > duty: 1213 ns polarity: normal
> >
> > I could observe a change in duty when we have stress testing the CPU.
>
> Can you share the complete change you did here ?
>

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

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>;
+               fan-supply = <&vcc_5v>;
+               #cooling-cells = <2>;
+               cooling-levels = <0 100 170 230>;
+       };
+
        leds {
                compatible = "gpio-leds";

@@ -410,6 +418,40 @@ &cpu103 {
        clock-latency = <50000>;
 };

+&cpu_thermal {
+       trips {
+               cpu_active: cpu-active {
+                       temperature = <55000>; /* millicelsius */
+                       hysteresis = <2000>; /* millicelsius */
+                       type = "active";
+               };
+       };
+
+       cooling-maps {
+               map {
+                       trip = <&cpu_active>;
+                       cooling-device = <&fan THERMAL_NO_LIMIT
THERMAL_NO_LIMIT>;
+               };
+       };
+};
+
+&ddr_thermal {
+       trips {
+               ddr_active: ddr-active {
+                       temperature = <55000>; /* millicelsius */
+                       hysteresis = <2000>; /* millicelsius */
+                       type = "active";
+               };
+       };
+
+       cooling-maps {
+               map {
+                       trip = <&ddr_active>;
+                       cooling-device = <&fan THERMAL_NO_LIMIT
THERMAL_NO_LIMIT>;
+               };
+       };
+};
+
 &ext_mdio {
        external_phy: ethernet-phy@0 {
                /* Realtek RTL8211F (0x001cc916) */
@@ -547,6 +589,14 @@ &pwm_ab {
        status = "okay";
 };

+&pwm_AO_ab {
+       pinctrl-0 = <&pwm_ao_d_10_pins>;
+       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 Armstrong Oct. 27, 2022, 12:49 p.m. UTC | #7
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
Neil Armstrong Oct. 27, 2022, 1:16 p.m. UTC | #8
On 27/10/2022 14:49, neil.armstrong@linaro.org wrote:
> 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.

I was meaning regulator-vddcpu-b.

> 
>>
>> 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
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Anand Moon Oct. 27, 2022, 7:18 p.m. UTC | #9
Hi Neil,

On Thu, 27 Oct 2022 at 19:02, Neil Armstrong
<narmstrong.kernel@gmail.com> wrote:
>
> On 27/10/2022 14:49, neil.armstrong@linaro.org wrote:
> > 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.
>
> I was meaning regulator-vddcpu-b.
>
> >
> >>
> >> 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.

Thanks for your review comments, ok now I get better understand of PWM
mux configuration
so now I switch to use &pwm_cd, But still, the issue of the fan keeps
spinning on start. persist.

$ sudo cat /sys/kernel/debug/pwm
platform/ffd1b000.pwm, 2 PWM devices
 pwm-0   (regulator-vddcpu-a  ): requested enabled period: 1250 ns
duty: 963 ns polarity: normal
 pwm-1   ((null)              ): period: 0 ns duty: 0 ns polarity: normal

platform/ffd1a000.pwm, 2 PWM devices
 pwm-0   ((null)              ): period: 0 ns duty: 0 ns polarity: normal
 pwm-1   (pwm-fan             ): requested period: 1250 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-b  ): requested enabled period: 1250 ns
duty: 88 ns polarity: normal

> >
> >> +               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.
> >
ok.
> >> +       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
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 45947c1031c4..10a09fe362fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1982,7 +1982,6 @@  pwm_ao_d_10_pins: pwm-ao-d-10 {
 						mux {
 							groups = "pwm_ao_d_10";
 							function = "pwm_ao_d";
-							bias-disable;
 						};
 					};
 
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..667d2b774924 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: gpio-fan {
+		compatible = "gpio-fan";
+		gpios = <&gpio_ao GPIOAO_10 GPIO_ACTIVE_HIGH>;
+		/* Using Dummy Speed */
+		gpio-fan,speed-map = <0 0>, <1 1>;
+		#cooling-cells = <2>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -410,6 +418,40 @@  &cpu103 {
 	clock-latency = <50000>;
 };
 
+&cpu_thermal {
+	trips {
+		cpu_active: cpu-active {
+			temperature = <60000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		map {
+			trip = <&cpu_active>;
+			cooling-device = <&fan THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+		};
+	};
+};
+
+&ddr_thermal {
+	trips {
+		ddr_active: ddr-active {
+			temperature = <60000>; /* millicelsius */
+			hysteresis = <2000>; /* millicelsius */
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		map {
+			trip = <&ddr_active>;
+			cooling-device = <&fan THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+		};
+	};
+};
+
 &ext_mdio {
 	external_phy: ethernet-phy@0 {
 		/* Realtek RTL8211F (0x001cc916) */