On Fri, Feb 2, 2018 at 6:20 PM, Daniel Baluta daniel.baluta@nxp.com wrote:
AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC for digital audio systems.
Datasheet is available at:
https://www.akm.com/akm/en/file/datasheet/AK5558VN.pdf
Initial patch includes support for normal and TDM modes.
Signed-off-by: Junichi Wakasugi wakasugi.jb@om.asahi-kasei.co.jp [initial coding for 3.18 kernel] Signed-off-by: Mihai Serban mihai.serban@nxp.com [cleanups and porting to 4.9 kernel] Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com [tdm support] Signed-off-by: Daniel Baluta daniel.baluta@nxp.com [pm support, cleanups and porting to latest kernel]
Thanks for an update.
Couple of nitpicks below, otherwise FWIW, Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
@@ -0,0 +1,626 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- ak5558.c -- audio driver for AK5558 ADC
I would remove filename. If file is ever be renamed this will make an additional noise.
- Copyright (C) 2015 Asahi Kasei Microdevices Corporation
- Copyright 2018 NXP
- */
+#include <linux/module.h> +#include <linux/delay.h> +#include <linux/slab.h> +#include <linux/gpio/consumer.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <linux/pm_runtime.h> +#include <sound/initval.h> +#include <linux/i2c.h> +#include <sound/tlv.h> +#include <linux/of_gpio.h> +#include <linux/regmap.h>
I would rather keep it sorted
+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/slab.h> // yeah keep an empty line here to split groups +#include <sound/initval.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> +#include <sound/tlv.h>
+#include <linux/of_gpio.h>
This one is redundant.
+static int ak5558_set_dai_mute(struct snd_soc_dai *dai, int mute) +{
struct snd_soc_codec *codec = dai->codec;struct ak5558_priv *ak5558 = snd_soc_codec_get_drvdata(codec);
int ndt = 0;
It might be even
int ndt = max(ak5558->fs ? 583000 / ak5558->fs : 5, 5);
if (!mute)return 0;
if (ak5558->fs != 0)ndt = 583000 / ak5558->fs;msleep(max(ndt, 5));
...and here just
msleep(ndt);
But I don't know if Mark is okay with that.
return 0;+}
pm_runtime_enable(&i2c->dev);pm_runtime_disable(&client->dev);
I'm also not sure about these calls, but I leave it to you.
+static const struct i2c_device_id ak5558_i2c_id[] = {
{ "ak5558", 0 },{ }+}; +MODULE_DEVICE_TABLE(i2c, ak5558_i2c_id);
I dunno if it's really helpful to have. Though it's up to Mark and you.