Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1250982

Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel output controller

From Mark Brown <broonie@kernel.org>
Newsgroups linux.kernel
Subject Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel output controller
Date 2015-10-19 20:10 +0200
Message-ID <qln2G-3Iv-19@gated-at.bofh.it> (permalink)
References <qlmJk-35z-39@gated-at.bofh.it> <qln2G-3Iv-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Mon, Oct 12, 2015 at 01:40:33PM +0100, Damien Horsley wrote:

> +	spin_lock_irqsave(&prl->lock, flags);
> +	reg = img_prl_out_readl(prl, IMG_PRL_OUT_CTL);
> +	ucontrol->value.integer.value[0] = !!(reg & IMG_PRL_OUT_CTL_EDGE_MASK);
> +	spin_unlock_irqrestore(&prl->lock, flags);

Do you need to lock a single register read?

> +static struct snd_kcontrol_new img_prl_out_controls[] = {
> +	{
> +		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> +		.name = "Parallel Out Edge Falling",
> +		.info = img_prl_out_edge_info,
> +		.get = img_prl_out_get_edge,
> +		.put = img_prl_out_set_edge
> +	}
> +};

If this is a boolean control (it looked like one) it should be called
Switch but it's not clear to me what exactly is being controlled here or
why it's something that should be exposed to userspace.

> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		reg = img_prl_out_readl(prl, IMG_PRL_OUT_CTL);
> +		reg |= IMG_PRL_OUT_CTL_ME_MASK;
> +		img_prl_out_writel(prl, reg, IMG_PRL_OUT_CTL);
> +		prl->active = true;
> +		break;
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		img_prl_out_reset(prl);
> +		prl->active = false;
> +		break;

No need for locking on the reset (and doesn't this mean that the control
state is going to be changed underneath userspace)?

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel  output controller Mark Brown <broonie@kernel.org> - 2015-10-19 20:10 +0200
  Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel  output controller Damien Horsley <Damien.Horsley@imgtec.com> - 2015-10-22 21:30 +0200
    Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel  output controller Mark Brown <broonie@kernel.org> - 2015-10-24 01:00 +0200

csiph-web