[alsa-devel] [PATCH 1/3] regmap: Properly round reg_bytes and val_bytes
For the upcoming 2/6-format, we don't see debugfs output otherwise, since the current division results in 0. I'd think 10/14 is broken currently, too.
Signed-off-by: Wolfram Sang w.sang@pengutronix.de --- drivers/base/regmap/regmap.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 6555803..8bd0232 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -159,8 +159,8 @@ struct regmap *regmap_init(struct device *dev,
mutex_init(&map->lock); map->format.buf_size = (config->reg_bits + config->val_bits) / 8; - map->format.reg_bytes = config->reg_bits / 8; - map->format.val_bytes = config->val_bits / 8; + map->format.reg_bytes = DIV_ROUND_UP(config->reg_bits, 8); + map->format.val_bytes = DIV_ROUND_UP(config->val_bits, 8); map->dev = dev; map->bus = bus; map->max_register = config->max_register;
Signed-off-by: Wolfram Sang w.sang@pengutronix.de --- drivers/base/regmap/regmap.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 8bd0232..fb3c132 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -76,6 +76,14 @@ static bool regmap_volatile_range(struct regmap *map, unsigned int reg, return true; }
+static void regmap_format_2_6_write(struct regmap *map, + unsigned int reg, unsigned int val) +{ + u8 *out = map->work_buf; + + *out = (reg << 6) | val; +} + static void regmap_format_4_12_write(struct regmap *map, unsigned int reg, unsigned int val) { @@ -178,6 +186,16 @@ struct regmap *regmap_init(struct device *dev, }
switch (config->reg_bits) { + case 2: + switch (config->val_bits) { + case 6: + map->format.format_write = regmap_format_2_6_write; + break; + default: + goto err_map; + } + break; + case 4: switch (config->val_bits) { case 12:
On Fri, Jan 27, 2012 at 04:10:22PM +0100, Wolfram Sang wrote:
Signed-off-by: Wolfram Sang w.sang@pengutronix.de
Applied, thanks.
Add a driver supporting the volume control and the mute pin. Shdn pin and DAPM are not taken care of yet.
Signed-off-by: Wolfram Sang w.sang@pengutronix.de ---
Mark: Still no DAPM support. We don't have PM on this board, and since I couldn't easily figure what is expected from me here, I'd like to leave this for people who have real interest in that.
include/sound/max9768.h | 24 ++++ sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/max9768.c | 253 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 283 insertions(+), 0 deletions(-) create mode 100644 include/sound/max9768.h create mode 100644 sound/soc/codecs/max9768.c
diff --git a/include/sound/max9768.h b/include/sound/max9768.h new file mode 100644 index 0000000..0f78b41 --- /dev/null +++ b/include/sound/max9768.h @@ -0,0 +1,24 @@ +/* + * Platform data for MAX9768 + * Copyright (C) 2011, 2012 by Wolfram Sang, Pengutronix e.K. + * same licence as the driver + */ + +#ifndef __SOUND_MAX9768_PDATA_H__ +#define __SOUND_MAX9768_PDATA_H__ + +/** + * struct max9768_pdata - optional platform specific MAX9768 configuration + * @shdn_gpio: GPIO to SHDN pin. If not valid, pin must be hardwired HIGH + * @mute_gpio: GPIO to MUTE pin. If not valid, control for mute won't be added + * @flags: configuration flags, e.g. set classic PWM mode (check datasheet + * regarding "filterless modulation" which is default). + */ +struct max9768_pdata { + int shdn_gpio; + int mute_gpio; + unsigned flags; +#define MAX9768_FLAG_CLASSIC_PWM (1 << 0) +}; + +#endif /* __SOUND_MAX9768_PDATA_H__*/ diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 7c205e7..2587c3b 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -40,6 +40,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_MAX98088 if I2C select SND_SOC_MAX98095 if I2C select SND_SOC_MAX9850 if I2C + select SND_SOC_MAX9768 if I2C select SND_SOC_MAX9877 if I2C select SND_SOC_PCM3008 select SND_SOC_RT5631 if I2C @@ -425,6 +426,9 @@ config SND_SOC_WM9713 config SND_SOC_LM4857 tristate
+config SND_SOC_MAX9768 + tristate + config SND_SOC_MAX9877 tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index de80781..cf3555b 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -25,6 +25,7 @@ snd-soc-dmic-objs := dmic.o snd-soc-jz4740-codec-objs := jz4740.o snd-soc-l3-objs := l3.o snd-soc-lm4857-objs := lm4857.o +snd-soc-max9768-objs := max9768.o snd-soc-max98088-objs := max98088.o snd-soc-max98095-objs := max98095.o snd-soc-max9850-objs := max9850.o @@ -129,6 +130,7 @@ obj-$(CONFIG_SND_SOC_DMIC) += snd-soc-dmic.o obj-$(CONFIG_SND_SOC_L3) += snd-soc-l3.o obj-$(CONFIG_SND_SOC_LM4857) += snd-soc-lm4857.o obj-$(CONFIG_SND_SOC_JZ4740_CODEC) += snd-soc-jz4740-codec.o +obj-$(CONFIG_SND_SOC_MAX9768) += snd-soc-max9768.o obj-$(CONFIG_SND_SOC_MAX98088) += snd-soc-max98088.o obj-$(CONFIG_SND_SOC_MAX98095) += snd-soc-max98095.o obj-$(CONFIG_SND_SOC_MAX9850) += snd-soc-max9850.o diff --git a/sound/soc/codecs/max9768.c b/sound/soc/codecs/max9768.c new file mode 100644 index 0000000..6348995 --- /dev/null +++ b/sound/soc/codecs/max9768.c @@ -0,0 +1,253 @@ +/* + * MAX9768 AMP driver + * + * Copyright (C) 2011, 2012 by Wolfram Sang, Pengutronix e.K. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; version 2 of the License. + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/slab.h> +#include <linux/gpio.h> +#include <linux/regmap.h> + +#include <sound/core.h> +#include <sound/soc.h> +#include <sound/tlv.h> +#include <sound/max9768.h> + +/* "Registers" */ +#define MAX9768_VOL 0 +#define MAX9768_CTRL 3 + +/* Commands */ +#define MAX9768_CTRL_PWM 0x15 +#define MAX9768_CTRL_FILTERLESS 0x16 + +struct max9768 { + struct regmap *regmap; + int mute_gpio; + int shdn_gpio; + u32 flags; +}; + +static struct reg_default max9768_default_regs[] = { + { 0, 0 }, + { 3, MAX9768_CTRL_FILTERLESS}, +}; + +static int max9768_get_gpio(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct max9768 *max9768 = snd_soc_codec_get_drvdata(codec); + int val = gpio_get_value_cansleep(max9768->mute_gpio); + + ucontrol->value.integer.value[0] = !val; + + return 0; +} + +static int max9768_set_gpio(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct max9768 *max9768 = snd_soc_codec_get_drvdata(codec); + + gpio_set_value_cansleep(max9768->mute_gpio, !ucontrol->value.integer.value[0]); + + return 0; +} + +static const unsigned int volume_tlv[] = { + TLV_DB_RANGE_HEAD(43), + 0, 0, TLV_DB_SCALE_ITEM(-16150, 0, 0), + 1, 1, TLV_DB_SCALE_ITEM(-9280, 0, 0), + 2, 2, TLV_DB_SCALE_ITEM(-9030, 0, 0), + 3, 3, TLV_DB_SCALE_ITEM(-8680, 0, 0), + 4, 4, TLV_DB_SCALE_ITEM(-8430, 0, 0), + 5, 5, TLV_DB_SCALE_ITEM(-8080, 0, 0), + 6, 6, TLV_DB_SCALE_ITEM(-7830, 0, 0), + 7, 7, TLV_DB_SCALE_ITEM(-7470, 0, 0), + 8, 8, TLV_DB_SCALE_ITEM(-7220, 0, 0), + 9, 9, TLV_DB_SCALE_ITEM(-6870, 0, 0), + 10, 10, TLV_DB_SCALE_ITEM(-6620, 0, 0), + 11, 11, TLV_DB_SCALE_ITEM(-6270, 0, 0), + 12, 12, TLV_DB_SCALE_ITEM(-6020, 0, 0), + 13, 13, TLV_DB_SCALE_ITEM(-5670, 0, 0), + 14, 14, TLV_DB_SCALE_ITEM(-5420, 0, 0), + 15, 17, TLV_DB_SCALE_ITEM(-5060, 250, 0), + 18, 18, TLV_DB_SCALE_ITEM(-4370, 0, 0), + 19, 19, TLV_DB_SCALE_ITEM(-4210, 0, 0), + 20, 20, TLV_DB_SCALE_ITEM(-3960, 0, 0), + 21, 21, TLV_DB_SCALE_ITEM(-3760, 0, 0), + 22, 22, TLV_DB_SCALE_ITEM(-3600, 0, 0), + 23, 23, TLV_DB_SCALE_ITEM(-3340, 0, 0), + 24, 24, TLV_DB_SCALE_ITEM(-3150, 0, 0), + 25, 25, TLV_DB_SCALE_ITEM(-2980, 0, 0), + 26, 26, TLV_DB_SCALE_ITEM(-2720, 0, 0), + 27, 27, TLV_DB_SCALE_ITEM(-2520, 0, 0), + 28, 30, TLV_DB_SCALE_ITEM(-2350, 190, 0), + 31, 31, TLV_DB_SCALE_ITEM(-1750, 0, 0), + 32, 34, TLV_DB_SCALE_ITEM(-1640, 100, 0), + 35, 37, TLV_DB_SCALE_ITEM(-1310, 110, 0), + 38, 39, TLV_DB_SCALE_ITEM(-990, 100, 0), + 40, 40, TLV_DB_SCALE_ITEM(-710, 0, 0), + 41, 41, TLV_DB_SCALE_ITEM(-600, 0, 0), + 42, 42, TLV_DB_SCALE_ITEM(-500, 0, 0), + 43, 43, TLV_DB_SCALE_ITEM(-340, 0, 0), + 44, 44, TLV_DB_SCALE_ITEM(-190, 0, 0), + 45, 45, TLV_DB_SCALE_ITEM(-50, 0, 0), + 46, 46, TLV_DB_SCALE_ITEM(50, 0, 0), + 47, 50, TLV_DB_SCALE_ITEM(120, 40, 0), + 51, 57, TLV_DB_SCALE_ITEM(290, 50, 0), + 58, 58, TLV_DB_SCALE_ITEM(650, 0, 0), + 59, 62, TLV_DB_SCALE_ITEM(700, 60, 0), + 63, 63, TLV_DB_SCALE_ITEM(950, 0, 0), +}; + +static const struct snd_kcontrol_new max9768_volume[] = { + SOC_SINGLE_TLV("Playback Volume", MAX9768_VOL, 0, 63, 0, volume_tlv), +}; + +static const struct snd_kcontrol_new max9768_mute[] = { + SOC_SINGLE_BOOL_EXT("Mute Switch", 0, max9768_get_gpio, max9768_set_gpio), +}; + +static int max9768_probe(struct snd_soc_codec *codec) +{ + struct max9768 *max9768 = snd_soc_codec_get_drvdata(codec); + int ret; + + codec->control_data = max9768->regmap; + ret = snd_soc_codec_set_cache_io(codec, 2, 6, SND_SOC_REGMAP); + if (ret) + return ret; + + if (max9768->flags & MAX9768_FLAG_CLASSIC_PWM) { + ret = snd_soc_write(codec, MAX9768_CTRL, MAX9768_CTRL_PWM); + if (ret) + return ret; + } + + if (gpio_is_valid(max9768->mute_gpio)) { + ret = snd_soc_add_controls(codec, max9768_mute, + ARRAY_SIZE(max9768_mute)); + if (ret) + return ret; + } + + return 0; +} + +static struct snd_soc_codec_driver max9768_codec_driver = { + .probe = max9768_probe, + .controls = max9768_volume, + .num_controls = ARRAY_SIZE(max9768_volume), +}; + +static bool max9768_always_false(struct device *dev, unsigned int reg) +{ + return false; +} + +static const struct regmap_config max9768_i2c_regmap_config = { + .reg_bits = 2, + .val_bits = 6, + .max_register = 3, + .reg_defaults = max9768_default_regs, + .num_reg_defaults = ARRAY_SIZE(max9768_default_regs), + .volatile_reg = max9768_always_false, + .cache_type = REGCACHE_RBTREE, +}; + +static int __devinit max9768_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct max9768 *max9768; + struct max9768_pdata *pdata = client->dev.platform_data; + int err; + + max9768 = devm_kzalloc(&client->dev, sizeof(*max9768), GFP_KERNEL); + if (!max9768) + return -ENOMEM; + + if (pdata) { + /* Mute on powerup to avoid clicks */ + err = gpio_request_one(pdata->mute_gpio, GPIOF_INIT_HIGH, "MAX9768 Mute"); + max9768->mute_gpio = err ?: pdata->mute_gpio; + + /* Activate chip by releasing shutdown, enables I2C */ + err = gpio_request_one(pdata->shdn_gpio, GPIOF_INIT_HIGH, "MAX9768 Shutdown"); + max9768->shdn_gpio = err ?: pdata->shdn_gpio; + + max9768->flags = pdata->flags; + } else { + max9768->shdn_gpio = -EINVAL; + max9768->mute_gpio = -EINVAL; + } + + i2c_set_clientdata(client, max9768); + + max9768->regmap = regmap_init_i2c(client, &max9768_i2c_regmap_config); + if (IS_ERR(max9768->regmap)) { + err = PTR_ERR(max9768->regmap); + goto err_gpio_free; + } + + err = snd_soc_register_codec(&client->dev, &max9768_codec_driver, NULL, 0); + if (err) + goto err_regmap_free; + + return 0; + + err_regmap_free: + regmap_exit(max9768->regmap); + err_gpio_free: + if (gpio_is_valid(max9768->shdn_gpio)) + gpio_free(max9768->shdn_gpio); + if (gpio_is_valid(max9768->mute_gpio)) + gpio_free(max9768->mute_gpio); + + return err; +} + +static int __devexit max9768_i2c_remove(struct i2c_client *client) +{ + struct max9768 *max9768 = i2c_get_clientdata(client); + + snd_soc_unregister_codec(&client->dev); + regmap_exit(max9768->regmap); + + if (gpio_is_valid(max9768->shdn_gpio)) + gpio_free(max9768->shdn_gpio); + if (gpio_is_valid(max9768->mute_gpio)) + gpio_free(max9768->mute_gpio); + + return 0; +} + +static const struct i2c_device_id max9768_i2c_id[] = { + { "max9768", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, max9768_i2c_id); + +static struct i2c_driver max9768_i2c_driver = { + .driver = { + .name = "max9768", + .owner = THIS_MODULE, + }, + .probe = max9768_i2c_probe, + .remove = __devexit_p(max9768_i2c_remove), + .id_table = max9768_i2c_id, +}; +module_i2c_driver(max9768_i2c_driver); + +MODULE_AUTHOR("Wolfram Sang w.sang@pengutronix.de"); +MODULE_DESCRIPTION("ASoC MAX9768 amplifier driver"); +MODULE_LICENSE("GPL v2");
On Fri, Jan 27, 2012 at 04:10:23PM +0100, Wolfram Sang wrote:
Mark: Still no DAPM support. We don't have PM on this board, and since I couldn't easily figure what is expected from me here, I'd like to leave this for people who have real interest in that.
I don't understand what was unclear here...
You should at least use set_bias_level() to manage the shutdown GPIO I guess.
+static const struct snd_kcontrol_new max9768_volume[] = {
- SOC_SINGLE_TLV("Playback Volume", MAX9768_VOL, 0, 63, 0, volume_tlv),
+};
+static const struct snd_kcontrol_new max9768_mute[] = {
- SOC_SINGLE_BOOL_EXT("Mute Switch", 0, max9768_get_gpio, max9768_set_gpio),
+};
Should be "Playback Switch" to match the volume control - that way userspace can present a single control to applications.
+static bool max9768_always_false(struct device *dev, unsigned int reg) +{
- return false;
+}
This should be the default in regmap for formats that use format_write (and hence can't read back), please fix in regmap rather than add this.
- if (pdata) {
/* Mute on powerup to avoid clicks */err = gpio_request_one(pdata->mute_gpio, GPIOF_INIT_HIGH, "MAX9768 Mute");
This need to avoid clicks is the reason I was asking for the DAPM stuff.
/* Activate chip by releasing shutdown, enables I2C */err = gpio_request_one(pdata->shdn_gpio, GPIOF_INIT_HIGH, "MAX9768 Shutdown");max9768->shdn_gpio = err ?: pdata->shdn_gpio;
Eeew.
On Fri, Jan 27, 2012 at 04:10:23PM +0100, Wolfram Sang wrote:
Add a driver supporting the volume control and the mute pin. Shdn pin and DAPM are not taken care of yet.
Given your comments about being out of time I went ahead and applied this, I'll do a patch fixing up the main review comments I had on top of it (the Mute Switch name and the always_false thing which you now fixed in the regmap core).
On Fri, Jan 27, 2012 at 04:10:21PM +0100, Wolfram Sang wrote:
For the upcoming 2/6-format, we don't see debugfs output otherwise, since the current division results in 0. I'd think 10/14 is broken currently, too.
This looks good but could you please rebase on top of topic/core, it conflicts with the support for padding bytes. It's not really entirely incorrect, the bytes values aren't terribly well defined for register sizes that aren't integer numbers of bytes as they were never intended to be used then. The assumption did leak out into debugfs though.
On Fri, Jan 27, 2012 at 04:28:37PM +0000, Mark Brown wrote:
On Fri, Jan 27, 2012 at 04:10:21PM +0100, Wolfram Sang wrote:
For the upcoming 2/6-format, we don't see debugfs output otherwise, since the current division results in 0. I'd think 10/14 is broken currently, too.
This looks good but could you please rebase on top of topic/core, it conflicts with the support for padding bytes. It's not really entirely incorrect, the bytes values aren't terribly well defined for register sizes that aren't integer numbers of bytes as they were never intended to be used then. The assumption did leak out into debugfs though.
I wondered if you'd save reg_bits and val_bits there (converting to *_bytes when needed), you could replace all the format_x_y-functions with a few generic functions doing (reg << y) | val. Unsure if it is worth it...
On Sat, Jan 28, 2012 at 02:14:03AM +0100, Wolfram Sang wrote:
I wondered if you'd save reg_bits and val_bits there (converting to *_bytes when needed), you could replace all the format_x_y-functions with a few generic functions doing (reg << y) | val. Unsure if it is worth it...
Clearly we *can* but there's other bits (chiefly the cache code once we acquire the ability to do block operations from cache) which really want stuff to be 8 bit aligned. I deliberately chose to make things byte aligned so we have to think about this stuff before we do things that make byte aligned stuff hard, it felt like if we were going to take a complexity hit we probably want it to be on the odd register formats as pretty much all of the more demanding modern devices seem to be going for multiples of 8 bits.
Clearly we *can* but there's other bits (chiefly the cache code once we acquire the ability to do block operations from cache) which really want stuff to be 8 bit aligned. I deliberately chose to make things byte aligned so we have to think about this stuff before we do things that make byte aligned stuff hard, it felt like if we were going to take a complexity hit we probably want it to be on the odd register formats as pretty much all of the more demanding modern devices seem to be going for multiples of 8 bits.
Understood. I'd think we will find a non-intrusive way when a more generic format_x_y-function seems worthwhile.
On Mon, Jan 30, 2012 at 02:35:29PM +0100, Wolfram Sang wrote:
Understood. I'd think we will find a non-intrusive way when a more generic format_x_y-function seems worthwhile.
Probably either your suggestion of just storing the shifts in there will be it or it'll just turn out that there aren't actually enough odd formats to make it worth worrying about the hard coding. I was making this decision before we had the cache or anything like that so was thinking about what made sense for facilitating the infrastructure work.
participants (2)
-
Mark Brown -
Wolfram Sang