Skip to content

Conversation

@FRASTM
Copy link
Contributor

@FRASTM FRASTM commented Jan 25, 2022

with this PR, the complementary PWM output is enabled when the pin is choosen in the DTS of the target

Using a newly defined PWM_COMPLEMENTARY flag in the target board DTS as follows:

		red_pwm_led: red_pwm_led {
			pwms = <&pwm1 2 4 (PWM_POLARITY_NORMAL | PWM_COMPLEMENTARY)>;
		};
  • At this time, only used by the stm32 mcus
  • Does not impact the PWM input capture.

Fixes #40657

Signed-off-by: Francois Ramu [email protected]

@FRASTM
Copy link
Contributor Author

FRASTM commented Jan 25, 2022

Tested on the nucleo_l4r5zi target where the PB 14 pin which is for the red Led is a output of a PWM signal from

-  Timer15 channel 1, when
-  		red_pwm_led: red_pwm_led {
			pwms = <&pwm15 1 4 PWM_POLARITY_NORMAL>;
		};
  • or Timer 1 channel 2 N (complementary output), when
- 		red_pwm_led: red_pwm_led {
			pwms = <&pwm1 2 4 (PWM_POLARITY_NORMAL | PWM_COMPLEMENTARY)>;
		};

@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: PWM Pulse Width Modulation area: Samples Samples platform: STM32 ST Micro STM32 labels Jan 25, 2022
@FRASTM FRASTM force-pushed the pwm_complementary branch from 79215c2 to 7b95749 Compare January 25, 2022 11:11
@erwango erwango requested review from ABOSTM, erwango and gmarull January 25, 2022 12:52
@erwango erwango added this to the v3.1.0 milestone Jan 25, 2022
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinctrl information is not meant to be used by drivers directly. If you want PWM to enable complementary output, you should add an additional flag at the pwm node or, propose a new PWM flag. Then user is responsible to set pinctrl appropriately.

@erwango
Copy link
Member

erwango commented Jan 25, 2022

pinctrl information is not meant to be used by drivers directly. If you want PWM to enable complementary output, you should add an additional flag at the pwm node or, propose a new PWM flag. Then user is responsible to set pinctrl appropriately.

IIUC this means that we should be able to identify a PWM channel to a pin.
For single channel PWM configuration this is fine, but what about multiple channel configuration, as for instance:

	pwm4: pwm {
		status = "okay";
		pinctrl-0 = <&tim4_ch1_pd12
			     &tim4_ch2n_pd13
			     &tim4_ch3n_pd14
			     &tim4_ch4_pd15>;
		pinctrl-names = "default";
	};

Does it mean that we can rely on the order of the pins established by the user in pins array ?
(Current work is anyway not sufficient to achieve this anyway as we miss channel to pin matching, but I'd like to understand what are the possibilities)

@gmarull
Copy link
Member

gmarull commented Jan 25, 2022

One option would be to do something like this:

red_pwm_led: red_pwm_led {
    pwms = <&pwm12 1 4 (PWM_POLARITY_NORMAL | PWM_COMPLEMENTARY)>;
}; 

@gmarull
Copy link
Member

gmarull commented Jan 25, 2022

Or, another option (not tested):

#define STM32_PWM_CH(ch) (ch)
#define STM32_PWM_CHN(ch) ((ch) << 3U)

red_pwm_led: red_pwm_led {
    /* use regular output */
    pwms = <&pwm12 STM32_PWM_CH(1) 4 PWM_POLARITY_NORMAL>;
}; 

red_pwm_led: red_pwm_led {
    /* use N output */
    pwms = <&pwm12 STM32_PWM_CHN(1) 4 PWM_POLARITY_NORMAL>;
}; 

STM32_PWM_CHN would add an offset that indicates complementary output is used for channel 1 (up to 4 or 6 channels are available).

@FRASTM FRASTM force-pushed the pwm_complementary branch from 7b95749 to 034e901 Compare January 25, 2022 14:58
@FRASTM
Copy link
Contributor Author

FRASTM commented Jan 26, 2022

This option is clear and readable to me because it direcly goes to the pwm driver

red_pwm_led: red_pwm_led {
    pwms = <&pwm1 2 4 (PWM_POLARITY_NORMAL | PWM_COMPLEMENTARY)>;
}; 

@FRASTM
Copy link
Contributor Author

FRASTM commented Jan 26, 2022

new version without impact on the pinctrl but only the PWM driver.

@FRASTM FRASTM force-pushed the pwm_complementary branch from 97f1998 to 281104b Compare January 26, 2022 09:46
@FRASTM FRASTM marked this pull request as ready for review January 26, 2022 12:27
@FRASTM
Copy link
Contributor Author

FRASTM commented Jan 26, 2022

splitting in 2 PR:

  1. one for the PWM API dts: pwm : flag to enable a complementary output of a pwm channel pin #42174
  2. this one for using that on stm32 mcus with a running samples/basic/blinky_pwm

@FRASTM FRASTM force-pushed the pwm_complementary branch 2 times, most recently from ccb31f5 to 4ec7c99 Compare January 26, 2022 12:56
@carlescufi carlescufi requested a review from gmarull March 21, 2022 14:26
gmarull
gmarull previously approved these changes Mar 21, 2022
Comment on lines 295 to 301
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's define a STM32_MAX_N_CHANNEL constant and use it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/** Maximum number of complemented timer channels. */
#define STM32_MAX_N_CHANNEL 4

@FRASTM
Copy link
Contributor Author

FRASTM commented Mar 21, 2022

rebase on e8f93a9
include the #42174
define STM32_MAX_N_CHANNEL
define a current_channel value

@FRASTM FRASTM force-pushed the pwm_complementary branch 2 times, most recently from 65cd39d to 3d29d57 Compare March 22, 2022 08:05
@FRASTM
Copy link
Contributor Author

FRASTM commented Mar 22, 2022

rebase on 095001c

it also includes #42174

@codecov-commenter
Copy link

Codecov Report

Merging #42119 (095001c) into main (095001c) will not change coverage.
The diff coverage is n/a.

❗ Current head 095001c differs from pull request most recent head 3d29d57. Consider uploading reports for the commit 3d29d57 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #42119   +/-   ##
=======================================
  Coverage   49.52%   49.52%           
=======================================
  Files         634      634           
  Lines       79487    79487           
  Branches    18588    18588           
=======================================
  Hits        39364    39364           
  Misses      33621    33621           
  Partials     6502     6502           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 095001c...3d29d57. Read the comment docs.

@FRASTM FRASTM force-pushed the pwm_complementary branch from 3d29d57 to 8685a07 Compare March 22, 2022 11:49
@FRASTM
Copy link
Contributor Author

FRASTM commented Mar 22, 2022

using stm32Cube LL definition to detect the Max nb of timer channels, to define the table of complementary channels
Raise an error if PWM_STM32_COMPLEMENTARY flag is set for a non-existing channel.

@FRASTM FRASTM force-pushed the pwm_complementary branch 2 times, most recently from a40f924 to 005a2e3 Compare March 22, 2022 12:08
Add 8 bits to the pwm_flags_t so that upper byte is reserved
for non-standard but soc specific dts flags and keeping the low
Byte for standard flags.
Some of SoC like STM32 mcus can then define their own flags
in their dt-bindings/pwm.

Signed-off-by: Francois Ramu <[email protected]>
@FRASTM FRASTM force-pushed the pwm_complementary branch from 005a2e3 to 9004b1b Compare March 22, 2022 14:31
Depending on the stm32 soc and the timer instance, several channels
can enable the complementary output for the PWM signal
This flag completes the PWM_POLARITY for a PWM channel in the
upper byte of the pwm_flags_t.

Signed-off-by: Francois Ramu <[email protected]>
@FRASTM FRASTM force-pushed the pwm_complementary branch from 9004b1b to 1c74cc7 Compare March 22, 2022 14:36
ABOSTM
ABOSTM previously approved these changes Mar 22, 2022
Copy link
Contributor

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

FRASTM added 3 commits March 23, 2022 13:43
Depending on the stm32 mcus and the timer instance, several channels
can enable the complementary output for the PWM signal.
Example of a DTS <&pwm1 2 4 (PWM_POLARITY_NORMAL | PWM_COMPLEMENTARY)>;
Note that the timer channel must support the complementary output
on that channel : usually channels 1-3 + channel 4 on stm32g4x.

Signed-off-by: Francois Ramu <[email protected]>
If the dts defines the PWM complementary output, then the OCN
must be init in place of the OC state and polarity.
This is an exclusive setting for this pin.
The channel in LL_TIM_OC_SetPolarity can be the complementary one.

Signed-off-by: Francois Ramu <[email protected]>
Adding an overlay to configure the nucleo_l4r5zi for testing the pwm
blinky application on red led (PB14 on nucleo board)
Each has a specific pwm output from different timers/channels

Signed-off-by: Francois Ramu <[email protected]>
@henrikbrixandersen
Copy link
Member

Can #42174 be closed in favour of this PR?

@carlescufi carlescufi merged commit 8043fb9 into zephyrproject-rtos:main Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: PWM Pulse Width Modulation area: Samples Samples platform: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot enable secondary pwm out channels on stm32f3

7 participants