[GIT PULL] pwm: Changes for v5.5-rc1

[GIT PULL] pwm: Changes for v5.5-rc1

[Thierry Reding]

Hi Linus,

The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:

  Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)

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.5-rc1

for you to fetch changes up to f5ff2628867b9c7cb4abb6c6a5a7eea079dad4b6:

  pwm: imx27: Unconditionally write state to hardware (2019-10-21 16:58:09 +0200)

Thanks,
Thierry

----------------------------------------------------------------
pwm: Changes for v5.5-rc1

Various changes and minor fixes across a couple of drivers.

----------------------------------------------------------------
Colin Ian King (1):
      pwm: sun4i: Drop redundant assignment to variable pval

Fabrice Gasnier (3):
      dt-bindings: pwm-stm32: Document pinctrl sleep state
      pwm: stm32: Split breakinput apply routine to ease PM support
      pwm: stm32: Add power management support

Ondrej Jirman (1):
      pwm: sun4i: Fix incorrect calculation of duty_cycle/period

Rasmus Villemoes (1):
      pwm: Update comment on struct pwm_ops::apply

Thierry Reding (8):
      dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
      pwm: stm32: Validate breakinput data from DT
      pwm: stm32: Remove clutter from ternary operator
      pwm: stm32: Pass breakinput instead of its values
      pwm: Read initial hardware state at request time
      pwm: cros-ec: Cache duty cycle value
      pwm: imx27: Cache duty cycle register value
      pwm: imx27: Unconditionally write state to hardware

 .../devicetree/bindings/pwm/pwm-mediatek.txt       |   2 +-
 .../devicetree/bindings/pwm/pwm-stm32.txt          |   8 +-
 drivers/pwm/core.c                                 |   6 +-
 drivers/pwm/pwm-cros-ec.c                          |  58 ++++++++-
 drivers/pwm/pwm-imx27.c                            | 137 ++++++++++++---------
 drivers/pwm/pwm-stm32.c                            | 112 ++++++++++++-----
 drivers/pwm/pwm-sun4i.c                            |   5 +-
 include/linux/mfd/stm32-timers.h                   |  12 +-
 include/linux/pwm.h                                |   5 +-
 9 files changed, 228 insertions(+), 117 deletions(-)

Re: [GIT PULL] pwm: Changes for v5.5-rc1

[Uwe Kleine-König]

Hello Thierry,

On Thu, Dec 05, 2019 at 07:10:44AM +0100, Thierry Reding wrote:
> The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:
> 
>   Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)
> 
> 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.5-rc1
> 
> for you to fetch changes up to f5ff2628867b9c7cb4abb6c6a5a7eea079dad4b6:
> 
>   pwm: imx27: Unconditionally write state to hardware (2019-10-21 16:58:09 +0200)
> 
> Thanks,
> Thierry
> 
> ----------------------------------------------------------------
> pwm: Changes for v5.5-rc1
> 
> Various changes and minor fixes across a couple of drivers.
> 
> ----------------------------------------------------------------
> Colin Ian King (1):
>       pwm: sun4i: Drop redundant assignment to variable pval
> 
> Fabrice Gasnier (3):
>       dt-bindings: pwm-stm32: Document pinctrl sleep state
>       pwm: stm32: Split breakinput apply routine to ease PM support
>       pwm: stm32: Add power management support
> 
> Ondrej Jirman (1):
>       pwm: sun4i: Fix incorrect calculation of duty_cycle/period
> 
> Rasmus Villemoes (1):
>       pwm: Update comment on struct pwm_ops::apply
> 
> Thierry Reding (8):
>       dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
>       pwm: stm32: Validate breakinput data from DT
>       pwm: stm32: Remove clutter from ternary operator
>       pwm: stm32: Pass breakinput instead of its values
>       pwm: Read initial hardware state at request time
>       pwm: cros-ec: Cache duty cycle value
>       pwm: imx27: Cache duty cycle register value
>       pwm: imx27: Unconditionally write state to hardware

It's a bit of a surprise for me that you included the three last patches
as last minute changes. I'm not sure if I oppose them, but they were not
in next (as of next-20191205) and I would really like to have some time
for patches (that are not obvious fixes of course) there before they go
into a pull request. And if it's only to get some transparency.
(But in this case I had the impression that the discussion isn't over
yet, your last mail in the thread said: "I'm not sure yet about the
remainder of the series. Depending on what we decide to do about drivers
that can't (or don't want to) write all state through to the hardware,
patches 2-4 may become moot." in October which made me expect there is
still something to come, at least a statement before the fact. Still
more as also several further drivers are affected (according to my
research described in
https://patchwork.ozlabs.org/patch/1178351/#2282269).)

In return there are a few patches I would really have liked to be seen
here:

 - The series "Updates for the atmel PWM driver" that didn't get any
   response from you since August although it got some acks from the
   atmel guys.
   (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=127096&state=*,
   This is marked Superseeded for reasons unknown to me that I asked
   about in October.)

 - The series starting with "pwm: rcar: Drop useless call to
   pwm_get_state" from October which got reviews by a renesas guy.
   (https://patchwork.ozlabs.org/patch/1182649/,
   https://patchwork.ozlabs.org/patch/1182648/,
   Patchwork didn't identify this as a series, so listing the patches
   individually.)

 - The patch "pwm: implement tracing for .get_state() and
   .apply_state()" that got an review by Steven Rostedt.
   (https://patchwork.ozlabs.org/patch/1182679/)

 - The series starting with "pwm: omap-dmtimer: remove pwmchip in
   .remove before making it unfunctional" from November which IMHO is
   simple and contains two fixes
   (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=142030)

And I'm still waiting for feedback on

 - "Documentation: pwm: rework documentation for the framework" (since
   January)

 - "pwm: add debug knob to help driver authors" (since August)

:-(
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [GIT PULL] pwm: Changes for v5.5-rc1

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5354 bytes --]

On Thu, Dec 05, 2019 at 08:59:58AM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Thu, Dec 05, 2019 at 07:10:44AM +0100, Thierry Reding wrote:
> > The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:
> > 
> >   Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)
> > 
> > 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.5-rc1
> > 
> > for you to fetch changes up to f5ff2628867b9c7cb4abb6c6a5a7eea079dad4b6:
> > 
> >   pwm: imx27: Unconditionally write state to hardware (2019-10-21 16:58:09 +0200)
> > 
> > Thanks,
> > Thierry
> > 
> > ----------------------------------------------------------------
> > pwm: Changes for v5.5-rc1
> > 
> > Various changes and minor fixes across a couple of drivers.
> > 
> > ----------------------------------------------------------------
> > Colin Ian King (1):
> >       pwm: sun4i: Drop redundant assignment to variable pval
> > 
> > Fabrice Gasnier (3):
> >       dt-bindings: pwm-stm32: Document pinctrl sleep state
> >       pwm: stm32: Split breakinput apply routine to ease PM support
> >       pwm: stm32: Add power management support
> > 
> > Ondrej Jirman (1):
> >       pwm: sun4i: Fix incorrect calculation of duty_cycle/period
> > 
> > Rasmus Villemoes (1):
> >       pwm: Update comment on struct pwm_ops::apply
> > 
> > Thierry Reding (8):
> >       dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
> >       pwm: stm32: Validate breakinput data from DT
> >       pwm: stm32: Remove clutter from ternary operator
> >       pwm: stm32: Pass breakinput instead of its values
> >       pwm: Read initial hardware state at request time
> >       pwm: cros-ec: Cache duty cycle value
> >       pwm: imx27: Cache duty cycle register value
> >       pwm: imx27: Unconditionally write state to hardware
> 
> It's a bit of a surprise for me that you included the three last patches
> as last minute changes. I'm not sure if I oppose them, but they were not
> in next (as of next-20191205) and I would really like to have some time
> for patches (that are not obvious fixes of course) there before they go
> into a pull request. And if it's only to get some transparency.
> (But in this case I had the impression that the discussion isn't over
> yet, your last mail in the thread said: "I'm not sure yet about the
> remainder of the series. Depending on what we decide to do about drivers
> that can't (or don't want to) write all state through to the hardware,
> patches 2-4 may become moot." in October which made me expect there is
> still something to come, at least a statement before the fact. Still
> more as also several further drivers are affected (according to my
> research described in
> https://patchwork.ozlabs.org/patch/1178351/#2282269).)

Yes, the last four patches weren't meant to be in this pull request.
That's what I get for trying to squeeze this in before coffee.

Linus, please ignore this one, I'll send out a new pull request shortly.

> In return there are a few patches I would really have liked to be seen
> here:
> 
>  - The series "Updates for the atmel PWM driver" that didn't get any
>    response from you since August although it got some acks from the
>    atmel guys.
>    (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=127096&state=*,
>    This is marked Superseeded for reasons unknown to me that I asked
>    about in October.)

I usually mark patches as superseeded when there are new revisions. It
looks like I might have marked these accidentally since I can't see any
new versions of that series. I'll pick those up after v5.5-rc1.

>  - The series starting with "pwm: rcar: Drop useless call to
>    pwm_get_state" from October which got reviews by a renesas guy.
>    (https://patchwork.ozlabs.org/patch/1182649/,
>    https://patchwork.ozlabs.org/patch/1182648/,
>    Patchwork didn't identify this as a series, so listing the patches
>    individually.)

I missed those. Please do ping me if I haven't reviewed or applied
patches after a week or so to remind me. Sometimes my inbox fills up so
quickly that some patches get lost.

>  - The patch "pwm: implement tracing for .get_state() and
>    .apply_state()" that got an review by Steven Rostedt.
>    (https://patchwork.ozlabs.org/patch/1182679/)

Review for this came in after v5.4-rc7, so I didn't consider it for
v5.5. I'll pick it up after v5.5-rc1.

>  - The series starting with "pwm: omap-dmtimer: remove pwmchip in
>    .remove before making it unfunctional" from November which IMHO is
>    simple and contains two fixes
>    (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=142030)

Same here.

> And I'm still waiting for feedback on
> 
>  - "Documentation: pwm: rework documentation for the framework" (since
>    January)

Please resend this, I can't find it in my inbox.

>  - "pwm: add debug knob to help driver authors" (since August)

My recollection is that this flagged a bunch of issues right out of the
box, so I'm hesitant to apply it without wider concensus that we want
this, or some effort to address the issues that this flags.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [GIT PULL] pwm: Changes for v5.5-rc1

[Uwe Kleine-König]

Hello Thierry,

On Thu, Dec 05, 2019 at 09:41:02AM +0100, Thierry Reding wrote:
> On Thu, Dec 05, 2019 at 08:59:58AM +0100, Uwe Kleine-König wrote:
> > On Thu, Dec 05, 2019 at 07:10:44AM +0100, Thierry Reding wrote:
> > > The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:
> > > 
> > >   Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)
> > > 
> > > 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.5-rc1
> > > 
> > > for you to fetch changes up to f5ff2628867b9c7cb4abb6c6a5a7eea079dad4b6:
> > > 
> > >   pwm: imx27: Unconditionally write state to hardware (2019-10-21 16:58:09 +0200)
> > > 
> > > Thanks,
> > > Thierry
> > > 
> > > ----------------------------------------------------------------
> > > pwm: Changes for v5.5-rc1
> > > 
> > > Various changes and minor fixes across a couple of drivers.
> > > 
> > > ----------------------------------------------------------------
> > > Colin Ian King (1):
> > >       pwm: sun4i: Drop redundant assignment to variable pval
> > > 
> > > Fabrice Gasnier (3):
> > >       dt-bindings: pwm-stm32: Document pinctrl sleep state
> > >       pwm: stm32: Split breakinput apply routine to ease PM support
> > >       pwm: stm32: Add power management support
> > > 
> > > Ondrej Jirman (1):
> > >       pwm: sun4i: Fix incorrect calculation of duty_cycle/period
> > > 
> > > Rasmus Villemoes (1):
> > >       pwm: Update comment on struct pwm_ops::apply
> > > 
> > > Thierry Reding (8):
> > >       dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
> > >       pwm: stm32: Validate breakinput data from DT
> > >       pwm: stm32: Remove clutter from ternary operator
> > >       pwm: stm32: Pass breakinput instead of its values
> > >       pwm: Read initial hardware state at request time
> > >       pwm: cros-ec: Cache duty cycle value
> > >       pwm: imx27: Cache duty cycle register value
> > >       pwm: imx27: Unconditionally write state to hardware
> > 
> > It's a bit of a surprise for me that you included the three last patches
> > as last minute changes. I'm not sure if I oppose them, but they were not
> > in next (as of next-20191205) and I would really like to have some time
> > for patches (that are not obvious fixes of course) there before they go
> > into a pull request. And if it's only to get some transparency.
> > (But in this case I had the impression that the discussion isn't over
> > yet, your last mail in the thread said: "I'm not sure yet about the
> > remainder of the series. Depending on what we decide to do about drivers
> > that can't (or don't want to) write all state through to the hardware,
> > patches 2-4 may become moot." in October which made me expect there is
> > still something to come, at least a statement before the fact. Still
> > more as also several further drivers are affected (according to my
> > research described in
> > https://patchwork.ozlabs.org/patch/1178351/#2282269).)
> 
> Yes, the last four patches weren't meant to be in this pull request.
> That's what I get for trying to squeeze this in before coffee.

Ah right, it's four patches, not three. (I thought I saw "pwm: Read
initial hardware state at request time" in next.)

> Please do ping me if I haven't reviewed or applied patches after a
> week or so to remind me. Sometimes my inbox fills up so quickly that
> some patches get lost.

ok.

> >  - The patch "pwm: implement tracing for .get_state() and
> >    .apply_state()" that got an review by Steven Rostedt.
> >    (https://patchwork.ozlabs.org/patch/1182679/)
> 
> Review for this came in after v5.4-rc7, so I didn't consider it for
> v5.5. I'll pick it up after v5.5-rc1.

I got Steven's mail on Oct 24 which is the week between -rc4 and -rc5,
but ok, I won't argue.
> 
> >  - The series starting with "pwm: omap-dmtimer: remove pwmchip in
> >    .remove before making it unfunctional" from November which IMHO is
> >    simple and contains two fixes
> >    (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=142030)
> 
> Same here.

Does "after v5.5-rc1" mean "for v5.5" or "for v5.6-rc1". I agree that
the tracing stuff is merge window material (very useful though in my
eyes) while the omap-dmtimer series (at least the first 3 of 4 patches)
is about fixes.

> > And I'm still waiting for feedback on
> > 
> >  - "Documentation: pwm: rework documentation for the framework" (since
> >    January)
> 
> Please resend this, I can't find it in my inbox.

:-|, given that I sent this already twice, pinged several times
(https://patchwork.ozlabs.org/patch/1021566/,
https://patchwork.ozlabs.org/patch/1000709/) and also asked at least
once before in a mail where I pinged several patches using a list.

> >  - "pwm: add debug knob to help driver authors" (since August)
> 
> My recollection is that this flagged a bunch of issues right out of the
> box, so I'm hesitant to apply it without wider concensus that we want
> this, or some effort to address the issues that this flags.

I didn't want you to apply it. That it is not ready for that is out of
the question. I assume the patch doesn't apply anymore and needs work
for sure. The last mail in the respective thread had a single paragraph:

	do you consider the idea here worthwile? If so I'd update the
	patch to current mainline and address the feedback I got so far.

This is still interesting, as I don't want to spend my time working on
an idea that is then turned down in the end for conceptual reasons.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [GIT PULL] pwm: Changes for v5.5-rc1

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 7333 bytes --]

On Thu, Dec 05, 2019 at 11:05:35AM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Thu, Dec 05, 2019 at 09:41:02AM +0100, Thierry Reding wrote:
> > On Thu, Dec 05, 2019 at 08:59:58AM +0100, Uwe Kleine-König wrote:
> > > On Thu, Dec 05, 2019 at 07:10:44AM +0100, Thierry Reding wrote:
> > > > The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:
> > > > 
> > > >   Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)
> > > > 
> > > > 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.5-rc1
> > > > 
> > > > for you to fetch changes up to f5ff2628867b9c7cb4abb6c6a5a7eea079dad4b6:
> > > > 
> > > >   pwm: imx27: Unconditionally write state to hardware (2019-10-21 16:58:09 +0200)
> > > > 
> > > > Thanks,
> > > > Thierry
> > > > 
> > > > ----------------------------------------------------------------
> > > > pwm: Changes for v5.5-rc1
> > > > 
> > > > Various changes and minor fixes across a couple of drivers.
> > > > 
> > > > ----------------------------------------------------------------
> > > > Colin Ian King (1):
> > > >       pwm: sun4i: Drop redundant assignment to variable pval
> > > > 
> > > > Fabrice Gasnier (3):
> > > >       dt-bindings: pwm-stm32: Document pinctrl sleep state
> > > >       pwm: stm32: Split breakinput apply routine to ease PM support
> > > >       pwm: stm32: Add power management support
> > > > 
> > > > Ondrej Jirman (1):
> > > >       pwm: sun4i: Fix incorrect calculation of duty_cycle/period
> > > > 
> > > > Rasmus Villemoes (1):
> > > >       pwm: Update comment on struct pwm_ops::apply
> > > > 
> > > > Thierry Reding (8):
> > > >       dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
> > > >       pwm: stm32: Validate breakinput data from DT
> > > >       pwm: stm32: Remove clutter from ternary operator
> > > >       pwm: stm32: Pass breakinput instead of its values
> > > >       pwm: Read initial hardware state at request time
> > > >       pwm: cros-ec: Cache duty cycle value
> > > >       pwm: imx27: Cache duty cycle register value
> > > >       pwm: imx27: Unconditionally write state to hardware
> > > 
> > > It's a bit of a surprise for me that you included the three last patches
> > > as last minute changes. I'm not sure if I oppose them, but they were not
> > > in next (as of next-20191205) and I would really like to have some time
> > > for patches (that are not obvious fixes of course) there before they go
> > > into a pull request. And if it's only to get some transparency.
> > > (But in this case I had the impression that the discussion isn't over
> > > yet, your last mail in the thread said: "I'm not sure yet about the
> > > remainder of the series. Depending on what we decide to do about drivers
> > > that can't (or don't want to) write all state through to the hardware,
> > > patches 2-4 may become moot." in October which made me expect there is
> > > still something to come, at least a statement before the fact. Still
> > > more as also several further drivers are affected (according to my
> > > research described in
> > > https://patchwork.ozlabs.org/patch/1178351/#2282269).)
> > 
> > Yes, the last four patches weren't meant to be in this pull request.
> > That's what I get for trying to squeeze this in before coffee.
> 
> Ah right, it's four patches, not three. (I thought I saw "pwm: Read
> initial hardware state at request time" in next.)
> 
> > Please do ping me if I haven't reviewed or applied patches after a
> > week or so to remind me. Sometimes my inbox fills up so quickly that
> > some patches get lost.
> 
> ok.
> 
> > >  - The patch "pwm: implement tracing for .get_state() and
> > >    .apply_state()" that got an review by Steven Rostedt.
> > >    (https://patchwork.ozlabs.org/patch/1182679/)
> > 
> > Review for this came in after v5.4-rc7, so I didn't consider it for
> > v5.5. I'll pick it up after v5.5-rc1.
> 
> I got Steven's mail on Oct 24 which is the week between -rc4 and -rc5,
> but ok, I won't argue.

Both my inbox and patchwork say that the email arrived on November 13.

> > 
> > >  - The series starting with "pwm: omap-dmtimer: remove pwmchip in
> > >    .remove before making it unfunctional" from November which IMHO is
> > >    simple and contains two fixes
> > >    (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=142030)
> > 
> > Same here.
> 
> Does "after v5.5-rc1" mean "for v5.5" or "for v5.6-rc1". I agree that
> the tracing stuff is merge window material (very useful though in my
> eyes) while the omap-dmtimer series (at least the first 3 of 4 patches)
> is about fixes.

These all change code that's been like this for more than 4 years and
nobody's ever reported a bug. The very worst that can happen here is
that we leak a device reference, but these are platform devices, so
they aren't going to go anywhere anyway.

> > > And I'm still waiting for feedback on
> > > 
> > >  - "Documentation: pwm: rework documentation for the framework" (since
> > >    January)
> > 
> > Please resend this, I can't find it in my inbox.
> 
> :-|, given that I sent this already twice, pinged several times
> (https://patchwork.ozlabs.org/patch/1021566/,
> https://patchwork.ozlabs.org/patch/1000709/) and also asked at least
> once before in a mail where I pinged several patches using a list.

I find patchwork always difficult for reviewing, but that patch is
particularly difficult to review. You're basically rewriting all of the
documentation in a way that you think is better. It'd be much easier to
review if this was done more incrementally.

It looks like your goals could be easily met by just extending the
existing documentation, or bringing it up-to-date with the API.

> > >  - "pwm: add debug knob to help driver authors" (since August)
> > 
> > My recollection is that this flagged a bunch of issues right out of the
> > box, so I'm hesitant to apply it without wider concensus that we want
> > this, or some effort to address the issues that this flags.
> 
> I didn't want you to apply it. That it is not ready for that is out of
> the question. I assume the patch doesn't apply anymore and needs work
> for sure. The last mail in the respective thread had a single paragraph:
> 
> 	do you consider the idea here worthwile? If so I'd update the
> 	patch to current mainline and address the feedback I got so far.
> 
> This is still interesting, as I don't want to spend my time working on
> an idea that is then turned down in the end for conceptual reasons.

There's nothing conceptually wrong about it. The one problem that I see
with it is that we can't force people to use it because it's noisy when
things go wrong, and they currently do go wrong. On the other hand, if
we don't enforce its use, developers are likely to just ignore its
existence.

So I think we need to get drivers into a little better shape so that we
eliminate at least all of the issues that currently exist. Then we can
merge this and maybe opportunistically enforce this during a release
cycle to see if there's any fallout.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [GIT PULL] pwm: Changes for v5.5-rc1

[Uwe Kleine-König]

Hello Thierry,

On Thu, Dec 05, 2019 at 12:04:25PM +0100, Thierry Reding wrote:
> On Thu, Dec 05, 2019 at 11:05:35AM +0100, Uwe Kleine-König wrote:
> > On Thu, Dec 05, 2019 at 09:41:02AM +0100, Thierry Reding wrote:
> > > Review for this came in after v5.4-rc7, so I didn't consider it for
> > > v5.5. I'll pick it up after v5.5-rc1.
> > 
> > I got Steven's mail on Oct 24 which is the week between -rc4 and -rc5,
> > but ok, I won't argue.
> 
> Both my inbox and patchwork say that the email arrived on November 13.

Ah, you're right. Oct 24 was the date I sent the patch. Don't know what
I saw before. Please forgive my wrong correction.

> > > >  - The series starting with "pwm: omap-dmtimer: remove pwmchip in
> > > >    .remove before making it unfunctional" from November which IMHO is
> > > >    simple and contains two fixes
> > > >    (https://patchwork.ozlabs.org/project/linux-pwm/list/?series=142030)
> > > 
> > > Same here.
> > 
> > Does "after v5.5-rc1" mean "for v5.5" or "for v5.6-rc1". I agree that
> > the tracing stuff is merge window material (very useful though in my
> > eyes) while the omap-dmtimer series (at least the first 3 of 4 patches)
> > is about fixes.
> 
> These all change code that's been like this for more than 4 years and
> nobody's ever reported a bug.

That nobody uncovered a certain bug before should not stop us fixing it
quickly.

> The very worst that can happen here is that we leak a device
> reference, but these are platform devices, so they aren't going to go
> anywhere anyway.

Unless someone unloads the driver or unbinds it from the device. Even if
you question that this warrants an urgent fix (which I disagree to),
then patch 2 fixes an error path and patch 3 an unconditional resouce
leak (both in .probe).

> > > > And I'm still waiting for feedback on
> > > > 
> > > >  - "Documentation: pwm: rework documentation for the framework" (since
> > > >    January)
> > > 
> > > Please resend this, I can't find it in my inbox.
> > 
> > :-|, given that I sent this already twice, pinged several times
> > (https://patchwork.ozlabs.org/patch/1021566/,
> > https://patchwork.ozlabs.org/patch/1000709/) and also asked at least
> > once before in a mail where I pinged several patches using a list.
> 
> I find patchwork always difficult for reviewing, but that patch is
> particularly difficult to review. You're basically rewriting all of the
> documentation in a way that you think is better. It'd be much easier to
> review if this was done more incrementally.
> 
> It looks like your goals could be easily met by just extending the
> existing documentation, or bringing it up-to-date with the API.

OK, will try to split it in smaller chunks.

> > > >  - "pwm: add debug knob to help driver authors" (since August)
> > > 
> > > My recollection is that this flagged a bunch of issues right out of the
> > > box, so I'm hesitant to apply it without wider concensus that we want
> > > this, or some effort to address the issues that this flags.
> > 
> > I didn't want you to apply it. That it is not ready for that is out of
> > the question. I assume the patch doesn't apply anymore and needs work
> > for sure. The last mail in the respective thread had a single paragraph:
> > 
> > 	do you consider the idea here worthwile? If so I'd update the
> > 	patch to current mainline and address the feedback I got so far.
> > 
> > This is still interesting, as I don't want to spend my time working on
> > an idea that is then turned down in the end for conceptual reasons.
> 
> There's nothing conceptually wrong about it. The one problem that I see
> with it is that we can't force people to use it because it's noisy when
> things go wrong, and they currently do go wrong. On the other hand, if
> we don't enforce its use, developers are likely to just ignore its
> existence.

I agree that it doesn't help when a driver author has this disabled. But
still it helps to catch problems if testers know it or during review
something is catched and then you only need to point to this Kconfig
setting instead of referencing the documentation or otherwise explain
what you want.
And even if there is only a 0.2 chance that a prospective pwm driver
author will enable it and so have a better driver in patch submission
round 1 it's already worth the effort in my eyes.

All in all I think "but some might not profit unless you force them"
isn't a reason good enough to not do something for those who might
notice and benefit from your work. And even if it's only me who benefits
that's good enough, too (assuming there are no downsides for others).

> So I think we need to get drivers into a little better shape so that we

For getting there having this knob configurable (and default off) is
already helpful.

> eliminate at least all of the issues that currently exist. Then we can
> merge this and maybe opportunistically enforce this during a release
> cycle to see if there's any fallout.

And if there is fallout just having to change some Kconfig magic to
disable the checks would be good. So having PWM_DEBUG seems like a good
idea to me.

Not sure that defaulting to all these checks is a good idea as at least
some of them introduce some delays waiting for completed periods. Having
this configurable seems to work for other subsystems just fine (judging
from looking at the output of 

	git grep DEBUG drivers/*/Kconfig

).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[GIT PULL v2] pwm: Changes for v5.5-rc1

[Thierry Reding]

Hi Linus,

The following changes since commit 40a6b9a00930fd6b59aa2eb6135abc2efe5440c3:

  Revert "pwm: Let pwm_get_state() return the last implemented state" (2019-10-21 16:48:52 +0200)

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.5-rc1

for you to fetch changes up to 9e1b4999a1693d67cc87a887057d8012c28fb12b:

  pwm: stm32: Pass breakinput instead of its values (2019-10-21 16:50:05 +0200)

Thanks,
Thierry

----------------------------------------------------------------
pwm: Changes for v5.5-rc1

Various changes and minor fixes across a couple of drivers.

----------------------------------------------------------------
Colin Ian King (1):
      pwm: sun4i: Drop redundant assignment to variable pval

Fabrice Gasnier (3):
      dt-bindings: pwm-stm32: Document pinctrl sleep state
      pwm: stm32: Split breakinput apply routine to ease PM support
      pwm: stm32: Add power management support

Ondrej Jirman (1):
      pwm: sun4i: Fix incorrect calculation of duty_cycle/period

Rasmus Villemoes (1):
      pwm: Update comment on struct pwm_ops::apply

Thierry Reding (4):
      dt-bindings: pwm: mediatek: Remove gratuitous compatible string for MT7629
      pwm: stm32: Validate breakinput data from DT
      pwm: stm32: Remove clutter from ternary operator
      pwm: stm32: Pass breakinput instead of its values

 .../devicetree/bindings/pwm/pwm-mediatek.txt       |   2 +-
 .../devicetree/bindings/pwm/pwm-stm32.txt          |   8 +-
 drivers/pwm/pwm-stm32.c                            | 112 +++++++++++++++------
 drivers/pwm/pwm-sun4i.c                            |   5 +-
 include/linux/mfd/stm32-timers.h                   |  12 +--
 include/linux/pwm.h                                |   5 +-
 6 files changed, 95 insertions(+), 49 deletions(-)

Re: [GIT PULL v2] pwm: Changes for v5.5-rc1

[pr-tracker-bot]

The pull request you sent on Thu,  5 Dec 2019 09:42:03 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git tags/pwm/for-5.5-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/d9e48dc2a71a836f17d1febbedb31470f957edb4

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker