diff mbox

[v1,1/3] ASoC: zx-96p22: add documentation for zte's aud96p22 controller

Message ID 1487156110-12840-1-git-send-email-baoyou.xie@linaro.org
State Not Applicable, archived
Headers show

Commit Message

Baoyou Xie Feb. 15, 2017, 10:55 a.m. UTC 
This patch adds dt-binding documentation for zte's aud96p22 controller.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 .../devicetree/bindings/sound/zte,zx-96p22.txt     | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/zte,zx-96p22.txt

Comments

Charles Keepax Feb. 15, 2017, 11:24 a.m. UTC | #1 
On Wed, Feb 15, 2017 at 06:55:10PM +0800, Baoyou Xie wrote:
> This patch adds aud96p22 controller driver for zte's SoC family.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
<snip>
> +static int zx_aud96p22_i2c_write(struct i2c_client *i2c_client,
> +				 const void *data, size_t count)
> +{
> +	int xfer;
> +
> +	xfer = i2c_master_send(i2c_client, data, count);
> +	if (xfer == count)
> +		return 0;
> +	else if (xfer < 0)
> +		return xfer;
> +	else
> +		return -EIO;
> +}
> +
> +static int zx_aud96p22_i2c_read(struct i2c_client *i2c_client,
> +				unsigned char addr)
> +{
> +	int xfer;
> +
> +	xfer = i2c_smbus_read_word_data(i2c_client, addr);
> +	if (xfer < 0)
> +		dev_warn(&i2c_client->dev, "transfer error %d\n", xfer);
> +
> +	return xfer;
> +}
> +

Is there any reason this isn't using regmap? It looks like it
should be, have a look at any of the other mainline CODECs for an
example.

Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo Feb. 16, 2017, 11 a.m. UTC | #2 
On Wed, Feb 15, 2017 at 06:55:08PM +0800, Baoyou Xie wrote:
> This patch adds dt-binding documentation for zte's aud96p22 controller.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

Suggest to replace "documentation" with "bindings doc" in patch subject.

> ---
>  .../devicetree/bindings/sound/zte,zx-96p22.txt     | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> new file mode 100644
> index 0000000..4184566
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> @@ -0,0 +1,24 @@
> +ZTE zx96p22 controller
> +
> +Required properties:
> + - compatible : Must be "zte,zx-aud96p22"
> + - #sound-dai-cells: Should be 0
> + - reg : Offset of I2C register for zx96p22

"zte,zx-96p22.txt", "zte,zx-aud96p22" and "zx96p22".  Can we make these
names consistent?

> +
> +Example:
> +
> +	audio_i2c0: audio_i2c0@1486000 {

Node name should be as generic as possible.  I think the following one
is what we want.

	audio_i2c0: i2c@1486000 {

> +		compatible = "zte,zx296718-i2c";
> +		reg = <0x01486000 0x1000>;
> +		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&audiocrm AUDIO_I2C0_WCLK>;
> +		clock-frequency = <1600000>;
> +		status = "ok";

Drop this 'status' property, which is not so meaningful for example in
bindings doc.

And have a newline between properties and child node.

> +		inner_codec: aud96p22@22 {

		aud96p22: codec@22 {

Shawn

> +			compatible = "zte,zx-aud96p22";
> +			#sound-dai-cells = <0>;
> +			reg = <0x22>;
> +		};
> +	};
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo Feb. 16, 2017, 11:17 a.m. UTC | #3 
On Wed, Feb 15, 2017 at 06:55:10PM +0800, Baoyou Xie wrote:
> This patch adds aud96p22 controller driver for zte's SoC family.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

s/controller/codec in patch subject.

> ---
>  sound/soc/codecs/Kconfig       |   4 +
>  sound/soc/codecs/Makefile      |   2 +
>  sound/soc/codecs/zx_aud96p22.c | 588 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 594 insertions(+)
>  create mode 100644 sound/soc/codecs/zx_aud96p22.c
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index cfc108e..120af32 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -1116,4 +1116,8 @@ config SND_SOC_TPA6130A2
>  	tristate "Texas Instruments TPA6130A2 headphone amplifier"
>  	depends on I2C
>  
> +config SND_SOC_ZX96P22
> +	tristate "ZTE Inner AUD96P22 CODEC"
> +	depends on I2C
> +
>  endmenu
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 2624c73..dbc3818 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -219,6 +219,7 @@ snd-soc-wm9705-objs := wm9705.o
>  snd-soc-wm9712-objs := wm9712.o
>  snd-soc-wm9713-objs := wm9713.o
>  snd-soc-wm-hubs-objs := wm_hubs.o
> +snd-soc-zx96p22-objs := zx_aud96p22.o
>  # Amp
>  snd-soc-max9877-objs := max9877.o
>  snd-soc-max98504-objs := max98504.o
> @@ -444,6 +445,7 @@ obj-$(CONFIG_SND_SOC_WM9712)	+= snd-soc-wm9712.o
>  obj-$(CONFIG_SND_SOC_WM9713)	+= snd-soc-wm9713.o
>  obj-$(CONFIG_SND_SOC_WM_ADSP)	+= snd-soc-wm-adsp.o
>  obj-$(CONFIG_SND_SOC_WM_HUBS)	+= snd-soc-wm-hubs.o
> +obj-$(CONFIG_SND_SOC_ZX96P22)	+= snd-soc-zx96p22.o
>  
>  # Amp
>  obj-$(CONFIG_SND_SOC_MAX9877)	+= snd-soc-max9877.o
> diff --git a/sound/soc/codecs/zx_aud96p22.c b/sound/soc/codecs/zx_aud96p22.c
> new file mode 100644
> index 0000000..f2979df
> --- /dev/null
> +++ b/sound/soc/codecs/zx_aud96p22.c
> @@ -0,0 +1,588 @@
> +/*
> + * ZTE's audio 96p22 driver
> + *
> + * Copyright (C) 2017 ZTE Ltd
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dai.h>
> +#include <sound/pcm.h>
> +
> +#define BGPIO64				(64)

GPIO resource is a board level configuration, which might be different
from one board to another.  Instead of hard-coding, it should be
retrieved from device tree.

> +#define snd_kcontrol_dev(kcontrol)	\
> +		((struct device *)((kcontrol)->private_value))
> +
> +struct i2c_reg {
> +	unsigned char addr;
> +	unsigned char high_data;
> +	unsigned char low_data;
> +};
> +
> +struct zx_aud96p22_info {
> +	struct device   *dev;

One space is good enough between type and variable.

> +	int gpio;
> +	bool capture;
> +};
> +
> +static struct i2c_reg i2c_dac_master_volume_table[] = {
> +	{ 0x34, 0xe7, 0xe7 },
> +};
> +
> +static struct i2c_reg i2c_adc_master_volume_table[] = {
> +	{ 0x24, 0xbf, 0xbf },
> +};
> +
> +static struct i2c_reg i2c_dac_headset_volume_table[] = {
> +	{ 0x38, 0x0d, 0x0d },
> +};
> +
> +static struct i2c_reg i2c_dac_sleep_table[] = {
> +	{ 0x18, 0x00, 0x00 }, //play power down

/* single line comment */ please.

> +};
> +
> +static struct i2c_reg i2c_dac_wakeup_table[] = {
> +	{ 0x18, 0x00, 0xff }, //play power up
> +};
> +
> +static struct i2c_reg i2c_adc_sleep_table[] = {
> +	{ 0x16, 0x00, 0x00 }, //record power down
> +};
> +
> +static struct i2c_reg i2c_adc_wakeup_table[] = {
> +	{ 0x16, 0x00, 0x0f }, //record power up
> +};
> +
> +static struct i2c_reg i2c_codec_start_table[] = {
> +	{ 0x15, 0x00, 0x00 }, //power down control
> +	{ 0x47, 0x00, 0x00 }, //record path slect
> +	{ 0x24, 0xbf, 0xbf }, //record volume control
> +	{ 0x26, 0x30, 0x30 }, //record pga volume control
> +	{ 0xc8, 0x00, 0x00 }, //ALC control
> +	{ 0xce, 0x00, 0xf5 }, //record noise gate
> +	{ 0xf3, 0x00, 0xc0 }, //dac noise dithe
> +	{ 0xcd, 0x00, 0x20 }, //max record volume
> +	{ 0x15, 0x00, 0x01 }, //power down control
> +	{ 0x18, 0x00, 0xff }, //play power  control
> +	{ 0x16, 0x00, 0x0f }, //record power up
> +	{ 0x19, 0x00, 0x04 }, //power down control
> +	{ 0x02, 0x00, 0x05 }, //ext clock slect
> +	{ 0x01, 0x00, 0x05 }, //ext clock slect
> +	{ 0x00, 0x00, 0x00 }, //adc dpz reset
> +	{ 0x00, 0x00, 0x03 }, //dac dpz reset
> +	{ 0x04, 0x00, 0x40 }, //clk div
> +	{ 0x05, 0x00, 0x04 }, //clk div0x4
> +	{ 0x06, 0x00, 0x40 }, //clk div
> +	{ 0x07, 0x00, 0x04 }, //clk div 0x4
> +	{ 0x03, 0x00, 0x01 }, //slave 16bit i2s
> +	{ 0x00, 0x00, 0x00 }, //adc dpz reset
> +	{ 0x00, 0x00, 0x03 }, //dac dpz reset
> +};

<snip>

I skipped the code in between, and will review them in the next version,
since most of them needs update anyway to use regmap interface.

> +static int zx_aud96p22_i2c_probe(struct i2c_client *i2c_client,
> +				const struct i2c_device_id *id)
> +{
> +	int ret = 0;
> +	struct device *pdev = &i2c_client->dev;

'pdev' is usually used for struct platform_device type.  Please use
'dev' here.

> +
> +	if (!i2c_client)
> +		return -ENODEV;
> +
> +	ret = snd_soc_register_codec(pdev, &zx_aud96p22_driver,
> +				&zx_aud96p22_dai, 1);
> +
> +	return ret;
> +}
> +
> +static int zx_aud96p22_i2c_remove(struct i2c_client *i2c_client)
> +{
> +	struct device *pdev = &i2c_client->dev;

Ditto

Shawn

> +
> +	snd_soc_unregister_codec(pdev);
> +
> +	return 0;
> +}
> +
> +const struct of_device_id zx_aud96p22_of_dt_ids[] = {
> +	{ .compatible = "zte,zx-aud96p22", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, zx_aud96p22_of_dt_ids);
> +
> +static struct i2c_driver aud96p22_i2c_driver = {
> +	.driver = {
> +		.name = "zx-aud96p22",
> +		.of_match_table = zx_aud96p22_of_dt_ids,
> +	},
> +	.probe = zx_aud96p22_i2c_probe,
> +	.remove = zx_aud96p22_i2c_remove,
> +};
> +module_i2c_driver(aud96p22_i2c_driver);
> +
> +MODULE_DESCRIPTION("ZTE ASoC AUD96P22 driver");
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) Feb. 27, 2017, 5:20 p.m. UTC | #4 
On Wed, Feb 15, 2017 at 06:55:08PM +0800, Baoyou Xie wrote:
> This patch adds dt-binding documentation for zte's aud96p22 controller.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
>  .../devicetree/bindings/sound/zte,zx-96p22.txt     | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> new file mode 100644
> index 0000000..4184566
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
> @@ -0,0 +1,24 @@
> +ZTE zx96p22 controller
> +
> +Required properties:
> + - compatible : Must be "zte,zx-aud96p22"
> + - #sound-dai-cells: Should be 0
> + - reg : Offset of I2C register for zx96p22
> +
> +Example:
> +
> +	audio_i2c0: audio_i2c0@1486000 {

i2c@...

> +		compatible = "zte,zx296718-i2c";
> +		reg = <0x01486000 0x1000>;
> +		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&audiocrm AUDIO_I2C0_WCLK>;
> +		clock-frequency = <1600000>;
> +		status = "ok";

No need for status in examples.

With that,

Acked-by: Rob Herring <robh@kernel.org>


> +		inner_codec: aud96p22@22 {
> +			compatible = "zte,zx-aud96p22";
> +			#sound-dai-cells = <0>;
> +			reg = <0x22>;
> +		};
> +	};
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
new file mode 100644
index 0000000..4184566
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/zte,zx-96p22.txt
@@ -0,0 +1,24 @@ 
+ZTE zx96p22 controller
+
+Required properties:
+ - compatible : Must be "zte,zx-aud96p22"
+ - #sound-dai-cells: Should be 0
+ - reg : Offset of I2C register for zx96p22
+
+Example:
+
+	audio_i2c0: audio_i2c0@1486000 {
+		compatible = "zte,zx296718-i2c";
+		reg = <0x01486000 0x1000>;
+		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&audiocrm AUDIO_I2C0_WCLK>;
+		clock-frequency = <1600000>;
+		status = "ok";
+		inner_codec: aud96p22@22 {
+			compatible = "zte,zx-aud96p22";
+			#sound-dai-cells = <0>;
+			reg = <0x22>;
+		};
+	};