All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: rockchip: rk3399: fix clk_i2sout parent selection bits
@ 2018-07-06 13:18 Anthony Brandon
       [not found] ` <1530883132-17678-1-git-send-email-anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Anthony Brandon @ 2018-07-06 13:18 UTC (permalink / raw)
  To: heiko-4mtYJXux2i+zQB+pC5nmwQ,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Alberto Panizzo, Anthony Brandon

From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>

Register, shift and mask were wrong according to datasheet.

Signed-off-by: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Anthony Brandon <anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 drivers/clk/rockchip/clk-rk3399.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index bca10d6..2a8634a 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -631,7 +631,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
 			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
 	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
-			RK3399_CLKSEL_CON(30), 8, 2, MFLAGS,
+			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
 			RK3399_CLKGATE_CON(8), 12, GFLAGS),
 
 	/* uart */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate
       [not found] ` <1530883132-17678-1-git-send-email-anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
@ 2018-07-06 13:18   ` Anthony Brandon
  2018-07-08  9:32       ` Heiko Stuebner
  2018-07-07 22:36   ` [PATCH 1/2] clk: rockchip: rk3399: fix clk_i2sout parent selection bits Heiko Stuebner
  1 sibling, 1 reply; 10+ messages in thread
From: Anthony Brandon @ 2018-07-06 13:18 UTC (permalink / raw)
  To: heiko-4mtYJXux2i+zQB+pC5nmwQ,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Alberto Panizzo, Anthony Brandon

From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>

clk_i2sout can be used as codec mclk.
On simple audio card clk_i2sout is just enabled/disabled while the rate
is decided on parent clock by i2s driver.
Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
clk_i2sout would return incorrect values after clk_i2sout's parents
update.

Signed-off-by: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Anthony Brandon <anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 drivers/clk/rockchip/clk-rk3399.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 2a8634a..6073479 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -630,7 +630,8 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 
 	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
 			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
-	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
+	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
+			CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
 			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
 			RK3399_CLKGATE_CON(8), 12, GFLAGS),
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] clk: rockchip: rk3399: fix clk_i2sout parent selection bits
       [not found] ` <1530883132-17678-1-git-send-email-anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
  2018-07-06 13:18   ` [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate Anthony Brandon
@ 2018-07-07 22:36   ` Heiko Stuebner
  1 sibling, 0 replies; 10+ messages in thread
From: Heiko Stuebner @ 2018-07-07 22:36 UTC (permalink / raw)
  To: Anthony Brandon
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alberto Panizzo

Am Freitag, 6. Juli 2018, 15:18:51 CEST schrieb Anthony Brandon:
> From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> 
> Register, shift and mask were wrong according to datasheet.
> 
> Signed-off-by: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Anthony Brandon <anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>

applied for 4.19, after double-checking against the TRM

Thanks for catching this
Heiko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
@ 2018-07-08  9:32       ` Heiko Stuebner
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Stuebner @ 2018-07-08  9:32 UTC (permalink / raw)
  To: Anthony Brandon
  Cc: linux-rockchip, Alberto Panizzo, mturquette, sboyd, linux-clk

Hi,

Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> From: Alberto Panizzo <alberto@amarulasolutions.com>
> 
> clk_i2sout can be used as codec mclk.
> On simple audio card clk_i2sout is just enabled/disabled while the rate
> is decided on parent clock by i2s driver.
> Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> clk_i2sout would return incorrect values after clk_i2sout's parents
> update.

Can you elaborate a bit more on the issue you see please?
Because as far as I remember the clock-framework should already
update child-clocks when the rate of their parent changed.

So even the cached rate should be correct after the parent changes,
so I don't really understand in what case you would get a wrong rate.


Heiko


> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> Signed-off-by: Anthony Brandon <anthony@amarulasolutions.com>
> ---
>  drivers/clk/rockchip/clk-rk3399.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 2a8634a..6073479 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -630,7 +630,8 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
>  
>  	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
>  			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
> -	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
> +	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> +			CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
>  			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
>  			RK3399_CLKGATE_CON(8), 12, GFLAGS),
>  
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
@ 2018-07-08  9:32       ` Heiko Stuebner
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Stuebner @ 2018-07-08  9:32 UTC (permalink / raw)
  To: Anthony Brandon
  Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Alberto Panizzo

Hi,

Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> 
> clk_i2sout can be used as codec mclk.
> On simple audio card clk_i2sout is just enabled/disabled while the rate
> is decided on parent clock by i2s driver.
> Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> clk_i2sout would return incorrect values after clk_i2sout's parents
> update.

Can you elaborate a bit more on the issue you see please?
Because as far as I remember the clock-framework should already
update child-clocks when the rate of their parent changed.

So even the cached rate should be correct after the parent changes,
so I don't really understand in what case you would get a wrong rate.


Heiko


> Signed-off-by: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Anthony Brandon <anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> ---
>  drivers/clk/rockchip/clk-rk3399.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 2a8634a..6073479 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -630,7 +630,8 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
>  
>  	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
>  			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
> -	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
> +	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> +			CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
>  			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
>  			RK3399_CLKGATE_CON(8), 12, GFLAGS),
>  
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
  2018-07-08  9:32       ` Heiko Stuebner
  (?)
@ 2018-07-09 16:03       ` Alberto Panizzo
  -1 siblings, 0 replies; 10+ messages in thread
From: Alberto Panizzo @ 2018-07-09 16:03 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Anthony Brandon, linux-rockchip, mturquette, sboyd, linux-clk

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

Hi Heiko,

On Sun, Jul 8, 2018 at 11:32 AM, Heiko Stuebner <heiko@sntech.de> wrote:

> Hi,
>
> Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> > From: Alberto Panizzo <alberto@amarulasolutions.com>
> >
> > clk_i2sout can be used as codec mclk.
> > On simple audio card clk_i2sout is just enabled/disabled while the rate
> > is decided on parent clock by i2s driver.
> > Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> > clk_i2sout would return incorrect values after clk_i2sout's parents
> > update.
>
> Can you elaborate a bit more on the issue you see please?
> Because as far as I remember the clock-framework should already
> update child-clocks when the rate of their parent changed.
>
> So even the cached rate should be correct after the parent changes,
> so I don't really understand in what case you would get a wrong rate.
>
>
You are right, I'm sorry this patch were coming from days of long test and
checks
to understand why clk_get_rate were returning 0 while parents clocks were
set.
(And working with a codec which does not offer deterministic results)

Especially, after having found and fixed the bits misconfigurations, a
clk_get_rate
on clk_i2sout before the first clk_i2s0 set_rate is returning 0.

But clk_i2s0_mux results unparented before first clk_i2s0 set_rate and with
or without
NOCACHE, until a set_rate is called on clk_i2s0, a get_rate on clk_i2sout
will return 0.

This does not prevent good behavior after first _hw_params() so please,
drop this patch.
But patch 1/2 is necessary, please apply.

Thanks,

Alberto Panizzo
--
Amarula Solutions SRL                     Via le Canevare 30 31100 Treviso
Italy
Amarula Solutions BV           Cruquiuskade 47 Amsterdam 1018 AM The
Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211
www.amarulasolutions.com



>
>
> > Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> > Signed-off-by: Anthony Brandon <anthony@amarulasolutions.com>
> > ---
> >  drivers/clk/rockchip/clk-rk3399.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/rockchip/clk-rk3399.c
> b/drivers/clk/rockchip/clk-rk3399.c
> > index 2a8634a..6073479 100644
> > --- a/drivers/clk/rockchip/clk-rk3399.c
> > +++ b/drivers/clk/rockchip/clk-rk3399.c
> > @@ -630,7 +630,8 @@ static struct rockchip_clk_branch
> rk3399_clk_branches[] __initdata = {
> >
> >       MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
> >                       RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
> > -     COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> CLK_SET_RATE_PARENT,
> > +     COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> > +                     CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> >                       RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
> >                       RK3399_CLKGATE_CON(8), 12, GFLAGS),
> >
> >
>
>
>
>
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
  2018-07-08  9:32       ` Heiko Stuebner
@ 2018-07-09 16:16         ` Alberto Panizzo
  -1 siblings, 0 replies; 10+ messages in thread
From: Alberto Panizzo @ 2018-07-09 16:16 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Anthony Brandon, linux-rockchip, mturquette, sboyd, linux-clk

Hi Heiko,
On Sun, Jul 08, 2018 at 11:32:19AM +0200, Heiko Stuebner wrote:
> Hi,
> 
> Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> > From: Alberto Panizzo <alberto@amarulasolutions.com>
> > 
> > clk_i2sout can be used as codec mclk.
> > On simple audio card clk_i2sout is just enabled/disabled while the rate
> > is decided on parent clock by i2s driver.
> > Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> > clk_i2sout would return incorrect values after clk_i2sout's parents
> > update.
> 
> Can you elaborate a bit more on the issue you see please?
> Because as far as I remember the clock-framework should already
> update child-clocks when the rate of their parent changed.
> 
> So even the cached rate should be correct after the parent changes,
> so I don't really understand in what case you would get a wrong rate.
> 

You are right, I'm sorry this patch were coming from days of long test and
checks to understand why clk_get_rate were returning 0 while parents clocks
were set.
(And working with a codec which does not offer deterministic results)

Especially, after having found and fixed the bits misconfigurations, a
clk_get_rate on clk_i2sout before the first clk_i2s0 set_rate is
returning 0.

But clk_i2s0_mux results unparented before first clk_i2s0 set_rate and with
or without NOCACHE, until a set_rate is called on clk_i2s0, a get_rate on
clk_i2sout will return 0.

This does not prevent good behavior after first _hw_params()
so please, drop this patch.

But patch 1/2 is necessary, please apply.

Thanks,

Alberto Panizzo
--
Amarula Solutions SRL                     Via le Canevare 30 31100 Treviso Italy
Amarula Solutions BV           Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211             www.amarulasolutions.com
 
> 
> 
> > Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> > Signed-off-by: Anthony Brandon <anthony@amarulasolutions.com>
> > ---
> >  drivers/clk/rockchip/clk-rk3399.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> > index 2a8634a..6073479 100644
> > --- a/drivers/clk/rockchip/clk-rk3399.c
> > +++ b/drivers/clk/rockchip/clk-rk3399.c
> > @@ -630,7 +630,8 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
> >  
> >  	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
> >  			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
> > -	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
> > +	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> > +			CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> >  			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
> >  			RK3399_CLKGATE_CON(8), 12, GFLAGS),
> >  
> > 
> 
> 
> 
> 

-- 
Amarula Solutions SRL                     Via le Canevare 30 31100 Treviso Italy
Amarula Solutions BV           Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211             www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
@ 2018-07-09 16:16         ` Alberto Panizzo
  0 siblings, 0 replies; 10+ messages in thread
From: Alberto Panizzo @ 2018-07-09 16:16 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Anthony Brandon

Hi Heiko,
On Sun, Jul 08, 2018 at 11:32:19AM +0200, Heiko Stuebner wrote:
> Hi,
> 
> Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> > From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > 
> > clk_i2sout can be used as codec mclk.
> > On simple audio card clk_i2sout is just enabled/disabled while the rate
> > is decided on parent clock by i2s driver.
> > Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> > clk_i2sout would return incorrect values after clk_i2sout's parents
> > update.
> 
> Can you elaborate a bit more on the issue you see please?
> Because as far as I remember the clock-framework should already
> update child-clocks when the rate of their parent changed.
> 
> So even the cached rate should be correct after the parent changes,
> so I don't really understand in what case you would get a wrong rate.
> 

You are right, I'm sorry this patch were coming from days of long test and
checks to understand why clk_get_rate were returning 0 while parents clocks
were set.
(And working with a codec which does not offer deterministic results)

Especially, after having found and fixed the bits misconfigurations, a
clk_get_rate on clk_i2sout before the first clk_i2s0 set_rate is
returning 0.

But clk_i2s0_mux results unparented before first clk_i2s0 set_rate and with
or without NOCACHE, until a set_rate is called on clk_i2s0, a get_rate on
clk_i2sout will return 0.

This does not prevent good behavior after first _hw_params()
so please, drop this patch.

But patch 1/2 is necessary, please apply.

Thanks,

Alberto Panizzo
--
Amarula Solutions SRL                     Via le Canevare 30 31100 Treviso Italy
Amarula Solutions BV           Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211             www.amarulasolutions.com
 
> 
> 
> > Signed-off-by: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > Signed-off-by: Anthony Brandon <anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > ---
> >  drivers/clk/rockchip/clk-rk3399.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> > index 2a8634a..6073479 100644
> > --- a/drivers/clk/rockchip/clk-rk3399.c
> > +++ b/drivers/clk/rockchip/clk-rk3399.c
> > @@ -630,7 +630,8 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
> >  
> >  	MUX(0, "clk_i2sout_src", mux_i2sch_p, CLK_SET_RATE_PARENT,
> >  			RK3399_CLKSEL_CON(31), 0, 2, MFLAGS),
> > -	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p, CLK_SET_RATE_PARENT,
> > +	COMPOSITE_NODIV(SCLK_I2S_8CH_OUT, "clk_i2sout", mux_i2sout_p,
> > +			CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> >  			RK3399_CLKSEL_CON(31), 2, 1, MFLAGS,
> >  			RK3399_CLKGATE_CON(8), 12, GFLAGS),
> >  
> > 
> 
> 
> 
> 

-- 
Amarula Solutions SRL                     Via le Canevare 30 31100 Treviso Italy
Amarula Solutions BV           Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211             www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
  2018-07-09 16:16         ` Alberto Panizzo
@ 2018-07-09 17:37           ` Heiko Stübner
  -1 siblings, 0 replies; 10+ messages in thread
From: Heiko Stübner @ 2018-07-09 17:37 UTC (permalink / raw)
  To: Alberto Panizzo
  Cc: Anthony Brandon, linux-rockchip, mturquette, sboyd, linux-clk

Am Montag, 9. Juli 2018, 18:16:21 CEST schrieb Alberto Panizzo:
> Hi Heiko,
> 
> On Sun, Jul 08, 2018 at 11:32:19AM +0200, Heiko Stuebner wrote:
> > Hi,
> > 
> > Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> > > From: Alberto Panizzo <alberto@amarulasolutions.com>
> > > 
> > > clk_i2sout can be used as codec mclk.
> > > On simple audio card clk_i2sout is just enabled/disabled while the rate
> > > is decided on parent clock by i2s driver.
> > > Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> > > clk_i2sout would return incorrect values after clk_i2sout's parents
> > > update.
> > 
> > Can you elaborate a bit more on the issue you see please?
> > Because as far as I remember the clock-framework should already
> > update child-clocks when the rate of their parent changed.
> > 
> > So even the cached rate should be correct after the parent changes,
> > so I don't really understand in what case you would get a wrong rate.
> 
> You are right, I'm sorry this patch were coming from days of long test and
> checks to understand why clk_get_rate were returning 0 while parents clocks
> were set.
> (And working with a codec which does not offer deterministic results)
> 
> Especially, after having found and fixed the bits misconfigurations, a
> clk_get_rate on clk_i2sout before the first clk_i2s0 set_rate is
> returning 0.
> 
> But clk_i2s0_mux results unparented before first clk_i2s0 set_rate and with
> or without NOCACHE, until a set_rate is called on clk_i2s0, a get_rate on
> clk_i2sout will return 0.
> 
> This does not prevent good behavior after first _hw_params()
> so please, drop this patch.

great to hear that :-)

> But patch 1/2 is necessary, please apply.

already done yesterday.


Heiko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
@ 2018-07-09 17:37           ` Heiko Stübner
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Stübner @ 2018-07-09 17:37 UTC (permalink / raw)
  To: Alberto Panizzo
  Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Anthony Brandon

Am Montag, 9. Juli 2018, 18:16:21 CEST schrieb Alberto Panizzo:
> Hi Heiko,
> 
> On Sun, Jul 08, 2018 at 11:32:19AM +0200, Heiko Stuebner wrote:
> > Hi,
> > 
> > Am Freitag, 6. Juli 2018, 15:18:52 CEST schrieb Anthony Brandon:
> > > From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > > 
> > > clk_i2sout can be used as codec mclk.
> > > On simple audio card clk_i2sout is just enabled/disabled while the rate
> > > is decided on parent clock by i2s driver.
> > > Without setting CLK_GET_RATE_NOCACHE flag, the get_rate function on
> > > clk_i2sout would return incorrect values after clk_i2sout's parents
> > > update.
> > 
> > Can you elaborate a bit more on the issue you see please?
> > Because as far as I remember the clock-framework should already
> > update child-clocks when the rate of their parent changed.
> > 
> > So even the cached rate should be correct after the parent changes,
> > so I don't really understand in what case you would get a wrong rate.
> 
> You are right, I'm sorry this patch were coming from days of long test and
> checks to understand why clk_get_rate were returning 0 while parents clocks
> were set.
> (And working with a codec which does not offer deterministic results)
> 
> Especially, after having found and fixed the bits misconfigurations, a
> clk_get_rate on clk_i2sout before the first clk_i2s0 set_rate is
> returning 0.
> 
> But clk_i2s0_mux results unparented before first clk_i2s0 set_rate and with
> or without NOCACHE, until a set_rate is called on clk_i2s0, a get_rate on
> clk_i2sout will return 0.
> 
> This does not prevent good behavior after first _hw_params()
> so please, drop this patch.

great to hear that :-)

> But patch 1/2 is necessary, please apply.

already done yesterday.


Heiko

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-07-09 17:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-06 13:18 [PATCH 1/2] clk: rockchip: rk3399: fix clk_i2sout parent selection bits Anthony Brandon
     [not found] ` <1530883132-17678-1-git-send-email-anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
2018-07-06 13:18   ` [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate Anthony Brandon
2018-07-08  9:32     ` [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE] Heiko Stuebner
2018-07-08  9:32       ` Heiko Stuebner
2018-07-09 16:03       ` Alberto Panizzo
2018-07-09 16:16       ` Alberto Panizzo
2018-07-09 16:16         ` Alberto Panizzo
2018-07-09 17:37         ` Heiko Stübner
2018-07-09 17:37           ` Heiko Stübner
2018-07-07 22:36   ` [PATCH 1/2] clk: rockchip: rk3399: fix clk_i2sout parent selection bits Heiko Stuebner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.