PWM: drop legacy wrapper for changing polarity

Message ID 20181015082152.5900-1-u.kleine-koenig@pengutronix.de
State Accepted
Headers show
Series
  • PWM: drop legacy wrapper for changing polarity
Related show

Commit Message

Uwe Kleine-König Oct. 15, 2018, 8:21 a.m.
The API to configure a PWM using pwm_enable(), pwm_disable(),
pwm_config() and pwm_set_polarity() is superseeded by atomically setting
the parameters using pwm_apply_state(). To get forward with deprecating
the former set of functions use the opportunity that there is no current
user of pwm_set_polarity() and remove it.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 include/linux/pwm.h | 42 ------------------------------------------
 1 file changed, 42 deletions(-)

Comments

Uwe Kleine-König Nov. 4, 2018, 9:19 p.m. | #1
Hello Thierry,

On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> The API to configure a PWM using pwm_enable(), pwm_disable(),
> pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> the parameters using pwm_apply_state(). To get forward with deprecating
> the former set of functions use the opportunity that there is no current
> user of pwm_set_polarity() and remove it.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I think this patch is undisputed and wonder if it fell through the
cracks given that a patch sent later made it into your pwm/for-4.20-rc1
pull request.

Best regards
Uwe
Uwe Kleine-König Nov. 20, 2018, 4:57 p.m. | #2
On Sun, Nov 04, 2018 at 10:19:45PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> > The API to configure a PWM using pwm_enable(), pwm_disable(),
> > pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> > the parameters using pwm_apply_state(). To get forward with deprecating
> > the former set of functions use the opportunity that there is no current
> > user of pwm_set_polarity() and remove it.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> I think this patch is undisputed and wonder if it fell through the
> cracks given that a patch sent later made it into your pwm/for-4.20-rc1
> pull request.

Now that you pushed out my lpc patch to your for-next branch (thanks!),
I wonder what you plan to do with this patch.

Best regards
Uwe
Uwe Kleine-König Nov. 30, 2018, 8:59 a.m. | #3
On Tue, Nov 20, 2018 at 05:57:58PM +0100, Uwe Kleine-König wrote:
> On Sun, Nov 04, 2018 at 10:19:45PM +0100, Uwe Kleine-König wrote:
> > Hello Thierry,
> > 
> > On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> > > The API to configure a PWM using pwm_enable(), pwm_disable(),
> > > pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> > > the parameters using pwm_apply_state(). To get forward with deprecating
> > > the former set of functions use the opportunity that there is no current
> > > user of pwm_set_polarity() and remove it.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > I think this patch is undisputed and wonder if it fell through the
> > cracks given that a patch sent later made it into your pwm/for-4.20-rc1
> > pull request.
> 
> Now that you pushed out my lpc patch to your for-next branch (thanks!),
> I wonder what you plan to do with this patch.

Ping! There are also a few other patches that didn't catch your
attention yet:

 - series for pwm-imx starting with
   "pwm: imx: remove if block where the condition is always wrong"
 - "pwm: don't use memcmp to compare struct state variables"
 - "pwm: drop per-chip dbg_show callback"

Best regards
Uwe
Uwe Kleine-König Dec. 6, 2018, 9:20 a.m. | #4
Hello Thierry,

On Fri, Nov 30, 2018 at 09:59:42AM +0100, Uwe Kleine-König wrote:
> On Tue, Nov 20, 2018 at 05:57:58PM +0100, Uwe Kleine-König wrote:
> > On Sun, Nov 04, 2018 at 10:19:45PM +0100, Uwe Kleine-König wrote:
> > > Hello Thierry,
> > > 
> > > On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> > > > The API to configure a PWM using pwm_enable(), pwm_disable(),
> > > > pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> > > > the parameters using pwm_apply_state(). To get forward with deprecating
> > > > the former set of functions use the opportunity that there is no current
> > > > user of pwm_set_polarity() and remove it.
> > > > 
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > 
> > > I think this patch is undisputed and wonder if it fell through the
> > > cracks given that a patch sent later made it into your pwm/for-4.20-rc1
> > > pull request.
> > 
> > Now that you pushed out my lpc patch to your for-next branch (thanks!),
> > I wonder what you plan to do with this patch.
> 
> Ping! There are also a few other patches that didn't catch your
> attention yet:
> 
>  - series for pwm-imx starting with
>    "pwm: imx: remove if block where the condition is always wrong"
>  - "pwm: don't use memcmp to compare struct state variables"
>  - "pwm: drop per-chip dbg_show callback"

In a different thread it was suggested to resend the patches instead of
sending a ping. Should I do that? There are also

 - pwm: rearrange structures to group members by purpose
 - [RFC] Documentation: pwm: rework documentation for the framework

which didn't get any feedback from you yet.

There is a patchwork instance for the pwm list[1]. Are you using that?
It seems to be orphaned for some time but was already used this year.
Would you accept some help here? E.g. 

 - https://patchwork.ozlabs.org/patch/1001130/
   -> superseeded as there is a v3
 - https://patchwork.ozlabs.org/patch/993022/
   -> rejected is probably right
 - https://patchwork.ozlabs.org/patch/860540/
   -> applied as bb084c0f61d659f0e6d371b096e0e57998f191d6
 - https://patchwork.ozlabs.org/project/linux-pwm/list/?series=72798
   -> rejected

Best regards
Uwe

[1] https://patchwork.ozlabs.org/project/linux-pwm/list/
Uwe Kleine-König Dec. 11, 2018, 9:26 p.m. | #5
On Thu, Dec 06, 2018 at 10:20:57AM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Fri, Nov 30, 2018 at 09:59:42AM +0100, Uwe Kleine-König wrote:
> > On Tue, Nov 20, 2018 at 05:57:58PM +0100, Uwe Kleine-König wrote:
> > > On Sun, Nov 04, 2018 at 10:19:45PM +0100, Uwe Kleine-König wrote:
> > > > Hello Thierry,
> > > > 
> > > > On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> > > > > The API to configure a PWM using pwm_enable(), pwm_disable(),
> > > > > pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> > > > > the parameters using pwm_apply_state(). To get forward with deprecating
> > > > > the former set of functions use the opportunity that there is no current
> > > > > user of pwm_set_polarity() and remove it.
> > > > > 
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > 
> > > > I think this patch is undisputed and wonder if it fell through the
> > > > cracks given that a patch sent later made it into your pwm/for-4.20-rc1
> > > > pull request.
> > > 
> > > Now that you pushed out my lpc patch to your for-next branch (thanks!),
> > > I wonder what you plan to do with this patch.
> > 
> > Ping! There are also a few other patches that didn't catch your
> > attention yet:
> > 
> >  - series for pwm-imx starting with
> >    "pwm: imx: remove if block where the condition is always wrong"
> >  - "pwm: don't use memcmp to compare struct state variables"
> >  - "pwm: drop per-chip dbg_show callback"
> 
> In a different thread it was suggested to resend the patches instead of
> sending a ping. Should I do that? There are also
> 
>  - pwm: rearrange structures to group members by purpose
>  - [RFC] Documentation: pwm: rework documentation for the framework
> 
> which didn't get any feedback from you yet.
> 
> There is a patchwork instance for the pwm list[1]. Are you using that?
> It seems to be orphaned for some time but was already used this year.
> Would you accept some help here? E.g. 
> 
>  - https://patchwork.ozlabs.org/patch/1001130/
>    -> superseeded as there is a v3
>  - https://patchwork.ozlabs.org/patch/993022/
>    -> rejected is probably right
>  - https://patchwork.ozlabs.org/patch/860540/
>    -> applied as bb084c0f61d659f0e6d371b096e0e57998f191d6
>  - https://patchwork.ozlabs.org/project/linux-pwm/list/?series=72798
>    -> rejected

It's sad to not get any feedback from you, and I'm not the only one
waiting for a reply. Linux is at -rc6 and there is only a single patch
in your (visible) queue for the next merge window although there are
some patches that look ready for application on the list.

I guess it's lack of time that keeps you from caring more for the pwm
stuff. If you need help (e.g. by caring for the patchwork todo list) I'm
willing to spend some time. If it helps you I can also collect the
patches that I think are ok and provide a pull request to your tree.

Best regards
Uwe
Thierry Reding Dec. 12, 2018, 10:25 a.m. | #6
On Tue, Dec 11, 2018 at 10:26:58PM +0100, Uwe Kleine-König wrote:
> On Thu, Dec 06, 2018 at 10:20:57AM +0100, Uwe Kleine-König wrote:
> > Hello Thierry,
> > 
> > On Fri, Nov 30, 2018 at 09:59:42AM +0100, Uwe Kleine-König wrote:
> > > On Tue, Nov 20, 2018 at 05:57:58PM +0100, Uwe Kleine-König wrote:
> > > > On Sun, Nov 04, 2018 at 10:19:45PM +0100, Uwe Kleine-König wrote:
> > > > > Hello Thierry,
> > > > > 
> > > > > On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> > > > > > The API to configure a PWM using pwm_enable(), pwm_disable(),
> > > > > > pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> > > > > > the parameters using pwm_apply_state(). To get forward with deprecating
> > > > > > the former set of functions use the opportunity that there is no current
> > > > > > user of pwm_set_polarity() and remove it.
> > > > > > 
> > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > 
> > > > > I think this patch is undisputed and wonder if it fell through the
> > > > > cracks given that a patch sent later made it into your pwm/for-4.20-rc1
> > > > > pull request.
> > > > 
> > > > Now that you pushed out my lpc patch to your for-next branch (thanks!),
> > > > I wonder what you plan to do with this patch.
> > > 
> > > Ping! There are also a few other patches that didn't catch your
> > > attention yet:
> > > 
> > >  - series for pwm-imx starting with
> > >    "pwm: imx: remove if block where the condition is always wrong"
> > >  - "pwm: don't use memcmp to compare struct state variables"
> > >  - "pwm: drop per-chip dbg_show callback"
> > 
> > In a different thread it was suggested to resend the patches instead of
> > sending a ping. Should I do that? There are also
> > 
> >  - pwm: rearrange structures to group members by purpose
> >  - [RFC] Documentation: pwm: rework documentation for the framework
> > 
> > which didn't get any feedback from you yet.
> > 
> > There is a patchwork instance for the pwm list[1]. Are you using that?
> > It seems to be orphaned for some time but was already used this year.
> > Would you accept some help here? E.g. 
> > 
> >  - https://patchwork.ozlabs.org/patch/1001130/
> >    -> superseeded as there is a v3
> >  - https://patchwork.ozlabs.org/patch/993022/
> >    -> rejected is probably right
> >  - https://patchwork.ozlabs.org/patch/860540/
> >    -> applied as bb084c0f61d659f0e6d371b096e0e57998f191d6
> >  - https://patchwork.ozlabs.org/project/linux-pwm/list/?series=72798
> >    -> rejected
> 
> It's sad to not get any feedback from you, and I'm not the only one
> waiting for a reply. Linux is at -rc6 and there is only a single patch
> in your (visible) queue for the next merge window although there are
> some patches that look ready for application on the list.
> 
> I guess it's lack of time that keeps you from caring more for the pwm
> stuff. If you need help (e.g. by caring for the patchwork todo list) I'm
> willing to spend some time. If it helps you I can also collect the
> patches that I think are ok and provide a pull request to your tree.

I certainly wouldn't mind a little help. Reviewing patches will
definitely help move things along.

Thierry
Thierry Reding Dec. 12, 2018, 10:56 a.m. | #7
On Mon, Oct 15, 2018 at 10:21:52AM +0200, Uwe Kleine-König wrote:
> The API to configure a PWM using pwm_enable(), pwm_disable(),
> pwm_config() and pwm_set_polarity() is superseeded by atomically setting
> the parameters using pwm_apply_state(). To get forward with deprecating
> the former set of functions use the opportunity that there is no current
> user of pwm_set_polarity() and remove it.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  include/linux/pwm.h | 42 ------------------------------------------
>  1 file changed, 42 deletions(-)

Applied, thanks. I did change the subject to start with "pwm: Drop" to
make it consistent with other commit messages.

Thierry
Uwe Kleine-König Dec. 12, 2018, 10:59 a.m. | #8
Hello Thierry,

On Wed, Dec 12, 2018 at 11:25:47AM +0100, Thierry Reding wrote:
> I certainly wouldn't mind a little help. Reviewing patches will
> definitely help move things along.

I already did. Apart from my patch series noted before:

 - series for pwm-imx starting with
   "pwm: imx: remove if block where the condition is always wrong"
 - "pwm: don't use memcmp to compare struct state variables"
 - "pwm: drop per-chip dbg_show callback"

I think the following patches are ready for application:

 - [v3] pwm: kconfig: enable kona pwm to be built for cygnus arch
 - [13/17] pwm: bcm2835: Switch to SPDX identifier

As I only recently subscribed to the linux-pwm list I don't have the
older patches listed on
https://patchwork.ozlabs.org/project/linux-pwm/list/ in my inbox. Just
grabbed Michal's imx get_state series from there and replied.

Other than that feedback to my RFC doc patch would be great. One thing I
didn't mention there is: Is pwm_disable supposed to wait for the current
period to end? I'd say yes (even though it might make sense to have the
possibility to stop immediately in some cases).

Best regards
Uwe

Patch

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 56518adc31dd..d5199b507d79 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -348,42 +348,6 @@  static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
 	return pwm_apply_state(pwm, &state);
 }
 
-/**
- * pwm_set_polarity() - configure the polarity of a PWM signal
- * @pwm: PWM device
- * @polarity: new polarity of the PWM signal
- *
- * Note that the polarity cannot be configured while the PWM device is
- * enabled.
- *
- * Returns: 0 on success or a negative error code on failure.
- */
-static inline int pwm_set_polarity(struct pwm_device *pwm,
-				   enum pwm_polarity polarity)
-{
-	struct pwm_state state;
-
-	if (!pwm)
-		return -EINVAL;
-
-	pwm_get_state(pwm, &state);
-	if (state.polarity == polarity)
-		return 0;
-
-	/*
-	 * Changing the polarity of a running PWM without adjusting the
-	 * dutycycle/period value is a bit risky (can introduce glitches).
-	 * Return -EBUSY in this case.
-	 * Note that this is allowed when using pwm_apply_state() because
-	 * the user specifies all the parameters.
-	 */
-	if (state.enabled)
-		return -EBUSY;
-
-	state.polarity = polarity;
-	return pwm_apply_state(pwm, &state);
-}
-
 /**
  * pwm_enable() - start a PWM output toggling
  * @pwm: PWM device
@@ -483,12 +447,6 @@  static inline int pwm_capture(struct pwm_device *pwm,
 	return -EINVAL;
 }
 
-static inline int pwm_set_polarity(struct pwm_device *pwm,
-				   enum pwm_polarity polarity)
-{
-	return -ENOTSUPP;
-}
-
 static inline int pwm_enable(struct pwm_device *pwm)
 {
 	return -EINVAL;