All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>,
	mark.rutland@arm.com, oder_chiou@realtek.com,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	Darren Hart <dvhart@linux.intel.com>,
	lgirdwood@gmail.com, linux-kernel@vger.kernel.org,
	Nicolin Chen <nicoleotsuka@gmail.com>,
	robh+dt@kernel.org, bardliao@realtek.com
Subject: Re: [alsa-devel] [PATCH] ASoC: rt5659: Add mclk controls
Date: Wed, 10 Aug 2016 18:06:32 +0100	[thread overview]
Message-ID: <20160810170632.GL9347@sirena.org.uk> (raw)
In-Reply-To: <c649c3fe-b111-56a8-275e-d967a961dd41@linux.intel.com>

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

On Wed, Aug 10, 2016 at 08:57:28AM -0500, Pierre-Louis Bossart wrote:

> Without going into a debate on x86 v. the clock API or the merits of a patch
> that has already been applied, I am pretty confused on who's supposed to
> manage the mclk between the machine and codec driver.

> So on a DAPM transition the clock is enabled. Fine.
> What's not clear here is that the codec driver doesn't know what rates are
> supported by the SOC/chipset. The machine driver is typically the one doing
> calls such as

This should really be being propagated through the clock tree by the
clock API rather than open coded - for a lot of things it'll just boil
down to a clk_set_rate() at the edge of the clock tree.  Any constraints
should also be applied through the clock API, though in a lot of cases
the devices are simple enough that it should be a fairly mechanical
process.

> so the summary is that we have two ways of doing the same thing - turning
> the mclk on when it's needed - and I wonder if doing this in the codec is
> really the right solution? Again this is not a question on the merits of the
> clk API/framework but whether we can have a single point of control instead
> of two pieces of code doing the same thing in two drivers.
> If I am missing something I am all ears.

We've got two ways of doing this at the minute partly because
historically things have been open coded in the machine drivers due to
the lack of a clock API, now we have one we can use we should be using
it consistently to set rates.  Where the machine driver needs to do
things dynamically it really ought to be able to express the constraints
it's trying to set through the clock API, if we can't do things we need
we should improve the clock API.  This will mean that we don't have to
reinvent the wheel when we're doing things with clocks, we have
consistent interfaces to all parts of the clock tree and other bits of
the system will get reuse from anything we've learned about
implementation.

The CODEC clearly has *some* idea of what's going on here, and
especially for simpler CODECs the code to drive the clocking should be
fairly easy to generalize as there's few options.  From a clock API
point of view the CODEC really ought to be the one requesting the clocks
that go into it, though there's nothing that says it has to only use its
own information to do that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: mark.rutland@arm.com, oder_chiou@realtek.com,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	Vinod Koul <vinod.koul@intel.com>,
	Darren Hart <dvhart@linux.intel.com>,
	lgirdwood@gmail.com, linux-kernel@vger.kernel.org,
	Nicolin Chen <nicoleotsuka@gmail.com>,
	robh+dt@kernel.org, bardliao@realtek.com
Subject: Re: [PATCH] ASoC: rt5659: Add mclk controls
Date: Wed, 10 Aug 2016 18:06:32 +0100	[thread overview]
Message-ID: <20160810170632.GL9347@sirena.org.uk> (raw)
In-Reply-To: <c649c3fe-b111-56a8-275e-d967a961dd41@linux.intel.com>


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

On Wed, Aug 10, 2016 at 08:57:28AM -0500, Pierre-Louis Bossart wrote:

> Without going into a debate on x86 v. the clock API or the merits of a patch
> that has already been applied, I am pretty confused on who's supposed to
> manage the mclk between the machine and codec driver.

> So on a DAPM transition the clock is enabled. Fine.
> What's not clear here is that the codec driver doesn't know what rates are
> supported by the SOC/chipset. The machine driver is typically the one doing
> calls such as

This should really be being propagated through the clock tree by the
clock API rather than open coded - for a lot of things it'll just boil
down to a clk_set_rate() at the edge of the clock tree.  Any constraints
should also be applied through the clock API, though in a lot of cases
the devices are simple enough that it should be a fairly mechanical
process.

> so the summary is that we have two ways of doing the same thing - turning
> the mclk on when it's needed - and I wonder if doing this in the codec is
> really the right solution? Again this is not a question on the merits of the
> clk API/framework but whether we can have a single point of control instead
> of two pieces of code doing the same thing in two drivers.
> If I am missing something I am all ears.

We've got two ways of doing this at the minute partly because
historically things have been open coded in the machine drivers due to
the lack of a clock API, now we have one we can use we should be using
it consistently to set rates.  Where the machine driver needs to do
things dynamically it really ought to be able to express the constraints
it's trying to set through the clock API, if we can't do things we need
we should improve the clock API.  This will mean that we don't have to
reinvent the wheel when we're doing things with clocks, we have
consistent interfaces to all parts of the clock tree and other bits of
the system will get reuse from anything we've learned about
implementation.

The CODEC clearly has *some* idea of what's going on here, and
especially for simpler CODECs the code to drive the clocking should be
fairly easy to generalize as there's few options.  From a clock API
point of view the CODEC really ought to be the one requesting the clocks
that go into it, though there's nothing that says it has to only use its
own information to do that.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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



  reply	other threads:[~2016-08-10 18:56 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-27 23:02 [PATCH] ASoC: rt5659: Add mclk controls Nicolin Chen
2016-07-27 23:02 ` Nicolin Chen
2016-07-28 15:57 ` Mark Brown
2016-07-28 15:57   ` Mark Brown
2016-07-28 18:14   ` Nicolin Chen
2016-07-28 18:14     ` Nicolin Chen
2016-07-28 18:55     ` Mark Brown
2016-07-28 18:55       ` Mark Brown
2016-07-28 19:03       ` Nicolin Chen
2016-07-29 16:15       ` Vinod Koul
2016-07-29 16:39         ` Mark Brown
2016-07-29 16:55           ` [alsa-devel] " Vinod Koul
2016-07-29 16:55             ` Vinod Koul
2016-08-10 13:57           ` Pierre-Louis Bossart
2016-08-10 13:57             ` Pierre-Louis Bossart
2016-08-10 17:06             ` Mark Brown [this message]
2016-08-10 17:06               ` Mark Brown
2016-08-10 17:31               ` [alsa-devel] " Pierre-Louis Bossart
2016-08-10 17:31                 ` Pierre-Louis Bossart
2016-08-10 17:52                 ` [alsa-devel] " Mark Brown
2016-08-10 17:52                   ` Mark Brown
2016-08-10 21:59                   ` [alsa-devel] " Pierre-Louis Bossart
2016-08-11 11:34                     ` Mark Brown
2016-08-11 11:34                       ` Mark Brown
2016-07-28 20:40 ` Lars-Peter Clausen
2016-07-28 20:40   ` Lars-Peter Clausen
2016-07-28 20:51   ` Nicolin Chen
2016-07-28 20:51     ` Nicolin Chen

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=20160810170632.GL9347@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=bardliao@realtek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dvhart@linux.intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nicoleotsuka@gmail.com \
    --cc=oder_chiou@realtek.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=robh+dt@kernel.org \
    --cc=vinod.koul@intel.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.