From: "Heiko Stübner" <heiko@sntech.de>
To: Jianqun Xu <jay.xu@rock-chips.com>
Cc: Mark Brown <broonie@kernel.org>,
Sonny Rao <sonnyrao@chromium.org>,
Caesar Wang <wxt@rock-chips.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
leozwang@google.com,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Kees Cook <keescook@google.com>
Subject: Re: [PATCH v3 3/9] ASoC: rockchip: i2s: add support for grabbing output clock to codec
Date: Tue, 26 Jan 2016 10:52:25 +0100 [thread overview]
Message-ID: <2073307.S3XLZp3hP0@diego> (raw)
In-Reply-To: <56A57733.8020200@rock-chips.com>
Hi Jay,
Am Montag, 25. Januar 2016, 09:15:31 schrieb Jianqun Xu:
> 在 23/01/2016 01:18, Mark Brown 写道:
> > On Fri, Jan 15, 2016 at 01:48:04PM -0800, Sonny Rao wrote:
> >> On Fri, Jan 15, 2016 at 9:46 AM, Mark Brown <broonie@kernel.org> wrote:
> >>> If the I2S block is providing a clock to the CODEC then that's what the
> >>> software should do so that the CODEC can gate and ungate the clock as
> >>> required. This patch has the I2S block using a clock, not providing
> >>> one.
> >>>
> >> From my read of the clock diagram for RK3288 there is a single clock
> >>
> >> signal (labeled "clk_i2s0") that comes out of a fractional divider,
> >> and it is split such that one path gets sent to the I2S block and the
> >> second path is sent to a mux after which that signal is sent to an
> >> external pin that goes to the codec.
> >>
> >> There are separate clock gates for the two paths: one for the I2S
> >> block and one after that mux before the external pin.
> >>
> >> I'm not sure if it's being modeled that way in the Linux code or not,
> >> but at least physically I don't think this clock signal actually goes
> >> through the I2S block before being sent to the codec.
> >
> > That's not really the issue here, the issue is that it's not the I2S
> > controller that is consuming the clock so it should not be the I2S
> > controller driver that ensures that the clock is enabled. The driver
> > that manages the clock should be the one that uses it, like I say this
> > means you should add the code to enable the clock to the CODEC driver if
> > the CODEC driver needs the clock enabled.
>
> Agree, now we almost use the simple-card for the CODEC driver, so I
> think we should enable the mclk(i2s-outclk) in the simple-card driver,
> is it ?
>
> I found a subnode property from simple-card document:
> - mclk-fs : Multiplication factor between
> stream
> rate and codec mclk, applied
> only for
> the dai-link.
> But the property responsible to the factor, not care if the mclk source
> clock is enabled or not. So does the simple-card driver can add support
> to enable/disable mclk ?
The mclk-input is part of the codec I'd think. So you'd want the clocks-
property in the i2c entry describing the codec itself and implement the clk
operations in the codec driver as well.
See codec-drivers for da7213, da7218,max98090 and many more for reference.
Heiko
> >> Does that help clarify?
> >
> > The problem here isn't a lack of clarity in the situation.
WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 3/9] ASoC: rockchip: i2s: add support for grabbing output clock to codec
Date: Tue, 26 Jan 2016 10:52:25 +0100 [thread overview]
Message-ID: <2073307.S3XLZp3hP0@diego> (raw)
In-Reply-To: <56A57733.8020200@rock-chips.com>
Hi Jay,
Am Montag, 25. Januar 2016, 09:15:31 schrieb Jianqun Xu:
> ? 23/01/2016 01:18, Mark Brown ??:
> > On Fri, Jan 15, 2016 at 01:48:04PM -0800, Sonny Rao wrote:
> >> On Fri, Jan 15, 2016 at 9:46 AM, Mark Brown <broonie@kernel.org> wrote:
> >>> If the I2S block is providing a clock to the CODEC then that's what the
> >>> software should do so that the CODEC can gate and ungate the clock as
> >>> required. This patch has the I2S block using a clock, not providing
> >>> one.
> >>>
> >> From my read of the clock diagram for RK3288 there is a single clock
> >>
> >> signal (labeled "clk_i2s0") that comes out of a fractional divider,
> >> and it is split such that one path gets sent to the I2S block and the
> >> second path is sent to a mux after which that signal is sent to an
> >> external pin that goes to the codec.
> >>
> >> There are separate clock gates for the two paths: one for the I2S
> >> block and one after that mux before the external pin.
> >>
> >> I'm not sure if it's being modeled that way in the Linux code or not,
> >> but at least physically I don't think this clock signal actually goes
> >> through the I2S block before being sent to the codec.
> >
> > That's not really the issue here, the issue is that it's not the I2S
> > controller that is consuming the clock so it should not be the I2S
> > controller driver that ensures that the clock is enabled. The driver
> > that manages the clock should be the one that uses it, like I say this
> > means you should add the code to enable the clock to the CODEC driver if
> > the CODEC driver needs the clock enabled.
>
> Agree, now we almost use the simple-card for the CODEC driver, so I
> think we should enable the mclk(i2s-outclk) in the simple-card driver,
> is it ?
>
> I found a subnode property from simple-card document:
> - mclk-fs : Multiplication factor between
> stream
> rate and codec mclk, applied
> only for
> the dai-link.
> But the property responsible to the factor, not care if the mclk source
> clock is enabled or not. So does the simple-card driver can add support
> to enable/disable mclk ?
The mclk-input is part of the codec I'd think. So you'd want the clocks-
property in the i2c entry describing the codec itself and implement the clk
operations in the codec driver as well.
See codec-drivers for da7213, da7218,max98090 and many more for reference.
Heiko
> >> Does that help clarify?
> >
> > The problem here isn't a lack of clarity in the situation.
next prev parent reply other threads:[~2016-01-26 9:52 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-15 13:49 [PATCH v3 0/9] Add the family patches to support for kylin board Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 13:49 ` [PATCH v3 1/9] ARM: dts: rockchip: add hdmi/vop device node for rk3036 Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 13:49 ` [PATCH v3 3/9] ASoC: rockchip: i2s: add support for grabbing output clock to codec Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 17:46 ` Mark Brown
2016-01-15 17:46 ` Mark Brown
[not found] ` <20160115174623.GZ6588-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-01-15 21:48 ` Sonny Rao
2016-01-15 21:48 ` Sonny Rao
2016-01-15 21:48 ` Sonny Rao
2016-01-22 17:18 ` Mark Brown
2016-01-22 17:18 ` Mark Brown
[not found] ` <20160122171815.GD6588-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2016-01-25 1:15 ` Jianqun Xu
2016-01-25 1:15 ` Jianqun Xu
2016-01-25 1:15 ` Jianqun Xu
2016-01-26 9:52 ` Heiko Stübner [this message]
2016-01-26 9:52 ` Heiko Stübner
2016-01-27 12:47 ` Mark Brown
2016-01-27 12:47 ` Mark Brown
2016-01-15 13:49 ` [PATCH v3 4/9] ARM: dts: rockchip: enable the uart0 for kylin board Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-17 19:23 ` Heiko Stuebner
2016-01-17 19:23 ` Heiko Stuebner
2016-01-15 13:49 ` [PATCH v3 5/9] ARM: dts: rockchip: enable the high speed on sdio " Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-17 19:26 ` Heiko Stuebner
2016-01-17 19:26 ` Heiko Stuebner
2016-01-15 13:49 ` [PATCH v3 6/9] ARM: dts: rockchip: add the sdio power sequence " Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-17 19:31 ` Heiko Stuebner
2016-01-17 19:31 ` Heiko Stuebner
[not found] ` <1452865796-23527-1-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-01-15 13:49 ` [PATCH v3 2/9] ARM: dts: rockchip: add i2s_clk_out to list of clocks used on rk3036 i2s Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 13:49 ` [PATCH v3 7/9] ARM: dts: rockchip: add the sdmmc for kylin board Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-17 19:37 ` Heiko Stuebner
2016-01-17 19:37 ` Heiko Stuebner
2016-01-15 13:49 ` [PATCH v3 8/9] ARM: dts: rockchip: add reboot-mode node for rk3036 SoCs Caesar Wang
2016-01-15 13:49 ` Caesar Wang
[not found] ` <1452865796-23527-9-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-01-17 19:18 ` Heiko Stuebner
2016-01-17 19:18 ` Heiko Stuebner
2016-01-17 19:18 ` Heiko Stuebner
2016-01-18 1:11 ` Andy Yan
2016-01-18 1:11 ` Andy Yan
2016-01-18 1:11 ` Andy Yan
2016-01-15 13:49 ` [PATCH v3 9/9] ARM: dts: rockchip: add pl330-broken-no-flushp quirk " Caesar Wang
2016-01-15 13:49 ` Caesar Wang
2016-01-17 19:20 ` Heiko Stuebner
2016-01-17 19:20 ` 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=2073307.S3XLZp3hP0@diego \
--to=heiko@sntech.de \
--cc=broonie@kernel.org \
--cc=jay.xu@rock-chips.com \
--cc=keescook@google.com \
--cc=leozwang@google.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=sonnyrao@chromium.org \
--cc=wxt@rock-chips.com \
/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.