Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Cc Carnaghi]

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

Mr

[-- Attachment #2: Type: text/html, Size: 25 bytes --]

Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Cc Carnaghi]

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



[-- Attachment #2: Type: text/html, Size: 23 bytes --]

[PATCH 0/8] Fix struct clk pointer comparing

[Shawn Guo]

On the first day back from Chinese new year holiday, I got a regression
report from rmk, saying Ethernet stops working on HimmingBoard with
v4.0-rc1.

I read through the thread [1] and found a couple of i.MX audio drivers
are also affected per Stephen's Coccinelle report.  That's why I came up
with this series based on Quentin's structclk.cocci, Stephen's result
and Mike's input (thanks all).

The first two patches are run-time tested and all others are
compile-tested only.

[1] http://thread.gmane.org/gmane.linux.kernel/1872604/focus=1880862

Shawn Guo (8):
  clk: add helper function clk_is_match()
  ARM: imx: fix struct clk pointer comparing
  drm: armada: fix struct clk pointer comparing
  pwm: atmel-hlcdc: fix struct clk pointer comparing
  serial: samsung: fix struct clk pointer comparing
  ASoC: fsl_esai: fix struct clk pointer comparing
  ASoC: fsl_spdif: fix struct clk pointer comparing
  ASoC: kirkwood: fix struct clk pointer comparing

 arch/arm/mach-imx/mach-imx6q.c      |  5 +++--
 drivers/clk/clk.c                   |  6 ++++++
 drivers/gpu/drm/armada/armada_510.c |  2 +-
 drivers/pwm/pwm-atmel-hlcdc.c       |  2 +-
 drivers/tty/serial/samsung.c        |  2 +-
 include/linux/clk.h                 | 16 ++++++++++++++++
 sound/soc/fsl/fsl_esai.c            |  2 +-
 sound/soc/fsl/fsl_spdif.c           |  4 ++--
 sound/soc/kirkwood/kirkwood-i2s.c   |  2 +-
 9 files changed, 32 insertions(+), 9 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Shawn Guo]

Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
instances"), clk API users can no longer check if two struct clk
pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
simply comparing two pointers.  That's because with the per-user clk
change, a brand new struct clk is created whenever clients try to look
up the clock by calling clk_get() or sister functions like clk_get_sys()
and of_clk_get().  This changes the original behavior where the struct
clk is only created for once when clock driver registers the clock to
CCF in the first place.  The net change here is before commit
035a61c314eb the struct clk pointer is unique for given hardware
clock, while after the commit the pointers returned by clk lookup calls
become different for the same hardware clock.

That said, the struct clk pointer comparing in the code doesn't work any
more.  Call helper function clk_is_match() instead to fix the problem.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 522f7075bb1a..36475949b829 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
 
 	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
 
-	if (new_clk != chip->cur_clk) {
+	if (!clk_is_match(new_clk, chip->cur_clk)) {
 		u32 gencfg = 0;
 		int ret;
 
-- 
1.9.1

Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Nicolas Ferre]

Le 25/02/2015 15:53, Shawn Guo a écrit :
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
> 
> That said, the struct clk pointer comparing in the code doesn't work any
> more.  Call helper function clk_is_match() instead to fix the problem.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I agree with the fix whichever name is chosen for the function in an
future version of this series. So you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Maybe Boris can double check...

> ---
>  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> index 522f7075bb1a..36475949b829 100644
> --- a/drivers/pwm/pwm-atmel-hlcdc.c
> +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> @@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
>  
>  	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
>  
> -	if (new_clk != chip->cur_clk) {
> +	if (!clk_is_match(new_clk, chip->cur_clk)) {
>  		u32 gencfg = 0;
>  		int ret;
>  
> 


-- 
Nicolas Ferre

Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Boris Brezillon]

Nicolas, Shawn,

On Thu, 26 Feb 2015 10:22:50 +0100
Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> Le 25/02/2015 15:53, Shawn Guo a écrit :
> > Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> > instances"), clk API users can no longer check if two struct clk
> > pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> > simply comparing two pointers.  That's because with the per-user clk
> > change, a brand new struct clk is created whenever clients try to look
> > up the clock by calling clk_get() or sister functions like clk_get_sys()
> > and of_clk_get().  This changes the original behavior where the struct
> > clk is only created for once when clock driver registers the clock to
> > CCF in the first place.  The net change here is before commit
> > 035a61c314eb the struct clk pointer is unique for given hardware
> > clock, while after the commit the pointers returned by clk lookup calls
> > become different for the same hardware clock.
> > 
> > That said, the struct clk pointer comparing in the code doesn't work any
> > more.  Call helper function clk_is_match() instead to fix the problem.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> I agree with the fix whichever name is chosen for the function in an
> future version of this series. So you can add my:
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Maybe Boris can double check...

Looks good to me.
Thanks for fixing that.

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> 
> > ---
> >  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
> > index 522f7075bb1a..36475949b829 100644
> > --- a/drivers/pwm/pwm-atmel-hlcdc.c
> > +++ b/drivers/pwm/pwm-atmel-hlcdc.c
> > @@ -97,7 +97,7 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
> >  
> >  	pwmcfg = ATMEL_HLCDC_PWMPS(pres);
> >  
> > -	if (new_clk != chip->cur_clk) {
> > +	if (!clk_is_match(new_clk, chip->cur_clk)) {
> >  		u32 gencfg = 0;
> >  		int ret;
> >  
> > 
> 
> 



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 4/8] pwm: atmel-hlcdc: fix struct clk pointer comparing

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1268 bytes --]

On Wed, Feb 25, 2015 at 10:53:34PM +0800, Shawn Guo wrote:
> Since commit 035a61c314eb ("clk: Make clk API return per-user struct clk
> instances"), clk API users can no longer check if two struct clk
> pointers are pointing to the same hardware clock, i.e. struct clk_hw, by
> simply comparing two pointers.  That's because with the per-user clk
> change, a brand new struct clk is created whenever clients try to look
> up the clock by calling clk_get() or sister functions like clk_get_sys()
> and of_clk_get().  This changes the original behavior where the struct
> clk is only created for once when clock driver registers the clock to
> CCF in the first place.  The net change here is before commit
> 035a61c314eb the struct clk pointer is unique for given hardware
> clock, while after the commit the pointers returned by clk lookup calls
> become different for the same hardware clock.
> 
> That said, the struct clk pointer comparing in the code doesn't work any
> more.  Call helper function clk_is_match() instead to fix the problem.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Thierry Reding <thierry.reding@gmail.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel