Re: [PATCH] dt-bindings: pwm: renesas: pwm-rcar: document R8A779{7|8}0 bindings

[Thierry Reding]

On Mon, Oct 15, 2018 at 01:57:13PM -0500, Rob Herring wrote:
> On Fri, Oct 12, 2018 at 01:21:14PM +0200, Thierry Reding wrote:
> > On Mon, Oct 01, 2018 at 10:57:39PM +0300, Sergei Shtylyov wrote:
> > > Document the R-Car V3{M|H} (R8A779{7|8}0) SoC in the  Renesas R-Car PWM
> > > bindings. R8A77970's hardware is a generic R-Car gen3 PWM, while R8A77980
> > > has an extra error injection register...
> > > 
> > > Signed-off-by: Sergei Shtylyov 
> > > 
> > > ---
> > > This patch is against the 'for-next' branch of Thierry Reding's 
> > > 'linux-pwm.git'
> > > repo.
> > > 
> > >  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt |2 ++
> > >  1 file changed, 2 insertions(+)
> > 
> > Applied, thanks.
> 
> Some reason your tree isn't updated for linux-next?

No particular reason, I just had not pushed it out yet because the build
tests were taking longer than expected and I had to clean up a couple of
minor things.

It's pushed out now.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: renesas-tpu: Document r8a7744 support

[Thierry Reding]

On Thu, Oct 04, 2018 at 05:21:34PM +0100, Biju Das wrote:
> Document r8a7744 specific compatible strings. No driver change is
> needed as the fallback compatible string "renesas,tpu" activates the
> right code in the driver.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 
> ---
> This patch is tested against next-20181004
> ---
>  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: rcar: Add r8a7744 support

[Thierry Reding]

On Thu, Oct 04, 2018 at 05:17:19PM +0100, Biju Das wrote:
> Document RZ/G1N (R8A7744) SoC bindings.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 
> ---
> This patch is tested against next-20181004
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: renesas: pwm-rcar: document R8A779{7|8}0 bindings

[Thierry Reding]

On Mon, Oct 01, 2018 at 10:57:39PM +0300, Sergei Shtylyov wrote:
> Document the R-Car V3{M|H} (R8A779{7|8}0) SoC in the  Renesas R-Car PWM
> bindings. R8A77970's hardware is a generic R-Car gen3 PWM, while R8A77980
> has an extra error injection register...
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'for-next' branch of Thierry Reding's 
> 'linux-pwm.git'
> repo.
> 
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt |2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: renesas: tpu: document R8A779{7|8}0 bindings

[Thierry Reding]

On Sat, Sep 22, 2018 at 10:57:29PM +0300, Sergei Shtylyov wrote:
> Document the R-Car V3{M|H} (R8A779{7|8}0) SoC in the Renesas TPU bindings;
> the TPU hardware in those is the Renesas standard 4-channel timer pulse
> unit.
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against Linus' repo plus the bindings fix just reposted...
> 
>  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt |4 
>  1 file changed, 4 insertions(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2] dt-bindings: pwm: renesas: tpu: fix "compatible" prop description

[Thierry Reding]

On Sat, Sep 22, 2018 at 10:43:24PM +0300, Sergei Shtylyov wrote:
> The "compatible" property description contradicts even the example given:
> it only says that there must be a single value while the example has the
> fallback value too -- which makes much more sense. Moreover, the generic
> property value is misdocumented as being R-Car (and RZ/G1) specific...
> 
> Fixes: 382457e562bb ("pwm: renesas-tpu: Add DT support")
> Fixes: 3ba111a01822 ("dt-bindings: pwm: renesas-tpu: Document r8a774[35] 
> support")
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against Linus' repo -- the 'fixes' branch in Thierry Reding's
> 'linux-pwm.git' repo is very outdated...
> 
> Changes in version 2:
> - fixed the wording of the fallback value description.
> 
>  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH resend] pwm: renesas-tpu: convert to SPDX identifiers

[Thierry Reding]

On Wed, Sep 26, 2018 at 01:45:47AM +, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto 
> 
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
> 
> Signed-off-by: Kuninori Morimoto 
> Reviewed-by: Simon Horman 
> ---
> Thierry
> 
> 2weeks past, resend patch
> 
>  drivers/pwm/pwm-renesas-tpu.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH resend] pwm: rcar: convert to SPDX identifiers

[Thierry Reding]

On Wed, Sep 26, 2018 at 01:45:17AM +, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto 
> 
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
> 
> Signed-off-by: Kuninori Morimoto 
> Reviewed-by: Simon Horman 
> ---
> Thierry
> 
> 2weeks past, resend patch

There's usually no need to resend a patch, just ping me on the original.

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2] dt-bindings: pwm: renesas: tpu: fix "compatible" prop description

[Thierry Reding]

On Sat, Sep 22, 2018 at 10:43:24PM +0300, Sergei Shtylyov wrote:
> The "compatible" property description contradicts even the example given:
> it only says that there must be a single value while the example has the
> fallback value too -- which makes much more sense. Moreover, the generic
> property value is misdocumented as being R-Car (and RZ/G1) specific...
> 
> Fixes: 382457e562bb ("pwm: renesas-tpu: Add DT support")
> Fixes: 3ba111a01822 ("dt-bindings: pwm: renesas-tpu: Document r8a774[35] 
> support")
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against Linus' repo -- the 'fixes' branch in Thierry Reding's
> 'linux-pwm.git' repo is very outdated...

Yes, that's because it is only updated if there's actually anything that
needs to be fixed during a release cycle. Please use Linus' tree or,
better yet, linux-next as your baseline when sending patches.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: rcar: Add bindings for R-Car E3 support

[Thierry Reding]

On Mon, Jul 30, 2018 at 08:49:51PM +0900, Yoshihiro Shimoda wrote:
> This patch adds bindings for R-Car E3. No driver update is needed.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH 10/61] gpio: simplify getting .drvdata

[Thierry Reding]

On Mon, Apr 30, 2018 at 11:03:25AM +0200, Linus Walleij wrote:
> On Thu, Apr 19, 2018 at 5:14 PM, Grygorii Strashko
>  wrote:
> > On 04/19/2018 09:05 AM, Wolfram Sang wrote:
> >>
> >> We should get drvdata from struct device directly. Going via
> >> platform_device is an unneeded step back and forth.
> >>
> >> Signed-off-by: Wolfram Sang 
> >> ---
> >>
> >> Build tested only. buildbot is happy. Please apply individually.
> >
> >
> > for gpio-omap.c:
> > Reviewed-by: Grygorii Strashko 
> 
> Hm I only see the responses not the actual patch, I guess I will find it
> sooner or later.
> 
> Wolfram: if I don't find it, poke me with something sharp so I get to apply 
> it.

I had the same problem, turned out this got classified as spam by Google
for some reason. Maybe try your spam folder?

Thierry


signature.asc
Description: PGP signature

Re: [PATCH 43/61] pwm: simplify getting .drvdata

[Thierry Reding]

On Thu, Apr 19, 2018 at 04:06:13PM +0200, Wolfram Sang wrote:
> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang 
> ---
> 
> Build tested only. buildbot is happy. Please apply individually.
> 
>  drivers/pwm/pwm-atmel-tcb.c | 6 ++
>  drivers/pwm/pwm-rcar.c  | 3 +--
>  2 files changed, 3 insertions(+), 6 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] drm/core: Remove drm_dev_unref() and it's uses

[Thierry Reding]

On Thu, Apr 26, 2018 at 03:58:19PM +0530, Vaishali Thakkar wrote:
> It's been a while since we introduced drm_dev{get/put} functions
> to replace reference/unreference in drm subsystem for the
> consistency purpose. So, with this patch, let's just replace
> all current use cases of drm_dev_unref() with drm_dev_put and remove
> the function itself.
> 
> Coccinelle was used for mass-patching.
> 
> Signed-off-by: Vaishali Thakkar <vthakkar1...@gmail.com>

Yes, please.

Acked-by: Thierry Reding <tred...@nvidia.com>


signature.asc
Description: PGP signature

Re: [PATCH v2 0/2] pwm: rcar: Add suspend/resume support and cleanup for runtime_pm

[Thierry Reding]

On Tue, Mar 13, 2018 at 05:18:16PM +0900, Yoshihiro Shimoda wrote:
> This patch set improves power management for Renesas PWM driver.
> 
> Changes from v1:
>  - Add Reviewed-by Geert-san in patch 1 (Thanks!).
>  - Check a condition in suspend/resume if requested or not in patch 2.
> 
> Hien Dang (1):
>   pwm: rcar: Use PM Runtime to control module clock
> 
> Yoshihiro Shimoda (1):
>   pwm: rcar: add suspend/resume support
> 
>  drivers/pwm/pwm-rcar.c | 50 
> --
>  1 file changed, 44 insertions(+), 6 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: pwm: renesas,pwm-rcar: add bindings for R-Car M3N support

[Thierry Reding]

On Fri, Mar 09, 2018 at 08:53:17PM +0900, Yoshihiro Shimoda wrote:
> This patch adds bindings for R-Car M3N. No driver update is needed.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] pwm: rcar: fix a condition to prevent mismatch value setting to duty

[Thierry Reding]

On Fri, Mar 09, 2018 at 08:24:21PM +0900, Yoshihiro Shimoda wrote:
> From: Ryo Kodama 
> 
> This patch fixes an issue that is possible to set mismatch value
> to duty for R-Car PWM if we input the following commands:
> 
>  # cd /sys/class/pwm//
>  # echo 0 > export
>  # cd pwm0
>  # echo 30 > period
>  # echo 30 > duty_cycle
>  # echo 0 > duty_cycle
>  # cat duty_cycle
>  0
>  # echo 1 > enable
>  --> Then, the actual duty_cycle is 30, not 0.
> 
> So, this patch adds a condition into rcar_pwm_config() to fix
> this issue.
> 
> Signed-off-by: Ryo Kodama 
> [shimoda: revise the commit log and add Fixes and Cc tags]
> Fixes: ed6c1476bf7f ("pwm: Add support for R-Car PWM Timer")
> Cc: Cc:  # v4.4+
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/pwm/pwm-rcar.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v4] dt-bindings: pwm: rcar: Document r8a774[35] PWM bindings

[Thierry Reding]

On Wed, Dec 20, 2017 at 11:15:43AM +, Fabrizio Castro wrote:
> This patch adds compatible strings specific to r8a774[35], no driver
> change is needed as the fallback compatible string will activate the
> right code.
> 
> Also, this patch replaces the example with a DT snippet used
> for adding PWM0 support to an r8a7743 based platform as the r8a7743 is
> now the first platform fully compatible with this driver and its PWM DT
> nodes refer to up-to-date code.
> 
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 
> Reviewed-by: Rob Herring 
> Reviewed-by: Geert Uytterhoeven 
> ---
> v3->v4:
> * The changelog now explains why the patch is replacing the example.
> 
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v3 4/5] dt-bindings: pwm: renesas-tpu: Document r8a774[35] support

[Thierry Reding]

On Tue, Dec 19, 2017 at 01:34:59PM +, Fabrizio Castro wrote:
> Document r8a774[35] specific compatible strings. No driver change is
> needed as the fallback compatible string "renesas,tpu" activates the
> right code in the driver.
> 
> Signed-off-by: Fabrizio Castro 
> Reviewed-by: Biju Das 
> Reviewed-by: Rob Herring 
> Reviewed-by: Geert Uytterhoeven 
> ---
> v2->v3:
> * No change
> 
>  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH repost2] pwm: renesas-tpu: remove support for sh7372

[Thierry Reding]

On Fri, Aug 18, 2017 at 12:09:01PM +0200, Simon Horman wrote:
> Remove support for the SH7372 (SH-Mobile AP4) from the renesas-tpu driver.
> 
> Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file")
> removes this SoC from the kernel in v4.1.
> 
> Acked-by: Geert Uytterhoeven 
> Signed-off-by: Simon Horman 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,tpu-pwm.txt | 1 -
>  drivers/pwm/pwm-renesas-tpu.c | 1 -
>  2 files changed, 2 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v4] dt-bindings: pwm: Add R-Car M3-W device tree bindings

[Thierry Reding]

On Fri, May 12, 2017 at 10:09:42AM +0200, Simon Horman wrote:
> From: Ulrich Hecht 
> 
> Add device tree bindings for the PWM controller found on R-Car M3-W SoCs.
> 
> Signed-off-by: Ulrich Hecht 
> ---
> v4
> * Broke out of lager patchset on which it appears to have no dependencies
> * Added Reviewed by tags 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2 05/13] drm: panels: Constify device node argument to of_drm_find_panel()

[Thierry Reding]

On Sat, Nov 19, 2016 at 05:28:05AM +0200, Laurent Pinchart wrote:
> The argument is never modified by the function, make it const.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/drm_panel.c | 2 +-
>  include/drm/drm_panel.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2.1 04/13] drm: Add data transmission order bus flag

[Thierry Reding]

On Wed, Jan 04, 2017 at 02:39:26AM +0200, Laurent Pinchart wrote:
> The flags indicate whether data is transmitted lsb to msb or msb to lsb
> on the bus.
> 
> The exact meaning is bus-type dependent. For instance, for LVDS buses
> the flags indicate whether the seven data bits transmitted in a clock
> pulse are sent in normal order (msb to lsb, slots 0 to 6) or reverse
> order (lsb to msb, slots 6 to 0).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
> ---
> Changes since v2:
> 
> - Rename the flag to DRM_BUS_FLAG_DATA_LSB_TO_MSB and add a
>   corresponding DRM_BUS_FLAG_DATA_MSB_TO_LSB flag.
> ---
>  include/drm/drm_connector.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a9b95246e26e..712f255577ea 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -160,6 +160,10 @@ struct drm_display_info {
>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2)
>  /* drive data on neg. edge */
>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3)
> +/* data is transmitted msb to lsb on the bus */
> +#define DRM_BUS_FLAG_DATA_MSB_TO_LSB (1<<4)
> +/* data is transmitted lsb to msb on the bus */
> +#define DRM_BUS_FLAG_DATA_LSB_TO_MSB (1<<5)

Nit: "LSB" and "MSB" because they're abbreviations. If I end up applying
this I'll probably do that myself, and I leave it up to whoever else
might apply it whether or not they want to be pedantic, so:

Reviewed-by: Thierry Reding <tred...@nvidia.com>


signature.asc
Description: PGP signature

Re: [PATCH v2 06/13] drm: panels: Add LVDS panel driver

[Thierry Reding]

On Sat, Nov 19, 2016 at 05:28:06AM +0200, Laurent Pinchart wrote:
> This driver supports LVDS panels that don't require device-specific
> handling of power supplies or control signals. It implements automatic
> backlight handling if the panel is attached to a backlight controller.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/panel/Kconfig  |  10 ++
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-lvds.c | 284 
> +
>  3 files changed, 295 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-lvds.c

The bulk of this is a duplicate of panel-simple.c and it adds all the
things on top that I've been repeatedly rejecting in the past.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Thierry Reding]

On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  .../bindings/display/panel/panel-common.txt| 91 
> ++
>  1 file changed, 91 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/panel-common.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt 
> b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> new file mode 100644
> index ..ec52c472c845
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> @@ -0,0 +1,91 @@
> +Common Properties for Display Panel
> +===
> +
> +This document defines device tree properties common to several classes of
> +display panels. It doesn't constitue a device tree binding specification by
> +itself but is meant to be referenced by device tree bindings.
> +
> +When referenced from panel device tree bindings the properties defined in 
> this
> +document are defined as follows. The panel device tree bindings are
> +responsible for defining whether each property is required or optional.
> +
> +
> +Descriptive Properties
> +--
> +
> +- width-mm,
> +- height-mm: The width-mm and height-mm specify the width and height of the
> +  physical area where images are displayed. These properties are expressed in
> +  millimeters and rounded to the closest unit.

Erm... this is already implied by the compatible string. Having this in
device tree is completely redundant.

> +- label: The label property specifies a symbolic name for the panel as a
> +  string suitable for use by humans. It typically contains a name inscribed 
> on
> +  the system (e.g. as an affixed label) or specified in the system's
> +  documentation (e.g. in the user's manual).
> +
> +  If no such name exists, and unless the property is mandatory according to
> +  device tree bindings, it shall rather be omitted than constructed of
> +  non-descriptive information. For instance an LCD panel in a system that
> +  contains a single panel shall not be labelled "LCD" if that name is not
> +  inscribed on the system or used in a descriptive fashion in system
> +  documentation.
> +
> +
> +Display Timings
> +---
> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> +  require specific display timings. The panel-timing subnode expresses those
> +  timings as specified in the timing subnode section of the display timing
> +  bindings defined in
> +  Documentation/devicetree/bindings/display/display-timing.txt.

Why? That's also implied by the compatible string. Honestly, I thought
by now we had been over this often enough...

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v3 5/6] pwm: add R-Car H3 device tree bindings

[Thierry Reding]

On Thu, Mar 31, 2016 at 01:39:15PM +0200, Ulrich Hecht wrote:
> Signed-off-by: Ulrich Hecht 
> Acked-by: Geert Uytterhoeven 
> ---
>  Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v3 1/6] pwm: rcar: Use ARCH_RENESAS

[Thierry Reding]

On Thu, Mar 31, 2016 at 01:39:11PM +0200, Ulrich Hecht wrote:
> From: Ryo Kodama 
> 
> Replace ARCH_RCAR_GEN{1,2} with ARCH_RENESAS in order to support R-Car Gen3.
> 
> Signed-off-by: Ryo Kodama 
> Signed-off-by: Harunobu Kurokawa 
> Signed-off-by: Ulrich Hecht 
> Acked-by: Geert Uytterhoeven 
> ---
>  drivers/pwm/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2] pwm: improve args checking in pwm_apply_state()

[Thierry Reding]

On Wed, Jun 22, 2016 at 01:46:48PM -0700, Brian Norris wrote:
> On Wed, Jun 22, 2016 at 10:41:14PM +0200, Boris Brezillon wrote:
> > On Wed, 22 Jun 2016 12:16:59 -0700
> > Brian Norris  wrote:
> > > Notably, you're dropping the 'if (!pwm) { }' safety checks that are part
> > > of pwm_disable() and pwm_set_polarity(). But I don't think there should
> > > be any users relying on that.
> > 
> > Indeed. I can add it back here if you prefer,
> 
> Nah, that's ok. I just had to say it anyway :)
> 
> > but honestly, PWM users
> > that are not checking the value returned by pwm_get() should be
> > considered buggy IMHO, and a NULL pointer exception is a good way to
> > make people realize they are not properly using the API :).
> 
> Seems OK.

I've applied this to my fixes branch, and I'll let it cook in linux-next
for a little while, then send it off to Linus for v4.7-rc6 next week if
no further fallout is caused by this.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v2] pwm: improve args checking in pwm_apply_state()

[Thierry Reding]

On Wed, Jun 22, 2016 at 10:04:22AM +0200, Boris Brezillon wrote:
> On Tue, 21 Jun 2016 11:37:31 -0700
> Brian Norris  wrote:
> 
> > Hi Geert,
> > 
> > On Tue, Jun 21, 2016 at 04:42:04PM +0200, Geert Uytterhoeven wrote:
> > > On Fri, May 27, 2016 at 6:45 PM, Brian Norris  
> > > wrote:  
> > > > It seems like in the process of refactoring pwm_config() to utilize the
> > > > newly-introduced pwm_apply_state() API, some args/bounds checking was
> > > > dropped.
> > > >
> > > > In particular, I noted that we are now allowing invalid period
> > > > selections. e.g.:
> > > >
> > > >   # echo 1 > /sys/class/pwm/pwmchip0/export
> > > >   # cat /sys/class/pwm/pwmchip0/pwm1/period
> > > >   100
> > > >   # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle
> > > >   [... driver may or may not reject the value, or trigger some logic 
> > > > bug ...]
> > > >
> > > > It's better to see:
> > > >
> > > >   # echo 1 > /sys/class/pwm/pwmchip0/export
> > > >   # cat /sys/class/pwm/pwmchip0/pwm1/period
> > > >   100
> > > >   # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle
> > > >   -bash: echo: write error: Invalid argument
> > > >
> > > > This patch reintroduces some bounds checks in both pwm_config() (for its
> > > > signed parameters; we don't want to convert negative values into large
> > > > unsigned values) and in pwm_apply_state() (which fix the above described
> > > > behavior, as well as other potential API misuses).
> > > >
> > > > Fixes: 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic 
> > > > updates")
> > > > Signed-off-by: Brian Norris 
> > > > ---
> > > > v2:
> > > >  * changed subject, as this covers more scope now
> > > >  * add Fixes tag, as this is a v4.7-rc regression
> > > >  * add more bounds/args checks in pwm_apply_state() and pwm_config()
> > > >
> > > >  drivers/pwm/core.c  | 3 ++-
> > > >  include/linux/pwm.h | 3 +++
> > > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > > > index dba3843c53b8..ed337a8c34ab 100644
> > > > --- a/drivers/pwm/core.c
> > > > +++ b/drivers/pwm/core.c
> > > > @@ -457,7 +457,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct 
> > > > pwm_state *state)
> > > >  {
> > > > int err;
> > > >
> > > > -   if (!pwm)
> > > > +   if (!pwm || !state || !state->period ||
> > > > +   state->duty_cycle > state->period)
> > > > return -EINVAL;  
> > > 
> > > This check breaks the LCD backlight on r8a7740/armadillo.
> > > Apparently both period and duty_cycle are zero during the first 
> > > invocation.
> > > Later, these are initialized from DT, cfr.
> > > 
> > > pwms = < 2 3 PWM_POLARITY_INVERTED>;
> > > 
> > > in arch/arm/boot/dts/r8a7740-armadillo800eva.dts.  
> > 
> > Hmm, this isn't super obvious how to best fix. On one hand, the
> > pwm_config() API used to reject period<=0, but on the other hand, I
> > think its replacement (pwm_apply_state()) is getting used in more places
> > than it used to be, and not all of them are really handling the "atomic
> > update" concept yet. Seems like a product of Boris's multi-phase attempt
> > to convert the PWM APIs to support atomic updates -- and many users
> > haven't really converted yet.
> > 
> > > With added debug printing, the difference between failure and success is:
> > > 
> > >  renesas-tpu-pwm e660.pwm: TPU PWM -1 registered
> > >  tpu_pwm_request:223
> > >  pwm_apply_state:460: pwm backlight/2: period 0, duty_cycle 0
> > > +Ignoring failure
> > > +pwm_apply_state:479: polarity 0 -> 1
> > > +tpu_pwm_set_polarity:343
> > > +pwm_apply_state:502: period 0 -> 0
> > > +pwm_apply_state:503: duty_cycle 0 -> 0
> > > +pwm_apply_state:516: enabled 0 -> 0
> > >  pwm_config:238: pwm backlight/2: duty_ns 3, period_ns 3
> > >  pwm_apply_state:460: pwm backlight/2: period 3, duty_cycle 3
> > > -pwm_apply_state:479: polarity 0 -> 0
> > > +pwm_apply_state:479: polarity 1 -> 1
> > >  pwm_apply_state:502: period 0 -> 3
> > >  pwm_apply_state:503: duty_cycle 0 -> 3
> > >  tpu_pwm_config:267
> > >  pwm_apply_state:516: enabled 0 -> 0
> > >  pwm_apply_state:460: pwm backlight/2: period 3, duty_cycle 3
> > > -pwm_apply_state:479: polarity 0 -> 0
> > > +pwm_apply_state:479: polarity 1 -> 1
> > >  pwm_apply_state:502: period 3 -> 3
> > >  pwm_apply_state:503: duty_cycle 3 -> 3
> > >  pwm_apply_state:516: enabled 0 -> 1
> > >  tpu_pwm_enable:354  
> > 
> > I'm not sure I 100% understand this debug log, but I think maybe the
> > problem is in pwm_apply_args(), which calls pwm_disable() and
> > pwm_set_polarity() sequentially, without ever configuring a period? What
> > if pwm_apply_args() were to configure the period for us?
> > 
> > Boris, any thoughts?
> > 
> 
> I had second thoughts and I think you're right: pwm_apply_args()
> should set the pargs.period period for us.
> 
> Here 

Re: [PATCH] pwm: sysfs: get return value from pwm_apply_state()

[Thierry Reding]

On Wed, Jun 08, 2016 at 10:58:23AM +0900, Yoshihiro Shimoda wrote:
> From: Ryo Kodama 
> 
> This patch adds to check the return value from pwm_apply_state()
> used in enable_store(). The error of enable_store() doesn't work
> if the return value doesn't received.
> 
> Signed-off-by: Ryo Kodama 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/pwm/sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH] pwm: rcar: Use ARCH_RENESAS

[Thierry Reding]

On Thu, Feb 25, 2016 at 10:03:23AM +0900, Simon Horman wrote:
> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
> 
> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
> appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.
> 
> Signed-off-by: Simon Horman 
> ---
>  drivers/pwm/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature