From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752997AbbJWW7T (ORCPT ); Fri, 23 Oct 2015 18:59:19 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:33426 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750869AbbJWW7R (ORCPT ); Fri, 23 Oct 2015 18:59:17 -0400 Date: Sat, 24 Oct 2015 07:58:58 +0900 From: Mark Brown To: Damien Horsley Cc: alsa-devel@alsa-project.org, James Hartley , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: <20151023225858.GP29919@sirena.org.uk> References: <1444653637-14711-1-git-send-email-Damien.Horsley@imgtec.com> <1444653637-14711-7-git-send-email-Damien.Horsley@imgtec.com> <20151019180757.GJ32054@sirena.org.uk> <5629371F.5080700@imgtec.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="s2lX4GznBIrto1wi" Content-Disposition: inline In-Reply-To: <5629371F.5080700@imgtec.com> X-Cookie: The coast was clear. User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: 58.123.138.205 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [alsa-devel] [PATCH V2 06/10] ASoC: img: Add driver for parallel output controller X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --s2lX4GznBIrto1wi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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. --s2lX4GznBIrto1wi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWKruyAAoJECTWi3JdVIfQcocH/jKKuF/aoqC6SjyPRGSNI2Pv NWIR1eVlnOwBFLP8X8TE/AInmkrVqw4Lraqb/r5nVMKWh88X9UwXh5xpoiSl3lcN 8krYXuCB7hvRyFtaP6jCaZwVTA/lHzTFMQHOMxG4T9AFCEF4RY/WTQnkUftMPoCD uh71vmCvyDjPoLH105IEoqgl8/RUTjwPH5PXOlDWyVflP+iugjxLFwwJ9dJ6LSf3 BK+oxP0sKhH5VDsPeVVdCPWlnF+i2F4MT4HZxitWNEsuv+ofZjqghVTHNbjyQ+iW 0Y9InDrDSMZpYIIT67XnIoqQjfJDYTvvYI2JGbaHlnh0pxT/E5GyFxtKpYSV4Po= =B7qK -----END PGP SIGNATURE----- --s2lX4GznBIrto1wi--