On Thu, Oct 22, 2015 at 08:21:03PM +0100, Damien Horsley wrote:
On 19/10/15 19:07, Mark Brown wrote:
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?
Between the calls to reset_control_assert and reset_control_deassert, the block is held in reset. During this time, no register access will succeed. All register access that may occur concurrently with the reset needs to be locked
Add a comment so readers know this.
+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.
This controls the edge (rising/falling) of the frame clock that the samples are generated on. Should I create a set_fmt function and use SND_SOC_DAIFMT_NB_NF / SND_SOC_DAIFMT_NB_IF to set this instead? I wasn't sure if those formats were just for I2S or not
Yes, that's part of the DAI format - and we definitely don't want users randomly changing this at runtime, the CODEC and CPU need to be configured together.