All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rosin <peda@lysator.liu.se>
To: alsa-devel@alsa-project.org
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	linux-kernel@vger.kernel.org, Peter Rosin <peda@axentia.se>
Subject: [v2] NXP Semiconductors TFA9879 Amplifier Driver
Date: Thu,  6 Nov 2014 17:39:44 +0100	[thread overview]
Message-ID: <1415291985-9784-1-git-send-email-peda@lysator.liu.se> (raw)
In-Reply-To: <20141106160241.GM8509@sirena.org.uk>

On 2014-11-06 17:02, Mark Brown wrote:> On Thu, Nov 06, 2014 at 02:37:31PM +0000, Peter Rosin wrote:
>> > Mark Brown wrote:
>>>> > > > +	if (tfa9879->lsb_justified)
>>>> > > > +		TFA9879_REG(codec, SERIAL_INTERFACE_1, I2S_SET, i2s_set);
>>> > > Why does this need to be reset every time, shouldn't we just be setting the
>>> > > register in set_fmt().?
>> > Yes, I'd sure like to do that, but how do I get to the width in set_fmt()?
> Oh, this has some width related thing in it?

Yes, the amp has a different setting for each lsb-justified width.
(It also supports 18 and 20 bits wide data)

>>>> > > > +	{ TFA9879_MISC_STATUS,		0x0000 }, /* 0x15, read-only */
>>>> > > > +};
>>>> > > > +static bool tfa9879_volatile_register(struct device *dev, unsigned
>>>> > > > +int reg) {
>>>> > > > +	return reg == TFA9879_MISC_STATUS;
>>> > > If the register is volatile it shouldn't have a default value provided.
>> > Then I misunderstood what volatile was meant to do. I'll just nuke the
>> > function. It works fine anyway...
> A volatile register is one that the chip may change autonomously (eg, an
> interrupt status register).

That was what I assumed, and the register behaves like that. I naively
thought that declaring it as volatile would prevent the asoc core from
writing to it. In retrospect, I don't understand why I thought that...
Anyway, that bit can wait until someone actually needs to read the staus.

Here's an update with the following changes since v1:

- squashed patch 2/2
- zapped the TFA9879_REG macro
- zapped tfa9879_probe (which needlessly registered the regmap)
- moved tfa9879_prepare/_shutdown to a DAPM_SUPPLY widget
- zapped the tfa9879_volatile() thing
- made tfa9879_dai_ops const
- erased the redundant "Gain" from the bass/treble volume controls
- using params_width() instead of params_format()

Cheers,
Peter



WARNING: multiple messages have this Message-ID (diff)
From: Peter Rosin <peda@lysator.liu.se>
To: alsa-devel@alsa-project.org
Cc: Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Peter Rosin <peda@axentia.se>,
	linux-kernel@vger.kernel.org
Subject: [v2] NXP Semiconductors TFA9879 Amplifier Driver
Date: Thu,  6 Nov 2014 17:39:44 +0100	[thread overview]
Message-ID: <1415291985-9784-1-git-send-email-peda@lysator.liu.se> (raw)
In-Reply-To: <20141106160241.GM8509@sirena.org.uk>

On 2014-11-06 17:02, Mark Brown wrote:> On Thu, Nov 06, 2014 at 02:37:31PM +0000, Peter Rosin wrote:
>> > Mark Brown wrote:
>>>> > > > +	if (tfa9879->lsb_justified)
>>>> > > > +		TFA9879_REG(codec, SERIAL_INTERFACE_1, I2S_SET, i2s_set);
>>> > > Why does this need to be reset every time, shouldn't we just be setting the
>>> > > register in set_fmt().?
>> > Yes, I'd sure like to do that, but how do I get to the width in set_fmt()?
> Oh, this has some width related thing in it?

Yes, the amp has a different setting for each lsb-justified width.
(It also supports 18 and 20 bits wide data)

>>>> > > > +	{ TFA9879_MISC_STATUS,		0x0000 }, /* 0x15, read-only */
>>>> > > > +};
>>>> > > > +static bool tfa9879_volatile_register(struct device *dev, unsigned
>>>> > > > +int reg) {
>>>> > > > +	return reg == TFA9879_MISC_STATUS;
>>> > > If the register is volatile it shouldn't have a default value provided.
>> > Then I misunderstood what volatile was meant to do. I'll just nuke the
>> > function. It works fine anyway...
> A volatile register is one that the chip may change autonomously (eg, an
> interrupt status register).

That was what I assumed, and the register behaves like that. I naively
thought that declaring it as volatile would prevent the asoc core from
writing to it. In retrospect, I don't understand why I thought that...
Anyway, that bit can wait until someone actually needs to read the staus.

Here's an update with the following changes since v1:

- squashed patch 2/2
- zapped the TFA9879_REG macro
- zapped tfa9879_probe (which needlessly registered the regmap)
- moved tfa9879_prepare/_shutdown to a DAPM_SUPPLY widget
- zapped the tfa9879_volatile() thing
- made tfa9879_dai_ops const
- erased the redundant "Gain" from the bass/treble volume controls
- using params_width() instead of params_format()

Cheers,
Peter

  reply	other threads:[~2014-11-06 16:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06 12:53 NXP Semiconductors TFA9879 Amplifier Driver Peter Rosin
2014-11-06 12:53 ` Peter Rosin
2014-11-06 12:54 ` [PATCH 1/2] ASoC: tfa9879: New driver for NXP Semiconductors TFA9879 amplifier Peter Rosin
2014-11-06 13:27   ` [alsa-devel] " Lars-Peter Clausen
2014-11-06 13:37   ` Mark Brown
2014-11-06 14:37     ` Peter Rosin
2014-11-06 16:02       ` Mark Brown
2014-11-06 16:02         ` Mark Brown
2014-11-06 16:39         ` Peter Rosin [this message]
2014-11-06 16:39           ` [v2] NXP Semiconductors TFA9879 Amplifier Driver Peter Rosin
2014-11-06 16:39           ` [PATCH v2] ASoC: tfa9879: New driver for NXP Semiconductors TFA9879 amplifier Peter Rosin
2014-11-07 11:17             ` Mark Brown
2014-11-08 13:40               ` NXP Semiconductors TFA9879 Amplifier Driver Peter Rosin
2014-11-08 13:40                 ` Peter Rosin
2014-11-08 13:40                 ` [PATCH v3] ASoC: tfa9879: New driver for NXP Semiconductors TFA9879 amplifier Peter Rosin
2014-11-09  9:46                   ` Mark Brown
2014-11-06 12:54 ` [PATCH 2/2] ASoC: tfa9879: Add bass and treble gain/freq controls Peter Rosin
2014-11-06 12:54   ` Peter Rosin
2014-11-06 13:17   ` [alsa-devel] " Lars-Peter Clausen
2014-11-06 13:40   ` Mark Brown
2014-11-06 13:15 ` NXP Semiconductors TFA9879 Amplifier Driver Frans Klaver

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=1415291985-9784-1-git-send-email-peda@lysator.liu.se \
    --to=peda@lysator.liu.se \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peda@axentia.se \
    /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.