| Message ID | 20210225193426.3679817-1-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [GIT,PULL] pwm: Changes for v5.12-rc1 | expand |
On Thu, Feb 25, 2021 at 11:34 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > As I was generating the pull request I noticed that I forgot to fast- > forward this to v5.11-rc1 after the last merge window. Honestly, there is very little reason to ever do the fast-forward if the new development doesn't depend on new features, and I have absolutely no issues pulling something like this that is simply just a continuation of a previous development tree. So you did the right thing in not re-basing, this looks fine to me. Obviously, every once in a while a development tree would want to update to a newer base, just because of various infrastructure changes that could otherwise cause semantic conflicts etc if you end up basing stuff on something _truly_ ancient. But a release or two? No problem at all. Thanks, Linus
The pull request you sent on Thu, 25 Feb 2021 20:34:26 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git tags/pwm/for-5.12-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2c87f7a38f930ef6f6a7bdd04aeb82ce3971b54b
Thank you!
Hello Thierry, On Thu, Feb 25, 2021 at 08:34:26PM +0100, Thierry Reding wrote: > ---------------------------------------------------------------- > pwm: Changes for v5.12-rc1 > > The ZTE ZX platform is being removed, so the PWM driver is no longer > needed and removed as well. Other than that this contains a small set of > fixes and cleanups across a couple of drivers. patches I'd have liked to be seen additionally in this pull request are: pwm: bcm2835: Improve period and duty cycle calculation https://patchwork.ozlabs.org/project/linux-pwm/patch/20210114204804.143892-1-u.kleine-koenig@pengutronix.de/ pwm: get rid of pwmchip_add_with_polarity() https://patchwork.ozlabs.org/project/linux-pwm/list/?series=219012 pwm: add a config symbol for legacy drivers https://patchwork.ozlabs.org/project/linux-pwm/patch/20200613155742.31528-1-uwe@kleine-koenig.org/ Can you please consider these for your next pull request or provide feedback in the respective mailing list threads. Best regards Uwe
On Fri, Feb 26, 2021 at 10:59:36AM +0100, Uwe Kleine-König wrote: > Hello Thierry, > > On Thu, Feb 25, 2021 at 08:34:26PM +0100, Thierry Reding wrote: > > ---------------------------------------------------------------- > > pwm: Changes for v5.12-rc1 > > > > The ZTE ZX platform is being removed, so the PWM driver is no longer > > needed and removed as well. Other than that this contains a small set of > > fixes and cleanups across a couple of drivers. > > patches I'd have liked to be seen additionally in this pull request are: > > pwm: bcm2835: Improve period and duty cycle calculation > https://patchwork.ozlabs.org/project/linux-pwm/patch/20210114204804.143892-1-u.kleine-koenig@pengutronix.de/ There was discussion on earlier versions of this, so I was hoping that Lino and/or Florian would provide a Reviewed-by/Acked-by/Tested-by on this. I'll go ping them to see if we can get a reaction. > pwm: get rid of pwmchip_add_with_polarity() > https://patchwork.ozlabs.org/project/linux-pwm/list/?series=219012 From a quick look I'm not sure I understand what this is trying to do. The goal of pwmchip_add_with_polarity() was initially to support chips that support only PWM_POLARITY_INVERSED (or use inversed polarity by default, rather than normal polarity). So without looking at it more closely it doesn't seem quite right to drop it. > pwm: add a config symbol for legacy drivers > https://patchwork.ozlabs.org/project/linux-pwm/patch/20200613155742.31528-1-uwe@kleine-koenig.org/ I think we've discussed this in the past. I don't see the point in this because your patch marks 30 out of 58 drivers as legacy. Calling the majority "legacy" is a bit of a stretch. Also, I don't see how this changes anything. It doesn't actually simplify the core because legacy code can't be removed. It's only "simplified" if we don't actually select the PWM_LEGACY_DRIVERS symbol, but that's both not going to happen on most configurations and doesn't actually simplify anything from a maintenance point of view. If anything it further complicates things because now we have to test that everything builds fine with or without PWM_LEGACY_DRIVERS. Also, going forward I don't plan on merging any drivers that don't use the atomic API, and I don't need a Kconfig option to remind me of that. A better way forward would be to start converting some of these drivers to the atomic API since there's apparently not enough incentive for the driver maintainers to do that themselves. Thierry
On Fri, Feb 26, 2021 at 12:53:38PM +0100, Thierry Reding wrote: > On Fri, Feb 26, 2021 at 10:59:36AM +0100, Uwe Kleine-König wrote: > > Hello Thierry, > > > > On Thu, Feb 25, 2021 at 08:34:26PM +0100, Thierry Reding wrote: > > > ---------------------------------------------------------------- > > > pwm: Changes for v5.12-rc1 > > > > > > The ZTE ZX platform is being removed, so the PWM driver is no longer > > > needed and removed as well. Other than that this contains a small set of > > > fixes and cleanups across a couple of drivers. > > > > patches I'd have liked to be seen additionally in this pull request are: > > > > pwm: bcm2835: Improve period and duty cycle calculation > > https://patchwork.ozlabs.org/project/linux-pwm/patch/20210114204804.143892-1-u.kleine-koenig@pengutronix.de/ > > There was discussion on earlier versions of this, so I was hoping that > Lino and/or Florian would provide a Reviewed-by/Acked-by/Tested-by on > this. I'll go ping them to see if we can get a reaction. Fine. > > pwm: get rid of pwmchip_add_with_polarity() > > https://patchwork.ozlabs.org/project/linux-pwm/list/?series=219012 > > From a quick look I'm not sure I understand what this is trying to do. > The goal of pwmchip_add_with_polarity() was initially to support chips > that support only PWM_POLARITY_INVERSED (or use inversed polarity by > default, rather than normal polarity). So without looking at it more > closely it doesn't seem quite right to drop it. This is at least not the effect of pwmchip_add_with_polarity. There is nothing in the core that ensures that apply with the other polarity fails. There are two drivers that make use of pwmchip_add_with_polarity(); both support both polarities. > > pwm: add a config symbol for legacy drivers > > https://patchwork.ozlabs.org/project/linux-pwm/patch/20200613155742.31528-1-uwe@kleine-koenig.org/ > > I think we've discussed this in the past. If so I don't remember and it wasn't on the list. > I don't see the point in this because your patch marks 30 out of 58 > drivers as legacy. Calling the majority "legacy" is a bit of a > stretch. Also, I don't see how this changes anything. It doesn't > actually simplify the core because legacy code can't be removed. It's > only "simplified" if we don't actually select the PWM_LEGACY_DRIVERS > symbol, but that's both not going to happen on most configurations and > doesn't actually simplify anything from a maintenance point of view. > If anything it further complicates things because now we have to test > that everything builds fine with or without PWM_LEGACY_DRIVERS. Also, > going forward I don't plan on merging any drivers that don't use the > atomic API, and I don't need a Kconfig option to remind me of that. My motivation is that it is obvious for people adding new drivers that they make something wrong when they need this symbol. > A better way forward would be to start converting some of these drivers > to the atomic API since there's apparently not enough incentive for the > driver maintainers to do that themselves. That's fine, I can work on that. Best regards Uwe
Hello Thierry, On Sat, Feb 27, 2021 at 06:46:08PM +0100, Uwe Kleine-König wrote: > On Fri, Feb 26, 2021 at 12:53:38PM +0100, Thierry Reding wrote: > > On Fri, Feb 26, 2021 at 10:59:36AM +0100, Uwe Kleine-König wrote: > > > On Thu, Feb 25, 2021 at 08:34:26PM +0100, Thierry Reding wrote: > > > > ---------------------------------------------------------------- > > > > pwm: Changes for v5.12-rc1 > > > > > > > > The ZTE ZX platform is being removed, so the PWM driver is no longer > > > > needed and removed as well. Other than that this contains a small set of > > > > fixes and cleanups across a couple of drivers. > > > > > > patches I'd have liked to be seen additionally in this pull request are: > > > > > > pwm: bcm2835: Improve period and duty cycle calculation > > > https://patchwork.ozlabs.org/project/linux-pwm/patch/20210114204804.143892-1-u.kleine-koenig@pengutronix.de/ > > > > There was discussion on earlier versions of this, so I was hoping that > > Lino and/or Florian would provide a Reviewed-by/Acked-by/Tested-by on > > this. I'll go ping them to see if we can get a reaction. > > Fine. Lino tested and reviewed that change in the meantime, so I assume that's ready to go. > > > pwm: get rid of pwmchip_add_with_polarity() > > > https://patchwork.ozlabs.org/project/linux-pwm/list/?series=219012 > > > > From a quick look I'm not sure I understand what this is trying to do. > > The goal of pwmchip_add_with_polarity() was initially to support chips > > that support only PWM_POLARITY_INVERSED (or use inversed polarity by > > default, rather than normal polarity). So without looking at it more > > closely it doesn't seem quite right to drop it. > > This is at least not the effect of pwmchip_add_with_polarity. There is > nothing in the core that ensures that apply with the other polarity > fails. > > There are two drivers that make use of pwmchip_add_with_polarity(); both > support both polarities. Does this convince you? If the intention of pwmchip_add_with_polarity() is that only the passed polarity is supported than the first two commits pwm: atmel-hlcdc: Use pwmchip_add() instead of pwmchip_add_with_polarity() pwm: bcm-kona: Use pwmchip_add() instead of pwmchip_add_with_polarity() are right for sure and pwmchip_add() shouldn't be a wrapper around pwmchip_add_with_polarity(). I think that lowlevel drivers should be polarity aware and return -EINVAL when .apply() is called with an unsupported polarity. This is how most drivers work[1] and conceptually I think this is the right approach to have the capabilities of each driver in a single place. > > > pwm: add a config symbol for legacy drivers > > > https://patchwork.ozlabs.org/project/linux-pwm/patch/20200613155742.31528-1-uwe@kleine-koenig.org/ > > > > I think we've discussed this in the past. > [...] > > A better way forward would be to start converting some of these drivers > > to the atomic API since there's apparently not enough incentive for the > > driver maintainers to do that themselves. > > That's fine, I can work on that. I already did this for two drivers: pwm: ab8500: Implement .apply instead of .config, .enable and .disable pwm: atmel-tcb: Implement .apply callback I prepared a branch with the mentioned patches and a few further patches from the list at https://git.pengutronix.de/git/ukl/linux pwm-next that I would consider ready for pushing to next. I'd really like to use the time until the next merge window opens to breed patches in next and not only start collecting at -rc6 (or so) time. Best regards Uwe [1] There are two drivers that currently miss this check, I just send out patches to fix these.
Hi Linus, The following changes since commit 6eefb79d6f5bc4086bd02c76f1072dd4a8d9d9f6: pwm: sun4i: Remove erroneous else branch (2020-12-17 14:23:49 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git tags/pwm/for-5.12-rc1 for you to fetch changes up to 9a9dd7e473517b68412fd2da3da8a4aeb4ecb38a: pwm: lpc18xx-sct: remove unneeded semicolon (2021-02-22 15:20:43 +0100) As I was generating the pull request I noticed that I forgot to fast- forward this to v5.11-rc1 after the last merge window. However, I did not think a last-minute rebase was appropriate. I did go back and ran my test builds on a rebase on top of v5.11-rc1 and everything checked out, so I think this is safe to merge. Also, linux-next would have caught any problems related to this. I'll make sure to properly roll forward the branch next time. Thanks, Thierry ---------------------------------------------------------------- pwm: Changes for v5.12-rc1 The ZTE ZX platform is being removed, so the PWM driver is no longer needed and removed as well. Other than that this contains a small set of fixes and cleanups across a couple of drivers. ---------------------------------------------------------------- Arnd Bergmann (1): pwm: Remove ZTE ZX driver Jeff LaBundy (1): pwm: iqs620a: Correct a stale state variable Simon South (5): pwm: rockchip: Enable APB clock during register access while probing pwm: rockchip: rockchip_pwm_probe(): Remove superfluous clk_unprepare() pwm: rockchip: Replace "bus clk" with "PWM clk" pwm: rockchip: Eliminate potential race condition when probing pwm: rockchip: Enable clock before calling clk_get_rate() Uwe Kleine-König (1): pwm: iqs620a: Fix overflow and optimize calculations Yang Li (1): pwm: lpc18xx-sct: remove unneeded semicolon Documentation/devicetree/bindings/pwm/pwm-zx.txt | 22 -- drivers/pwm/Kconfig | 10 - drivers/pwm/Makefile | 1 - drivers/pwm/pwm-iqs620a.c | 94 ++++---- drivers/pwm/pwm-lpc18xx-sct.c | 2 +- drivers/pwm/pwm-rockchip.c | 32 ++- drivers/pwm/pwm-zx.c | 278 ----------------------- 7 files changed, 65 insertions(+), 374 deletions(-) delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-zx.txt delete mode 100644 drivers/pwm/pwm-zx.c