* Inverted PWM output on iMX6
@ 2020-03-05 13:22 Paul Barker
2020-03-05 13:36 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Paul Barker @ 2020-03-05 13:22 UTC (permalink / raw)
To: Thierry Reding, Uwe Kleine-König, Shawn Guo, NXP Linux Team
Cc: linux-pwm, linux-arm-kernel, devicetree
Hi folks,
I recently ran into an issue using the pwm-fan driver with an inverted
PWM output on iMX6.
The fan is defined in the device tree as follows:
fan0: pwm-fan {
compatible = "pwm-fan";
pwms = <&pwm2 0 25000 PWM_POLARITY_INVERTED>;
...
}
In pwm_imx27_probe() the support for a third `flags` argument in a pwm
reference is enabled:
imx->chip.of_xlate = of_pwm_xlate_with_flags;
imx->chip.of_pwm_n_cells = 3;
However, the flag is ignored and the output is not inverted.
By adding some prints I saw that when of_pwm_xlate_with_flags() is
called, args->args_count is 2 instead of 3.
Looking at the definition of the pwm device itself in imx6qdl.dtsi I
can see that the number of cells in a pwm reference is set to 2 not 3:
pwm2: pwm@2084000 {
#pwm-cells = <2>;
...
};
That seems to be preventing a third argument from being passed.
I can change `#pwm-cells` to <3> and then everything works for my
device but I'm not sure that is the correct solution for everyone. That
would require all pwm references on iMX6 devices to use 3 cells. The
code in of_pwm_xlate_with_flags() seems to be built to handle either 2
or 3 argument cells but I can't see any way to allow this choice in the
device tree.
If the solution is to set `#pwm-cells` to <3> I'm happy to send a patch
which does this and updates all pwm references in device trees which
include `imx6dql.dtsi`. Before I do that I'd like to know that it's the
correct approach though.
For context I've confirmed this is the case in Linux 5.4 and that the
relevant files haven't changed between that release and 5.6.0-rc4.
Thanks,
--
Paul Barker
Konsulko Group
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Inverted PWM output on iMX6
2020-03-05 13:22 Inverted PWM output on iMX6 Paul Barker
@ 2020-03-05 13:36 ` Uwe Kleine-König
2020-03-06 7:43 ` Michal Vokáč
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2020-03-05 13:36 UTC (permalink / raw)
To: Paul Barker
Cc: linux-pwm, devicetree, Thierry Reding, NXP Linux Team, Shawn Guo,
linux-arm-kernel
Hello,
On Thu, Mar 05, 2020 at 01:22:32PM +0000, Paul Barker wrote:
> I recently ran into an issue using the pwm-fan driver with an inverted
> PWM output on iMX6.
>
> The fan is defined in the device tree as follows:
>
> fan0: pwm-fan {
> compatible = "pwm-fan";
> pwms = <&pwm2 0 25000 PWM_POLARITY_INVERTED>;
> ...
> }
>
> In pwm_imx27_probe() the support for a third `flags` argument in a pwm
> reference is enabled:
>
> imx->chip.of_xlate = of_pwm_xlate_with_flags;
> imx->chip.of_pwm_n_cells = 3;
>
> However, the flag is ignored and the output is not inverted.
>
> By adding some prints I saw that when of_pwm_xlate_with_flags() is
> called, args->args_count is 2 instead of 3.
>
> Looking at the definition of the pwm device itself in imx6qdl.dtsi I
> can see that the number of cells in a pwm reference is set to 2 not 3:
>
> pwm2: pwm@2084000 {
> #pwm-cells = <2>;
> ...
> };
>
> That seems to be preventing a third argument from being passed.
>
> I can change `#pwm-cells` to <3> and then everything works for my
> device but I'm not sure that is the correct solution for everyone. That
> would require all pwm references on iMX6 devices to use 3 cells. The
> code in of_pwm_xlate_with_flags() seems to be built to handle either 2
> or 3 argument cells but I can't see any way to allow this choice in the
> device tree.
>
> If the solution is to set `#pwm-cells` to <3> I'm happy to send a patch
> which does this and updates all pwm references in device trees which
> include `imx6dql.dtsi`. Before I do that I'd like to know that it's the
> correct approach though.
>
> For context I've confirmed this is the case in Linux 5.4 and that the
> relevant files haven't changed between that release and 5.6.0-rc4.
I think changing that is fine. However you'd have to care that all
in-tree users that rely on #pwm-cells = <2> are fixed accordingly.
I'd do: add #pwm-cells = <3> in the cpu.dtsi and then adapt all
machine.dts to add #pwm-cells = <2> until there are no more changes to
the generated files compared to the current state.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Inverted PWM output on iMX6
2020-03-05 13:36 ` Uwe Kleine-König
@ 2020-03-06 7:43 ` Michal Vokáč
2020-03-06 20:17 ` Paul Barker
0 siblings, 1 reply; 4+ messages in thread
From: Michal Vokáč @ 2020-03-06 7:43 UTC (permalink / raw)
To: Uwe Kleine-König, Paul Barker
Cc: linux-pwm, devicetree, Thierry Reding, NXP Linux Team, Shawn Guo,
linux-arm-kernel
On 05. 03. 20 14:36, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Mar 05, 2020 at 01:22:32PM +0000, Paul Barker wrote:
>> I recently ran into an issue using the pwm-fan driver with an inverted
>> PWM output on iMX6.
>>
>> The fan is defined in the device tree as follows:
>>
>> fan0: pwm-fan {
>> compatible = "pwm-fan";
>> pwms = <&pwm2 0 25000 PWM_POLARITY_INVERTED>;
>> ...
>> }
>>
>> In pwm_imx27_probe() the support for a third `flags` argument in a pwm
>> reference is enabled:
>>
>> imx->chip.of_xlate = of_pwm_xlate_with_flags;
>> imx->chip.of_pwm_n_cells = 3;
>>
>> However, the flag is ignored and the output is not inverted.
>>
>> By adding some prints I saw that when of_pwm_xlate_with_flags() is
>> called, args->args_count is 2 instead of 3.
>>
>> Looking at the definition of the pwm device itself in imx6qdl.dtsi I
>> can see that the number of cells in a pwm reference is set to 2 not 3:
>>
>> pwm2: pwm@2084000 {
>> #pwm-cells = <2>;
>> ...
>> };
>>
>> That seems to be preventing a third argument from being passed.
>>
>> I can change `#pwm-cells` to <3> and then everything works for my
>> device but I'm not sure that is the correct solution for everyone. That
>> would require all pwm references on iMX6 devices to use 3 cells. The
>> code in of_pwm_xlate_with_flags() seems to be built to handle either 2
>> or 3 argument cells but I can't see any way to allow this choice in the
>> device tree.
>>
>> If the solution is to set `#pwm-cells` to <3> I'm happy to send a patch
>> which does this and updates all pwm references in device trees which
>> include `imx6dql.dtsi`. Before I do that I'd like to know that it's the
>> correct approach though.
>>
>> For context I've confirmed this is the case in Linux 5.4 and that the
>> relevant files haven't changed between that release and 5.6.0-rc4.
>
> I think changing that is fine. However you'd have to care that all
> in-tree users that rely on #pwm-cells = <2> are fixed accordingly.
>
> I'd do: add #pwm-cells = <3> in the cpu.dtsi and then adapt all
> machine.dts to add #pwm-cells = <2> until there are no more changes to
> the generated files compared to the current state.
I solved that in the past on our board the same way as other imx6 boards
did. Just override the #pwm-cells property in your board specific devicetree
and you are fine:
linux-src$ git grep pwm-cells -- arch/arm/boot/dts/imx6*-*
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6q-display5.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6q-mccmon6.dts: #pwm-cells = <3>;
arch/arm/boot/dts/imx6qdl-tx6.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6qdl-tx6.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6ul-tx6ul.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Inverted PWM output on iMX6
2020-03-06 7:43 ` Michal Vokáč
@ 2020-03-06 20:17 ` Paul Barker
0 siblings, 0 replies; 4+ messages in thread
From: Paul Barker @ 2020-03-06 20:17 UTC (permalink / raw)
To: Michal Vokáč
Cc: linux-pwm, devicetree, Thierry Reding, NXP Linux Team,
Uwe Kleine-König, Shawn Guo, linux-arm-kernel
On Fri, 6 Mar 2020 08:43:45 +0100
Michal Vokáč <michal.vokac@ysoft.com> wrote:
> On 05. 03. 20 14:36, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Thu, Mar 05, 2020 at 01:22:32PM +0000, Paul Barker wrote:
> >> I recently ran into an issue using the pwm-fan driver with an inverted
> >> PWM output on iMX6.
> >>
> >> The fan is defined in the device tree as follows:
> >>
> >> fan0: pwm-fan {
> >> compatible = "pwm-fan";
> >> pwms = <&pwm2 0 25000 PWM_POLARITY_INVERTED>;
> >> ...
> >> }
> >>
> >> In pwm_imx27_probe() the support for a third `flags` argument in a pwm
> >> reference is enabled:
> >>
> >> imx->chip.of_xlate = of_pwm_xlate_with_flags;
> >> imx->chip.of_pwm_n_cells = 3;
> >>
> >> However, the flag is ignored and the output is not inverted.
> >>
> >> By adding some prints I saw that when of_pwm_xlate_with_flags() is
> >> called, args->args_count is 2 instead of 3.
> >>
> >> Looking at the definition of the pwm device itself in imx6qdl.dtsi I
> >> can see that the number of cells in a pwm reference is set to 2 not 3:
> >>
> >> pwm2: pwm@2084000 {
> >> #pwm-cells = <2>;
> >> ...
> >> };
> >>
> >> That seems to be preventing a third argument from being passed.
> >>
> >> I can change `#pwm-cells` to <3> and then everything works for my
> >> device but I'm not sure that is the correct solution for everyone. That
> >> would require all pwm references on iMX6 devices to use 3 cells. The
> >> code in of_pwm_xlate_with_flags() seems to be built to handle either 2
> >> or 3 argument cells but I can't see any way to allow this choice in the
> >> device tree.
> >>
> >> If the solution is to set `#pwm-cells` to <3> I'm happy to send a patch
> >> which does this and updates all pwm references in device trees which
> >> include `imx6dql.dtsi`. Before I do that I'd like to know that it's the
> >> correct approach though.
> >>
> >> For context I've confirmed this is the case in Linux 5.4 and that the
> >> relevant files haven't changed between that release and 5.6.0-rc4.
> >
> > I think changing that is fine. However you'd have to care that all
> > in-tree users that rely on #pwm-cells = <2> are fixed accordingly.
> >
> > I'd do: add #pwm-cells = <3> in the cpu.dtsi and then adapt all
> > machine.dts to add #pwm-cells = <2> until there are no more changes to
> > the generated files compared to the current state.
>
> I solved that in the past on our board the same way as other imx6 boards
> did. Just override the #pwm-cells property in your board specific devicetree
> and you are fine:
>
> linux-src$ git grep pwm-cells -- arch/arm/boot/dts/imx6*-*
> arch/arm/boot/dts/imx6dl-yapp4-common.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6q-display5.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6q-mccmon6.dts: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6qdl-tx6.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6qdl-tx6.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6ul-tx6ul.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
> arch/arm/boot/dts/imx6ull-colibri.dtsi: #pwm-cells = <3>;
Michal,
That approach would be good enough for my limited use case but it doesn't
prevent others from hitting the same confusion in the future. I'd rather
fix the base imx6 dtsi files to have #pwm-cells = <3> if the driver supports
the third cell containing flags.
I'll send a patch once I've done a bit more investigation.
Thanks,
--
Paul Barker
Konsulko Group
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-06 20:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-05 13:22 Inverted PWM output on iMX6 Paul Barker
2020-03-05 13:36 ` Uwe Kleine-König
2020-03-06 7:43 ` Michal Vokáč
2020-03-06 20:17 ` Paul Barker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).