Re: [alsa-devel] [PATCH 2/4] ASoc: rockchip: Add rockchip SPDIF transceiver driver

On Tue, 2015-07-28 at 16:28 +0200, Heiko Stübner wrote:

Hi,

could you streamline the prefixes a bit perhaps? I.e. so far I've seen

rk_spdif_dev spdif_runtime_suspend rockchip_snd_txctrl rockchip_spdif_hw_params

I guess rockchip_spdif_* or rk_spdif_* for everything might make this a bit nicer

Will do in V2, i probalby copied a few too many warts from the i2s driver ;)

Thanks for the review!

Am Dienstag, 28. Juli 2015, 14:03:29 schrieb Sjoerd Simons:

...

Add a driver for the SDPIF transceiver available on RK3066, RK3188 and RK3288. Heavily based on the rockchip i2s driver.

Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk

sound/soc/rockchip/Kconfig | 9 + sound/soc/rockchip/Makefile | 3 + sound/soc/rockchip/rockchip_spdif.c | 375 ++++++++++++++++++++++++++++++++++++ sound/soc/rockchip/rockchip_spdif.h | 63 ++++++ 4 files changed, 450 insertions(+) create mode 100644 sound/soc/rockchip/rockchip_spdif.c create mode 100644 sound/soc/rockchip/rockchip_spdif.h

diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig index 58bae8e..20bc676 100644 --- a/sound/soc/rockchip/Kconfig +++ b/sound/soc/rockchip/Kconfig @@ -15,6 +15,14 @@ config SND_SOC_ROCKCHIP_I2S Rockchip I2S device. The device supports upto maximum of 8 channels each for play and record.

+config SND_SOC_ROCKCHIP_SPDIF

• tristate "Rockchip SPDIF Device Driver"
• depends on CLKDEV_LOOKUP && SND_SOC_ROCKCHIP
• select SND_SOC_GENERIC_DMAENGINE_PCM
• help

•  Say Y or M if you want to add support for SPDIF driver 
  

for

•  Rockchip SPDIF transceiver device.
  

config SND_SOC_ROCKCHIP_MAX98090 tristate "ASoC support for Rockchip boards using a MAX98090 codec" depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB @@ -33,3 +41,4 @@ config SND_SOC_ROCKCHIP_RT5645 help Say Y or M here if you want to add support for SoC audio on Rockchip boards using the RT5645/RT5650 codec, such as Veyron.

unrelated newline

...

diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile index 1bc1dc3..b02ab69 100644 --- a/sound/soc/rockchip/Makefile +++ b/sound/soc/rockchip/Makefile @@ -1,10 +1,13 @@ # ROCKCHIP Platform Support snd-soc-i2s-objs := rockchip_i2s.o +snd-soc-spdif-objs := rockchip_spdif.o

obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o +obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-spdif.o

snd-soc-rockchip-max98090-objs := rockchip_max98090.o snd-soc-rockchip-rt5645-objs := rockchip_rt5645.o

obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip -max98090.o obj-$(CONFIG_SND_SOC_ROCKCHIP_RT5645) += snd-soc-rockchip-rt5645.o

diff --git a/sound/soc/rockchip/rockchip_spdif.c b/sound/soc/rockchip/rockchip_spdif.c new file mode 100644 index 0000000..e60ccf6 --- /dev/null +++ b/sound/soc/rockchip/rockchip_spdif.c @@ -0,0 +1,375 @@ +/* sound/soc/rockchip/rockchip_spdif.c

• • ALSA SoC Audio Layer - Rockchip I2S Controller driver

						^spd

if

...

• • Copyright (c) 2014 Rockchip Electronics Co. Ltd.
• • Author: Jianqun jay.xu@rock-chips.com
• • Copyright (c) 2015 Collabora Ltd.
• • Author: Sjoerd Simons sjoerd.simons@collabora.co.uk
• • This program is free software; you can redistribute it and/or

modify

• • it under the terms of the GNU General Public License version 2

as

• • published by the Free Software Foundation.
• */

+#include <linux/module.h> +#include <linux/delay.h> +#include <linux/of_gpio.h> +#include <linux/clk.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <sound/pcm_params.h> +#include <sound/dmaengine_pcm.h>

+#include "rockchip_spdif.h"

+#define DRV_NAME "rockchip-spdif"

+struct rk_spdif_dev {

• struct device *dev;
• struct clk *mclk;
• struct clk *hclk;
• struct snd_dmaengine_dai_dma_data playback_dma_data;
• struct regmap *regmap;

+};

+static int spdif_runtime_suspend(struct device *dev) +{

• struct rk_spdif_dev *spdif = dev_get_drvdata(dev);
• clk_disable_unprepare(spdif->mclk);
• return 0;

+}

+static int spdif_runtime_resume(struct device *dev) +{

• struct rk_spdif_dev *spdif = dev_get_drvdata(dev);
• int ret;
• ret = clk_prepare_enable(spdif->mclk);
• if (ret) {

• dev_err(spdif->dev, "clock enable failed %d\n", 
  

ret);

• return ret;
  

• }
• return 0;

+}

+static inline struct rk_spdif_dev *to_info(struct snd_soc_dai *dai) +{

• return snd_soc_dai_get_drvdata(dai);

+}

+static void rockchip_snd_txctrl(struct rk_spdif_dev *spdif, int on) +{

• if (on) {

• regmap_update_bits(spdif->regmap, SPDIF_DMACR,
  

• 		   SPDIF_DMACR_TDE_ENABLE,
  

• 		   SPDIF_DMACR_TDE_ENABLE);
  

• regmap_update_bits(spdif->regmap, SPDIF_XFER,
  

• 		   SPDIF_XFER_TXS_START,
  

• 		   SPDIF_XFER_TXS_START);
  

personally I'm always unsure of regmap return values. While the underlying method is mmio in this case, regmap_* in theory still has the possibility to return errors, so I'm not sure if it's ok to silently ignore them.

Here it would simply mean return the error and also return it in rockchip_spdif_trigger below.

Heiko