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 at 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 at rock-chips.com>
> > + * Copyright (c) 2015 Collabora Ltd.
> > + * Author: Sjoerd Simons <sjoerd.simons at 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
--
Sjoerd Simons <sjoerd.simons at collabora.co.uk>
Collabora Ltd.
