| Message ID | 1570534887-26181-1-git-send-email-fabrice.gasnier@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pwm: stm32: add comment to better describe breakinput feature | expand |
On Tue, Oct 08, 2019 at 01:41:27PM +0200, Fabrice Gasnier wrote: > Add a comment to better describe the purpose of breakinput feature that > can be found on some STM32 timer instances. Briefly comment on the > characteristics of this input for PWM, and pinmuxing as suggested in [1]. > > [1] https://lkml.org/lkml/2019/10/1/207 > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > drivers/pwm/pwm-stm32.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > index 359b085..6406ebb 100644 > --- a/drivers/pwm/pwm-stm32.c > +++ b/drivers/pwm/pwm-stm32.c > @@ -522,8 +522,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, > sizeof(struct stm32_breakinput)); > > /* > + * Some timer instances can have BRK input pins (e.g. basically a fault > + * pin from the output power stage). The break feature allows a safe > + * shut-down of the PWM outputs to a predefined state. Further details > + * are available in application note AN4277, "Using STM32 device PWM > + * shut-down features..." Without having read the application note I don't understand the purpose. Not sure if this should be a show stopper though. > * Because "st,breakinput" parameter is optional do not make probe > - * failed if it doesn't exist. > + * failed if it doesn't exist. The pinctrl handle must hold the BRK > + * pin(s) when using "st,breakinput" property. Is this a comment that has a better place in the binding doc? Best regards Uwe
On 10/8/19 4:45 PM, Uwe Kleine-König wrote: > On Tue, Oct 08, 2019 at 01:41:27PM +0200, Fabrice Gasnier wrote: >> Add a comment to better describe the purpose of breakinput feature that >> can be found on some STM32 timer instances. Briefly comment on the >> characteristics of this input for PWM, and pinmuxing as suggested in [1]. >> >> [1] https://lkml.org/lkml/2019/10/1/207 >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> --- >> drivers/pwm/pwm-stm32.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c >> index 359b085..6406ebb 100644 >> --- a/drivers/pwm/pwm-stm32.c >> +++ b/drivers/pwm/pwm-stm32.c >> @@ -522,8 +522,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, >> sizeof(struct stm32_breakinput)); >> >> /* >> + * Some timer instances can have BRK input pins (e.g. basically a fault >> + * pin from the output power stage). The break feature allows a safe >> + * shut-down of the PWM outputs to a predefined state. Further details >> + * are available in application note AN4277, "Using STM32 device PWM >> + * shut-down features..." > > Without having read the application note I don't understand the purpose. > Not sure if this should be a show stopper though. Hi Uwe, I can rephrase this. Do you think the bellow comment would be more relevant and easy to understand ? /* * The break feature allows a safe shut-down of the PWM outputs. * It's based on the BRK event signal defined in the dt-bindings * by <index level filter> values. * Because "st,breakinput" parameter is optional do not make probe * failed if it doesn't exist. */ > >> * Because "st,breakinput" parameter is optional do not make probe >> - * failed if it doesn't exist. >> + * failed if it doesn't exist. The pinctrl handle must hold the BRK >> + * pin(s) when using "st,breakinput" property. > > Is this a comment that has a better place in the binding doc? You're right, this is unneeded here. I'll remove this. Please advise, BR, Fabrice > > Best regards > Uwe > >
On Wed, Oct 09, 2019 at 11:51:05AM +0200, Fabrice Gasnier wrote: > On 10/8/19 4:45 PM, Uwe Kleine-König wrote: > > On Tue, Oct 08, 2019 at 01:41:27PM +0200, Fabrice Gasnier wrote: > >> Add a comment to better describe the purpose of breakinput feature that > >> can be found on some STM32 timer instances. Briefly comment on the > >> characteristics of this input for PWM, and pinmuxing as suggested in [1]. > >> > >> [1] https://lkml.org/lkml/2019/10/1/207 > >> > >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > >> --- > >> drivers/pwm/pwm-stm32.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > >> index 359b085..6406ebb 100644 > >> --- a/drivers/pwm/pwm-stm32.c > >> +++ b/drivers/pwm/pwm-stm32.c > >> @@ -522,8 +522,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, > >> sizeof(struct stm32_breakinput)); > >> > >> /* > >> + * Some timer instances can have BRK input pins (e.g. basically a fault > >> + * pin from the output power stage). The break feature allows a safe > >> + * shut-down of the PWM outputs to a predefined state. Further details > >> + * are available in application note AN4277, "Using STM32 device PWM > >> + * shut-down features..." > > > > Without having read the application note I don't understand the purpose. > > Not sure if this should be a show stopper though. > > Hi Uwe, > > I can rephrase this. Do you think the bellow comment would be more > relevant and easy to understand ? > > /* > * The break feature allows a safe shut-down of the PWM outputs. > * It's based on the BRK event signal defined in the dt-bindings > * by <index level filter> values. > * Because "st,breakinput" parameter is optional do not make probe > * failed if it doesn't exist. > */ I still fail to understand. This is an input that determines the actual value of the output pin? What makes a shutdown of outputs safe? What is a shutdown anyhow? Apart from that: s/do not make probe failed/don't fail to probe/. Best regards Uwe
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 359b085..6406ebb 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -522,8 +522,14 @@ static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv, sizeof(struct stm32_breakinput)); /* + * Some timer instances can have BRK input pins (e.g. basically a fault + * pin from the output power stage). The break feature allows a safe + * shut-down of the PWM outputs to a predefined state. Further details + * are available in application note AN4277, "Using STM32 device PWM + * shut-down features..." * Because "st,breakinput" parameter is optional do not make probe - * failed if it doesn't exist. + * failed if it doesn't exist. The pinctrl handle must hold the BRK + * pin(s) when using "st,breakinput" property. */ if (nb <= 0) return 0;
Add a comment to better describe the purpose of breakinput feature that can be found on some STM32 timer instances. Briefly comment on the characteristics of this input for PWM, and pinmuxing as suggested in [1]. [1] https://lkml.org/lkml/2019/10/1/207 Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/pwm/pwm-stm32.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)