All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Murphy <dmurphy@ti.com>
To: balbi@ti.com
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	broonie@kernel.org, linux-kernel@vger.kernel.org,
	linux-sound@vger.kernel.org
Subject: Re: [PATCH v4] ASoC: tas2552: Support TI TAS2552 Amplifier
Date: Thu, 3 Jul 2014 10:13:06 -0500	[thread overview]
Message-ID: <53B57302.8030100@ti.com> (raw)
In-Reply-To: <20140703150621.GQ5814@saruman.home>

Hi

On 07/03/2014 10:06 AM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jul 03, 2014 at 09:57:39AM -0500, Dan Murphy wrote:
>> On 07/03/2014 09:52 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Jul 03, 2014 at 09:39:53AM -0500, Dan Murphy wrote:
>>>> +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>>> +{
>>>> +	u8 serial_format;
>>>> +	struct snd_soc_codec *codec = dai->codec;
>>>> +
>>>> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>>>> +	case SND_SOC_DAIFMT_CBS_CFS:
>>>> +		serial_format = 0x00;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBS_CFM:
>>>> +		serial_format = TAS2552_WORD_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFS:
>>>> +		serial_format = TAS2552_BIT_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFM:
>>>> +		serial_format = (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK);
>>>> +		break;
>>>> +	default:
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	pm_runtime_get_sync(codec->dev);
>>>> +
>>>> +	snd_soc_update_bits(codec, TAS2552_SER_CTRL_1,
>>>> +			    (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK),
>>>> +			    serial_format);
>>>> +
>>>> +	pm_runtime_put(codec->dev);
>>> I have a feeling it's better to just put at the end of the function.
>>> Remember your pm_runtime_put() will issue i2c transfers which can take a
>>> looooooong time ;-)
>> I thought about that but the next switch case could return if the format
>> mask is invalid which means the runtime calls would not be balanced.
>>
>> So I decided to wrap the snd_soc calls with the pm_runtime calls to keep it
>> balanced.
> it looks like you can do both switch statements outside of the
> pm_runtime region and cache results on serial_format and do a single
> write to CTRL_1 register (?). If not, then just use two local variables.
>

Yeah I could probably consolidate these into a single call.  And throw a debug statement
in the default case to why the format was not set.

Dan

-- 
------------------
Dan Murphy

WARNING: multiple messages have this Message-ID (diff)
From: Dan Murphy <dmurphy@ti.com>
To: balbi@ti.com
Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	alsa-devel@alsa-project.org, broonie@kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v4] ASoC: tas2552: Support TI TAS2552 Amplifier
Date: Thu, 03 Jul 2014 15:13:06 +0000	[thread overview]
Message-ID: <53B57302.8030100@ti.com> (raw)
In-Reply-To: <20140703150621.GQ5814@saruman.home>

Hi

On 07/03/2014 10:06 AM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jul 03, 2014 at 09:57:39AM -0500, Dan Murphy wrote:
>> On 07/03/2014 09:52 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Jul 03, 2014 at 09:39:53AM -0500, Dan Murphy wrote:
>>>> +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>>> +{
>>>> +	u8 serial_format;
>>>> +	struct snd_soc_codec *codec = dai->codec;
>>>> +
>>>> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>>>> +	case SND_SOC_DAIFMT_CBS_CFS:
>>>> +		serial_format = 0x00;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBS_CFM:
>>>> +		serial_format = TAS2552_WORD_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFS:
>>>> +		serial_format = TAS2552_BIT_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFM:
>>>> +		serial_format = (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK);
>>>> +		break;
>>>> +	default:
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	pm_runtime_get_sync(codec->dev);
>>>> +
>>>> +	snd_soc_update_bits(codec, TAS2552_SER_CTRL_1,
>>>> +			    (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK),
>>>> +			    serial_format);
>>>> +
>>>> +	pm_runtime_put(codec->dev);
>>> I have a feeling it's better to just put at the end of the function.
>>> Remember your pm_runtime_put() will issue i2c transfers which can take a
>>> looooooong time ;-)
>> I thought about that but the next switch case could return if the format
>> mask is invalid which means the runtime calls would not be balanced.
>>
>> So I decided to wrap the snd_soc calls with the pm_runtime calls to keep it
>> balanced.
> it looks like you can do both switch statements outside of the
> pm_runtime region and cache results on serial_format and do a single
> write to CTRL_1 register (?). If not, then just use two local variables.
>

Yeah I could probably consolidate these into a single call.  And throw a debug statement
in the default case to why the format was not set.

Dan

-- 
------------------
Dan Murphy


WARNING: multiple messages have this Message-ID (diff)
From: Dan Murphy <dmurphy@ti.com>
To: <balbi@ti.com>
Cc: <linux-sound@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<alsa-devel@alsa-project.org>, <broonie@kernel.org>,
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH v4] ASoC: tas2552: Support TI TAS2552 Amplifier
Date: Thu, 3 Jul 2014 10:13:06 -0500	[thread overview]
Message-ID: <53B57302.8030100@ti.com> (raw)
In-Reply-To: <20140703150621.GQ5814@saruman.home>

Hi

On 07/03/2014 10:06 AM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jul 03, 2014 at 09:57:39AM -0500, Dan Murphy wrote:
>> On 07/03/2014 09:52 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Jul 03, 2014 at 09:39:53AM -0500, Dan Murphy wrote:
>>>> +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>>>> +{
>>>> +	u8 serial_format;
>>>> +	struct snd_soc_codec *codec = dai->codec;
>>>> +
>>>> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>>>> +	case SND_SOC_DAIFMT_CBS_CFS:
>>>> +		serial_format = 0x00;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBS_CFM:
>>>> +		serial_format = TAS2552_WORD_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFS:
>>>> +		serial_format = TAS2552_BIT_CLK_MASK;
>>>> +		break;
>>>> +	case SND_SOC_DAIFMT_CBM_CFM:
>>>> +		serial_format = (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK);
>>>> +		break;
>>>> +	default:
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	pm_runtime_get_sync(codec->dev);
>>>> +
>>>> +	snd_soc_update_bits(codec, TAS2552_SER_CTRL_1,
>>>> +			    (TAS2552_BIT_CLK_MASK | TAS2552_WORD_CLK_MASK),
>>>> +			    serial_format);
>>>> +
>>>> +	pm_runtime_put(codec->dev);
>>> I have a feeling it's better to just put at the end of the function.
>>> Remember your pm_runtime_put() will issue i2c transfers which can take a
>>> looooooong time ;-)
>> I thought about that but the next switch case could return if the format
>> mask is invalid which means the runtime calls would not be balanced.
>>
>> So I decided to wrap the snd_soc calls with the pm_runtime calls to keep it
>> balanced.
> it looks like you can do both switch statements outside of the
> pm_runtime region and cache results on serial_format and do a single
> write to CTRL_1 register (?). If not, then just use two local variables.
>

Yeah I could probably consolidate these into a single call.  And throw a debug statement
in the default case to why the format was not set.

Dan

-- 
------------------
Dan Murphy


  reply	other threads:[~2014-07-03 15:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 14:39 [PATCH v4] ASoC: tas2552: Support TI TAS2552 Amplifier Dan Murphy
2014-07-03 14:39 ` Dan Murphy
2014-07-03 14:39 ` Dan Murphy
2014-07-03 14:52 ` Felipe Balbi
2014-07-03 14:52   ` Felipe Balbi
2014-07-03 14:52   ` Felipe Balbi
2014-07-03 14:57   ` Dan Murphy
2014-07-03 14:57     ` Dan Murphy
2014-07-03 14:57     ` Dan Murphy
2014-07-03 15:06     ` Felipe Balbi
2014-07-03 15:06       ` Felipe Balbi
2014-07-03 15:06       ` Felipe Balbi
2014-07-03 15:13       ` Dan Murphy [this message]
2014-07-03 15:13         ` Dan Murphy
2014-07-03 15:13         ` Dan Murphy
2014-07-03 15:19         ` Felipe Balbi
2014-07-03 15:19           ` Felipe Balbi
2014-07-03 15:19           ` Felipe Balbi

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=53B57302.8030100@ti.com \
    --to=dmurphy@ti.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=balbi@ti.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.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.