From: Alberto Panizzo <alberto@amarulasolutions.com>
To: Heiko Stuebner <heiko@sntech.de>
Cc: Anthony Brandon <anthony@amarulasolutions.com>,
linux-rockchip@lists.infradead.org, mturquette@baylibre.com,
sboyd@kernel.org, linux-clk@vger.kernel.org
Subject: Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
Date: Mon, 9 Jul 2018 18:16:21 +0200 [thread overview]
Message-ID: <20180709161620.GA12454@change> (raw)
In-Reply-To: <4069085.blmd9nAIm0@phil>
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
WARNING: multiple messages have this Message-ID (diff)
From: Alberto Panizzo <alberto-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
To: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Anthony Brandon
<anthony-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
Subject: Re: [PATCH 2/2] clk: rockchip: rk3399: fix returning correct value on clk_i2sout get_rate [with GET_RATE_NOCACHE]
Date: Mon, 9 Jul 2018 18:16:21 +0200 [thread overview]
Message-ID: <20180709161620.GA12454@change> (raw)
In-Reply-To: <4069085.blmd9nAIm0@phil>
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
next prev parent reply other threads:[~2018-07-09 16:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180709161620.GA12454@change \
--to=alberto@amarulasolutions.com \
--cc=anthony@amarulasolutions.com \
--cc=heiko@sntech.de \
--cc=linux-clk@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mturquette@baylibre.com \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.