From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757232AbbJAUnk (ORCPT ); Thu, 1 Oct 2015 16:43:40 -0400 Received: from down.free-electrons.com ([37.187.137.238]:43693 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756752AbbJAUmx (ORCPT ); Thu, 1 Oct 2015 16:42:53 -0400 Date: Thu, 1 Oct 2015 22:11:22 +0200 From: Maxime Ripard To: codekipper@gmail.com Cc: lgirdwood@gmail.com, linux-arm-kernel@lists.infradead.org, broonie@kernel.org, linux-sunxi@googlegroups.com, linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, be17068@iperbole.bo.it Subject: Re: [PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block Message-ID: <20151001201122.GM7104@lukather> References: <1443635458-8873-1-git-send-email-codekipper@gmail.com> <1443635458-8873-5-git-send-email-codekipper@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rvv34iIeJXisuDc0" Content-Disposition: inline In-Reply-To: <1443635458-8873-5-git-send-email-codekipper@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --rvv34iIeJXisuDc0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 30, 2015 at 07:50:58PM +0200, codekipper@gmail.com wrote: > From: Marcus Cooper >=20 > The sun4i, sun6i and sun7i SoC families have an SPDIF > block which is capable of playback and capture. >=20 > This patch enables the playback of this block for > the sun4i and sun7i families. >=20 > Signed-off-by: Marcus Cooper > --- > sound/soc/sunxi/Kconfig | 12 + > sound/soc/sunxi/Makefile | 4 + > sound/soc/sunxi/sun4i-spdif.c | 612 ++++++++++++++++++++++++++++++++++++= ++++++ > 3 files changed, 628 insertions(+) > create mode 100644 sound/soc/sunxi/sun4i-spdif.c >=20 > diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig > index 84c72ec..2ebf43d 100644 > --- a/sound/soc/sunxi/Kconfig > +++ b/sound/soc/sunxi/Kconfig > @@ -8,4 +8,16 @@ config SND_SUN4I_CODEC > Select Y or M to add support for the Codec embedded in the Allwinner > A10 and affiliated SoCs. > =20 > +config SND_SOC_SUNXI_DAI_SPDIF > + tristate > + depends on OF > + select SND_SOC_GENERIC_DMAENGINE_PCM > + select REGMAP_MMIO > + > +config SND_SOC_SUNXI_MACHINE_SPDIF > + tristate "APB on-chip sun4i/sun5i/sun7i SPDIF" > + depends on OF > + select SND_SOC_SUNXI_DAI_SPDIF > + help > + Say Y if you want to add support for SoC S/PDIF audio as simpl= e audio card. You still haven't said why you can't use simple-card... > +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host) > +{ > + u32 reg_val; > + > + /* soft reset SPDIF */ > + regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET); > + > + /* MCLK OUTPUT enable */ > + regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL, > + SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN); The alignment is still not right.... > + /* flush TX FIFO */ > + regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL, > + SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX); > + > + /* clear interrupt status */ > + regmap_read(host->regmap, SUN4I_SPDIF_ISTA, ®_val); > + regmap_write(host->regmap, SUN4I_SPDIF_ISTA, reg_val); You're not using any interrupts. Why is this needed? > +static int sun4i_spdif_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *cpu_dai) > +{ > + struct snd_soc_pcm_runtime *rtd =3D substream->private_data; > + struct sun4i_spdif_dev *host =3D snd_soc_dai_get_drvdata(rtd->cpu_dai); > + > + if (substream->stream !=3D SNDRV_PCM_STREAM_PLAYBACK) > + return -EINVAL; > + > + sun4i_spdif_configure(host); > + > + return clk_prepare_enable(host->clk); You're still not using pm_runtime... > +} > + > +static void sun4i_spdif_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd =3D substream->private_data; > + struct sun4i_spdif_dev *host =3D snd_soc_dai_get_drvdata(rtd->cpu_dai); > + > + if (substream->stream !=3D SNDRV_PCM_STREAM_PLAYBACK) > + return; > + > + clk_disable_unprepare(host->clk); > +} > + > +static int sun4i_spdif_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *cpu_dai) > +{ > + int ret =3D 0; > + int fmt; > + unsigned long rate =3D params_rate(params); > + u32 mclk_div =3D 0; > + unsigned int mclk =3D 0; > + u32 reg_val; > + struct sun4i_spdif_dev *host =3D snd_soc_dai_get_drvdata(cpu_dai); > + struct platform_device *pdev =3D host->pdev; > + > + /* Add the PCM and raw data select interface */ > + switch (params_channels(params)) { > + case 1: /* PCM mode */ > + case 2: > + fmt =3D 0; > + break; > + case 4: /* raw data mode */ > + fmt =3D SUN4I_SPDIF_TXCFG_NONAUDIO; > + break; > + default: > + return -EINVAL; > + } > + > + switch (params_format(params)) { > + case SNDRV_PCM_FORMAT_S16_LE: > + fmt |=3D SUN4I_SPDIF_TXCFG_FMT16BIT; > + break; > + case SNDRV_PCM_FORMAT_S20_3LE: > + fmt |=3D SUN4I_SPDIF_TXCFG_FMT20BIT; > + break; > + case SNDRV_PCM_FORMAT_S24_LE: > + fmt |=3D SUN4I_SPDIF_TXCFG_FMT24BIT; > + break; > + default: > + return -EINVAL; > + } > + > + switch (rate) { > + case 22050: > + case 44100: > + case 88200: > + case 176400: > + mclk =3D 22579200; > + break; > + case 24000: > + case 32000: > + case 48000: > + case 96000: > + case 192000: > + mclk =3D 24576000; > + break; > + default: > + return -EINVAL; > + } > + > + ret =3D clk_set_rate(host->audio_clk, mclk); > + if (ret < 0) { > + dev_err(&pdev->dev, > + "Setting pll2 clock rate for %d Hz failed!\n", mclk); > + return ret; > + } You're still using the PLL2... > + > + ret =3D clk_set_rate(host->clk, mclk); > + if (ret < 0) { > + dev_err(&pdev->dev, > + "Setting SPDIF clock rate for %d Hz failed!\n", mclk); > + return ret; > + } > + > + reg_val =3D 0; > + reg_val &=3D ~SUN4I_SPDIF_FCTL_FIFOSRC; > + reg_val |=3D SUN4I_SPDIF_FCTL_TXTL_MASK; > + reg_val |=3D SUN4I_SPDIF_FCTL_RXTL_MASK; > + reg_val |=3D SUN4I_SPDIF_FCTL_TXIM; > + reg_val |=3D SUN4I_SPDIF_FCTL_RXOM_MASK; > + regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val); You're still not using regmap_update_bits... IF you're really going to ignore all the comments we did, please tell us upfront. That way, we will not waste our time doing a review of your patches. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com --rvv34iIeJXisuDc0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWDZNpAAoJEBx+YmzsjxAgJ1MP/1U0nBLUenG1KWes3AFVgfYL tKVjWC0AhoGnk6i6iLyF0m+a+eu9Lg4n4SY2NyTT5irr5tf5dG3kYTXTvnNXAcRS s+FT9c+E0eT3k7WSLomhuUbuCB5xoXgTg/J1CpnBbEMtgB736FwarIc861w5+LXo E+wawq0Gjo7SzshE6skFyJ7XTWCUdFt5cMPVVDTfdzNbMK8GMazm1L48yJ1VpJdD WOos6X7GKG8TDmow/athCwOrkENB5vQu+NxpbSEu0oDNMhym4HFffavCo9LDBn7W /IKRXi1f7VihzkwVC/iOIdqQvRlC3qavKGA2ad6nlnyH1dJV+NqM0ozqOrIvimyA oOZtuePcFQMOkePSrApX00fQ+5cez1vpxrbKb1OWlIDl81S1yDUqlh6tFIQM/9AF A622UiXfFUhayPIIKFfd1wsF4RLuUYcuYHKwNk9pb477Z1yrBXKXnYc9QVxcSS9I Mp/L3ZpMOMvj5hTFrOfGKlvKj2DPeoo91BJzDqL0E8GXoXmfhxH5h0GHla//zJhp 5F2YjmsvG/j6sb8Vhl6VntH8TZ881MvQLq6Q4K8JbSgF/wXeVJDoAy+y1jlAs2Px OL1dQioLadt27TEeOnus1Bw8YPdemY1mAWKcmGRp0p0NMJpka5KP4rvED0d4Hp3Z oliTGXuNhVRU791793rG =CjGx -----END PGP SIGNATURE----- --rvv34iIeJXisuDc0--