[alsa-devel] [PATCH 00/10] ASoC: sta32x: Add device tree binding
This patchset is the result of comparing the sta32x driver with the sta350 driver with the goal to make the sta32x driver device tree capable. In the merging process the coding style of the sta350 driver was adopted since it seemed more up to date. I did my best to break up the changes in logical chunks. Additionally some small fixes were added.
The comparison with the sta350 lead to the following changes: - Switch to direct usage of the regmap API - Use the kernel gpio framework to control the reset pin of the sta32x - adopt calculation of mlck divider and extrapolation ratio from sta350 - add device tree bindings - Change the Codecs DAI name to be coherent with the sta350 driver
Moreover some minor fixes were added: - Add the status register - Fix bit shift value for the IDE bit in the CONFF register - Add description for the driver and I2C dependency in Kconfig
Thomas Niederprüm (10): ASoC: sta32x: Convert to direct regmap API usage. ASoC: sta32x: make sta32x a gpio consumer for the reset GPIO ASoC: sta32x: use DECLARE_TLV_DB_RANGE macro. ASoC: sta32x: add status register. ASoC: sta32x: move code to calculate mclk divider and extrapolation ratio to sta32x_hw_params() ASoC: sta32x: add device tree binding. ASoC: sta32x: use dev_dbg() for debug output. ASoC: sta32x: minor Kconfig update. ASoC: sta32x: correct bit shift value for IDE register. ASoC: sta32x: change dai name to be in line with the sta350 driver.
.../devicetree/bindings/sound/st,sta32x.txt | 92 ++++ include/sound/sta32x.h | 18 +- sound/soc/codecs/Kconfig | 4 +- sound/soc/codecs/sta32x.c | 537 +++++++++++++-------- sound/soc/codecs/sta32x.h | 4 +- 5 files changed, 450 insertions(+), 205 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/st,sta32x.txt
use the regmap API directly rather than relying on the snd_soc_read/write functions as this seems to be in accordance with common practice.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/Kconfig | 1 + sound/soc/codecs/sta32x.c | 271 ++++++++++++++++++++++++++-------------------- 2 files changed, 152 insertions(+), 120 deletions(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index a68d173..a2683a0 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -558,6 +558,7 @@ config SND_SOC_SSM4567
config SND_SOC_STA32X tristate + select REGMAP_I2C
config SND_SOC_STA350 tristate "STA350 speaker amplifier" diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 4874085..03175fe 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -102,6 +102,35 @@ static const struct reg_default sta32x_regs[] = { { 0x2c, 0x0c }, };
+static const struct regmap_range sta32x_write_regs_range[] = { + regmap_reg_range(STA32X_CONFA, STA32X_AUTO2), + regmap_reg_range(STA32X_C1CFG, STA32X_FDRC2), +}; + +static const struct regmap_range sta32x_read_regs_range[] = { + regmap_reg_range(STA32X_CONFA, STA32X_AUTO2), + regmap_reg_range(STA32X_C1CFG, STA32X_FDRC2), +}; + +static const struct regmap_range sta32x_volatile_regs_range[] = { + regmap_reg_range(STA32X_CFADDR2, STA32X_CFUD), +}; + +static const struct regmap_access_table sta32x_write_regs = { + .yes_ranges = sta32x_write_regs_range, + .n_yes_ranges = ARRAY_SIZE(sta32x_write_regs_range), +}; + +static const struct regmap_access_table sta32x_read_regs = { + .yes_ranges = sta32x_read_regs_range, + .n_yes_ranges = ARRAY_SIZE(sta32x_read_regs_range), +}; + +static const struct regmap_access_table sta32x_volatile_regs = { + .yes_ranges = sta32x_volatile_regs_range, + .n_yes_ranges = ARRAY_SIZE(sta32x_volatile_regs_range), +}; + /* regulator power supply names */ static const char *sta32x_supply_names[] = { "Vdda", /* analog supply, 3.3VV */ @@ -122,6 +151,7 @@ struct sta32x_priv { u32 coef_shadow[STA32X_COEF_COUNT]; struct delayed_work watchdog_work; int shutdown; + struct mutex coeff_lock; };
static const DECLARE_TLV_DB_SCALE(mvol_tlv, -12700, 50, 1); @@ -244,29 +274,42 @@ static int sta32x_coefficient_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol); + struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); int numcoef = kcontrol->private_value >> 16; int index = kcontrol->private_value & 0xffff; - unsigned int cfud; - int i; + unsigned int cfud, val; + int i, ret = 0; + + mutex_lock(&sta32x->coeff_lock);
/* preserve reserved bits in STA32X_CFUD */ - cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0; - /* chip documentation does not say if the bits are self clearing, - * so do it explicitly */ - snd_soc_write(codec, STA32X_CFUD, cfud); + regmap_read(sta32x->regmap, STA32X_CFUD, &cfud); + cfud &= 0xf0; + /* + * chip documentation does not say if the bits are self clearing, + * so do it explicitly + */ + regmap_write(sta32x->regmap, STA32X_CFUD, cfud);
- snd_soc_write(codec, STA32X_CFADDR2, index); - if (numcoef == 1) - snd_soc_write(codec, STA32X_CFUD, cfud | 0x04); - else if (numcoef == 5) - snd_soc_write(codec, STA32X_CFUD, cfud | 0x08); - else - return -EINVAL; - for (i = 0; i < 3 * numcoef; i++) - ucontrol->value.bytes.data[i] = - snd_soc_read(codec, STA32X_B1CF1 + i); + regmap_write(sta32x->regmap, STA32X_CFADDR2, index); + if (numcoef == 1) { + regmap_write(sta32x->regmap, STA32X_CFUD, cfud | 0x04); + } else if (numcoef == 5) { + regmap_write(sta32x->regmap, STA32X_CFUD, cfud | 0x08); + } else { + ret = -EINVAL; + goto exit_unlock; + }
- return 0; + for (i = 0; i < 3 * numcoef; i++) { + regmap_read(sta32x->regmap, STA32X_B1CF1 + i, &val); + ucontrol->value.bytes.data[i] = val; + } + +exit_unlock: + mutex_unlock(&sta32x->coeff_lock); + + return ret; }
static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol, @@ -280,24 +323,27 @@ static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol, int i;
/* preserve reserved bits in STA32X_CFUD */ - cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0; - /* chip documentation does not say if the bits are self clearing, - * so do it explicitly */ - snd_soc_write(codec, STA32X_CFUD, cfud); + regmap_read(sta32x->regmap, STA32X_CFUD, &cfud); + cfud &= 0xf0; + /* + * chip documentation does not say if the bits are self clearing, + * so do it explicitly + */ + regmap_write(sta32x->regmap, STA32X_CFUD, cfud);
- snd_soc_write(codec, STA32X_CFADDR2, index); + regmap_write(sta32x->regmap, STA32X_CFADDR2, index); for (i = 0; i < numcoef && (index + i < STA32X_COEF_COUNT); i++) sta32x->coef_shadow[index + i] = (ucontrol->value.bytes.data[3 * i] << 16) | (ucontrol->value.bytes.data[3 * i + 1] << 8) | (ucontrol->value.bytes.data[3 * i + 2]); for (i = 0; i < 3 * numcoef; i++) - snd_soc_write(codec, STA32X_B1CF1 + i, - ucontrol->value.bytes.data[i]); + regmap_write(sta32x->regmap, STA32X_B1CF1 + i, + ucontrol->value.bytes.data[i]); if (numcoef == 1) - snd_soc_write(codec, STA32X_CFUD, cfud | 0x01); + regmap_write(sta32x->regmap, STA32X_CFUD, cfud | 0x01); else if (numcoef == 5) - snd_soc_write(codec, STA32X_CFUD, cfud | 0x02); + regmap_write(sta32x->regmap, STA32X_CFUD, cfud | 0x02); else return -EINVAL;
@@ -311,20 +357,23 @@ static int sta32x_sync_coef_shadow(struct snd_soc_codec *codec) int i;
/* preserve reserved bits in STA32X_CFUD */ - cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0; + regmap_read(sta32x->regmap, STA32X_CFUD, &cfud); + cfud &= 0xf0;
for (i = 0; i < STA32X_COEF_COUNT; i++) { - snd_soc_write(codec, STA32X_CFADDR2, i); - snd_soc_write(codec, STA32X_B1CF1, - (sta32x->coef_shadow[i] >> 16) & 0xff); - snd_soc_write(codec, STA32X_B1CF2, - (sta32x->coef_shadow[i] >> 8) & 0xff); - snd_soc_write(codec, STA32X_B1CF3, - (sta32x->coef_shadow[i]) & 0xff); - /* chip documentation does not say if the bits are - * self-clearing, so do it explicitly */ - snd_soc_write(codec, STA32X_CFUD, cfud); - snd_soc_write(codec, STA32X_CFUD, cfud | 0x01); + regmap_write(sta32x->regmap, STA32X_CFADDR2, i); + regmap_write(sta32x->regmap, STA32X_B1CF1, + (sta32x->coef_shadow[i] >> 16) & 0xff); + regmap_write(sta32x->regmap, STA32X_B1CF2, + (sta32x->coef_shadow[i] >> 8) & 0xff); + regmap_write(sta32x->regmap, STA32X_B1CF3, + (sta32x->coef_shadow[i]) & 0xff); + /* + * chip documentation does not say if the bits are + * self-clearing, so do it explicitly + */ + regmap_write(sta32x->regmap, STA32X_CFUD, cfud); + regmap_write(sta32x->regmap, STA32X_CFUD, cfud | 0x01); } return 0; } @@ -336,11 +385,11 @@ static int sta32x_cache_sync(struct snd_soc_codec *codec) int rc;
/* mute during register sync */ - mute = snd_soc_read(codec, STA32X_MMUTE); - snd_soc_write(codec, STA32X_MMUTE, mute | STA32X_MMUTE_MMUTE); + regmap_read(sta32x->regmap, STA32X_MMUTE, &mute); + regmap_write(sta32x->regmap, STA32X_MMUTE, mute | STA32X_MMUTE_MMUTE); sta32x_sync_coef_shadow(codec); rc = regcache_sync(sta32x->regmap); - snd_soc_write(codec, STA32X_MMUTE, mute); + regmap_write(sta32x->regmap, STA32X_MMUTE, mute); return rc; }
@@ -599,10 +648,7 @@ static int sta32x_set_dai_fmt(struct snd_soc_dai *codec_dai, { struct snd_soc_codec *codec = codec_dai->codec; struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); - u8 confb = snd_soc_read(codec, STA32X_CONFB); - - pr_debug("\n"); - confb &= ~(STA32X_CONFB_C1IM | STA32X_CONFB_C2IM); + u8 confb = 0;
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBS_CFS: @@ -632,8 +678,8 @@ static int sta32x_set_dai_fmt(struct snd_soc_dai *codec_dai, return -EINVAL; }
- snd_soc_write(codec, STA32X_CONFB, confb); - return 0; + return regmap_update_bits(sta32x->regmap, STA32X_CONFB, + STA32X_CONFB_C1IM | STA32X_CONFB_C2IM, confb); }
/** @@ -653,7 +699,7 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream, struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); unsigned int rate; int i, mcs = -1, ir = -1; - u8 confa, confb; + unsigned int confa, confb;
rate = params_rate(params); pr_debug("rate: %u\n", rate); @@ -672,12 +718,10 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream, if (mcs < 0) return -EINVAL;
- confa = snd_soc_read(codec, STA32X_CONFA); - confa &= ~(STA32X_CONFA_MCS_MASK | STA32X_CONFA_IR_MASK); - confa |= (ir << STA32X_CONFA_IR_SHIFT) | (mcs << STA32X_CONFA_MCS_SHIFT); + confa = (ir << STA32X_CONFA_IR_SHIFT) | + (mcs << STA32X_CONFA_MCS_SHIFT); + confb = 0;
- confb = snd_soc_read(codec, STA32X_CONFB); - confb &= ~(STA32X_CONFB_SAI_MASK | STA32X_CONFB_SAIFB); switch (params_width(params)) { case 24: pr_debug("24bit\n"); @@ -746,8 +790,20 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- snd_soc_write(codec, STA32X_CONFA, confa); - snd_soc_write(codec, STA32X_CONFB, confb); + ret = regmap_update_bits(sta32x->regmap, STA32X_CONFA, + STA32X_CONFA_MCS_MASK | STA32X_CONFA_IR_MASK, + confa); + if (ret < 0) + return ret; + + ret = regmap_update_bits(sta32x->regmap, STA32X_CONFB, + STA32X_CONFB_SAI_MASK | STA32X_CONFB_SAIFB, + confb); + if (ret < 0) + return ret; + + return 0; +} return 0; }
@@ -773,7 +829,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec,
case SND_SOC_BIAS_PREPARE: /* Full power on */ - snd_soc_update_bits(codec, STA32X_CONFF, + regmap_update_bits(sta32x->regmap, STA32X_CONFF, STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, STA32X_CONFF_PWDN | STA32X_CONFF_EAPD); break; @@ -792,19 +848,17 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, sta32x_watchdog_start(sta32x); }
- /* Power up to mute */ - /* FIXME */ - snd_soc_update_bits(codec, STA32X_CONFF, - STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, - STA32X_CONFF_PWDN | STA32X_CONFF_EAPD); + /* Power down */ + regmap_update_bits(sta32x->regmap, STA32X_CONFF, + STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, + 0);
break;
case SND_SOC_BIAS_OFF: /* The chip runs through the power down sequence for us. */ - snd_soc_update_bits(codec, STA32X_CONFF, - STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, - STA32X_CONFF_PWDN); + regmap_update_bits(sta32x->regmap, STA32X_CONFF, + STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, 0); msleep(300); sta32x_watchdog_stop(sta32x); regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), @@ -853,11 +907,8 @@ static int sta32x_resume(struct snd_soc_codec *codec) static int sta32x_probe(struct snd_soc_codec *codec) { struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); + struct sta32x_platform_data *pdata = sta32x->pdata; int i, ret = 0, thermal = 0; - - sta32x->codec = codec; - sta32x->pdata = dev_get_platdata(codec->dev); - ret = regulator_bulk_enable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); if (ret != 0) { @@ -865,50 +916,34 @@ static int sta32x_probe(struct snd_soc_codec *codec) return ret; }
- /* Chip documentation explicitly requires that the reset values - * of reserved register bits are left untouched. - * Write the register default value to cache for reserved registers, - * so the write to the these registers are suppressed by the cache - * restore code when it skips writes of default registers. - */ - regcache_cache_only(sta32x->regmap, true); - snd_soc_write(codec, STA32X_CONFC, 0xc2); - snd_soc_write(codec, STA32X_CONFE, 0xc2); - snd_soc_write(codec, STA32X_CONFF, 0x5c); - snd_soc_write(codec, STA32X_MMUTE, 0x10); - snd_soc_write(codec, STA32X_AUTO1, 0x60); - snd_soc_write(codec, STA32X_AUTO3, 0x00); - snd_soc_write(codec, STA32X_C3CFG, 0x40); - regcache_cache_only(sta32x->regmap, false); - /* set thermal warning adjustment and recovery */ - if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_ADJUSTMENT_ENABLE)) + if (!pdata->thermal_warning_recovery) thermal |= STA32X_CONFA_TWAB; - if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_RECOVERY_ENABLE)) + if (!pdata->thermal_warning_adjustment) thermal |= STA32X_CONFA_TWRB; - snd_soc_update_bits(codec, STA32X_CONFA, - STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, - thermal); + regmap_update_bits(sta32x->regmap, STA32X_CONFA, + STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, + thermal);
/* select output configuration */ - snd_soc_update_bits(codec, STA32X_CONFF, - STA32X_CONFF_OCFG_MASK, - sta32x->pdata->output_conf - << STA32X_CONFF_OCFG_SHIFT); + regmap_update_bits(sta32x->regmap, STA32X_CONFF, + STA32X_CONFF_OCFG_MASK, + pdata->output_conf + << STA32X_CONFF_OCFG_SHIFT);
/* channel to output mapping */ - snd_soc_update_bits(codec, STA32X_C1CFG, - STA32X_CxCFG_OM_MASK, - sta32x->pdata->ch1_output_mapping - << STA32X_CxCFG_OM_SHIFT); - snd_soc_update_bits(codec, STA32X_C2CFG, - STA32X_CxCFG_OM_MASK, - sta32x->pdata->ch2_output_mapping - << STA32X_CxCFG_OM_SHIFT); - snd_soc_update_bits(codec, STA32X_C3CFG, - STA32X_CxCFG_OM_MASK, - sta32x->pdata->ch3_output_mapping - << STA32X_CxCFG_OM_SHIFT); + regmap_update_bits(sta32x->regmap, STA32X_C1CFG, + STA32X_CxCFG_OM_MASK, + pdata->ch1_output_mapping + << STA32X_CxCFG_OM_SHIFT); + regmap_update_bits(sta32x->regmap, STA32X_C2CFG, + STA32X_CxCFG_OM_MASK, + pdata->ch2_output_mapping + << STA32X_CxCFG_OM_SHIFT); + regmap_update_bits(sta32x->regmap, STA32X_C3CFG, + STA32X_CxCFG_OM_MASK, + pdata->ch3_output_mapping + << STA32X_CxCFG_OM_SHIFT);
/* initialize coefficient shadow RAM with reset values */ for (i = 4; i <= 49; i += 5) @@ -942,16 +977,6 @@ static int sta32x_remove(struct snd_soc_codec *codec) return 0; }
-static bool sta32x_reg_is_volatile(struct device *dev, unsigned int reg) -{ - switch (reg) { - case STA32X_CONFA ... STA32X_L2ATRT: - case STA32X_MPCC1 ... STA32X_FDRC2: - return 0; - } - return 1; -} - static const struct snd_soc_codec_driver sta32x_codec = { .probe = sta32x_probe, .remove = sta32x_remove, @@ -973,12 +998,16 @@ static const struct regmap_config sta32x_regmap = { .reg_defaults = sta32x_regs, .num_reg_defaults = ARRAY_SIZE(sta32x_regs), .cache_type = REGCACHE_RBTREE, - .volatile_reg = sta32x_reg_is_volatile, + .wr_table = &sta32x_write_regs, + .rd_table = &sta32x_read_regs, + .volatile_table = &sta32x_volatile_regs, +}; };
static int sta32x_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { + struct device *dev = &i2c->dev; struct sta32x_priv *sta32x; int ret, i;
@@ -987,6 +1016,8 @@ static int sta32x_i2c_probe(struct i2c_client *i2c, if (!sta32x) return -ENOMEM;
+ mutex_init(&sta32x->coeff_lock); + sta32x->pdata = dev_get_platdata(dev); /* regulators */ for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) sta32x->supplies[i].supply = sta32x_supply_names[i]; @@ -1001,15 +1032,15 @@ static int sta32x_i2c_probe(struct i2c_client *i2c, sta32x->regmap = devm_regmap_init_i2c(i2c, &sta32x_regmap); if (IS_ERR(sta32x->regmap)) { ret = PTR_ERR(sta32x->regmap); - dev_err(&i2c->dev, "Failed to init regmap: %d\n", ret); + dev_err(dev, "Failed to init regmap: %d\n", ret); return ret; }
i2c_set_clientdata(i2c, sta32x);
- ret = snd_soc_register_codec(&i2c->dev, &sta32x_codec, &sta32x_dai, 1); - if (ret != 0) - dev_err(&i2c->dev, "Failed to register codec (%d)\n", ret); + ret = snd_soc_register_codec(dev, &sta32x_codec, &sta32x_dai, 1); + if (ret < 0) + dev_err(dev, "Failed to register codec (%d)\n", ret);
return ret; }
The reset GPIO on the STA32X Codecs is used to reset the Codec and clear all registers. Also taking it down puts the IC in power save mode, so we put the device in reset mode when we go to sleep.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 03175fe..78f2009 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -26,6 +26,7 @@ #include <linux/i2c.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/gpio/consumer.h> #include <linux/slab.h> #include <linux/workqueue.h> #include <sound/core.h> @@ -151,6 +152,7 @@ struct sta32x_priv { u32 coef_shadow[STA32X_COEF_COUNT]; struct delayed_work watchdog_work; int shutdown; + struct gpio_desc *gpiod_nreset; struct mutex coeff_lock; };
@@ -804,6 +806,16 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream,
return 0; } + +static int sta32x_startup_sequence(struct sta32x_priv *sta32x) +{ + if (sta32x->gpiod_nreset) { + gpiod_set_value(sta32x->gpiod_nreset, 0); + mdelay(1); + gpiod_set_value(sta32x->gpiod_nreset, 1); + mdelay(1); + } + return 0; }
@@ -844,6 +856,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, return ret; }
+ sta32x_startup_sequence(sta32x); sta32x_cache_sync(codec); sta32x_watchdog_start(sta32x); } @@ -861,6 +874,10 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, 0); msleep(300); sta32x_watchdog_stop(sta32x); + + if (sta32x->gpiod_nreset) + gpiod_set_value(sta32x->gpiod_nreset, 0); + regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); break; @@ -916,6 +933,11 @@ static int sta32x_probe(struct snd_soc_codec *codec) return ret; }
+ ret = sta32x_startup_sequence(sta32x); + if (ret < 0) { + dev_err(codec->dev, "Failed to startup device\n"); + return ret; + } /* set thermal warning adjustment and recovery */ if (!pdata->thermal_warning_recovery) thermal |= STA32X_CONFA_TWAB; @@ -1018,6 +1040,19 @@ static int sta32x_i2c_probe(struct i2c_client *i2c,
mutex_init(&sta32x->coeff_lock); sta32x->pdata = dev_get_platdata(dev); + + /* GPIOs */ + sta32x->gpiod_nreset = devm_gpiod_get(dev, "reset"); + if (IS_ERR(sta32x->gpiod_nreset)) { + ret = PTR_ERR(sta32x->gpiod_nreset); + if (ret != -ENOENT && ret != -ENOSYS) + return ret; + + sta32x->gpiod_nreset = NULL; + } else { + gpiod_direction_output(sta32x->gpiod_nreset, 0); + } + /* regulators */ for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) sta32x->supplies[i].supply = sta32x_supply_names[i];
On Thu, Jan 22, 2015 at 12:01:54AM +0100, Thomas Niederprüm wrote:
- /* GPIOs */
- sta32x->gpiod_nreset = devm_gpiod_get(dev, "reset");
- if (IS_ERR(sta32x->gpiod_nreset)) {
ret = PTR_ERR(sta32x->gpiod_nreset);if (ret != -ENOENT && ret != -ENOSYS)return ret;sta32x->gpiod_nreset = NULL;
I'll apply this but in general this pattern of replacing error pointers with NULL is a bit suspect - if the way you test for failure is IS_ERR() then just carry on using IS_ERR() all through the code to check if you got something valid, it's less likely for GPIOs but most other things can substitute in dummies and NULL can be a good way of specifying a dummy.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 78f2009..d2f6bed 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -187,37 +187,32 @@ static const char *sta32x_limiter_release_rate[] = { "0.5116", "0.1370", "0.0744", "0.0499", "0.0360", "0.0299", "0.0264", "0.0208", "0.0198", "0.0172", "0.0147", "0.0137", "0.0134", "0.0117", "0.0110", "0.0104" }; - -static const unsigned int sta32x_limiter_ac_attack_tlv[] = { - TLV_DB_RANGE_HEAD(2), +static DECLARE_TLV_DB_RANGE(sta32x_limiter_ac_attack_tlv, 0, 7, TLV_DB_SCALE_ITEM(-1200, 200, 0), 8, 16, TLV_DB_SCALE_ITEM(300, 100, 0), -}; +);
-static const unsigned int sta32x_limiter_ac_release_tlv[] = { - TLV_DB_RANGE_HEAD(5), +static DECLARE_TLV_DB_RANGE(sta32x_limiter_ac_release_tlv, 0, 0, TLV_DB_SCALE_ITEM(TLV_DB_GAIN_MUTE, 0, 0), 1, 1, TLV_DB_SCALE_ITEM(-2900, 0, 0), 2, 2, TLV_DB_SCALE_ITEM(-2000, 0, 0), 3, 8, TLV_DB_SCALE_ITEM(-1400, 200, 0), 8, 16, TLV_DB_SCALE_ITEM(-700, 100, 0), -}; +);
-static const unsigned int sta32x_limiter_drc_attack_tlv[] = { - TLV_DB_RANGE_HEAD(3), +static DECLARE_TLV_DB_RANGE(sta32x_limiter_drc_attack_tlv, 0, 7, TLV_DB_SCALE_ITEM(-3100, 200, 0), 8, 13, TLV_DB_SCALE_ITEM(-1600, 100, 0), 14, 16, TLV_DB_SCALE_ITEM(-1000, 300, 0), -}; +);
-static const unsigned int sta32x_limiter_drc_release_tlv[] = { - TLV_DB_RANGE_HEAD(5), +static DECLARE_TLV_DB_RANGE(sta32x_limiter_drc_release_tlv, 0, 0, TLV_DB_SCALE_ITEM(TLV_DB_GAIN_MUTE, 0, 0), 1, 2, TLV_DB_SCALE_ITEM(-3800, 200, 0), 3, 4, TLV_DB_SCALE_ITEM(-3300, 200, 0), 5, 12, TLV_DB_SCALE_ITEM(-3000, 200, 0), 13, 16, TLV_DB_SCALE_ITEM(-1500, 300, 0), -}; +);
static SOC_ENUM_SINGLE_DECL(sta32x_drc_ac_enum, STA32X_CONFD, STA32X_CONFD_DRC_SHIFT,
The sta32x defines a read-only status register at address 0x2d.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 5 +++-- sound/soc/codecs/sta32x.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index d2f6bed..77be160 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -101,6 +101,7 @@ static const struct reg_default sta32x_regs[] = { { 0x28, 0xc0 }, { 0x2b, 0x00 }, { 0x2c, 0x0c }, + { 0x2d, 0x0f }, };
static const struct regmap_range sta32x_write_regs_range[] = { @@ -110,7 +111,7 @@ static const struct regmap_range sta32x_write_regs_range[] = {
static const struct regmap_range sta32x_read_regs_range[] = { regmap_reg_range(STA32X_CONFA, STA32X_AUTO2), - regmap_reg_range(STA32X_C1CFG, STA32X_FDRC2), + regmap_reg_range(STA32X_C1CFG, STA32X_STATUS), };
static const struct regmap_range sta32x_volatile_regs_range[] = { @@ -1011,7 +1012,7 @@ static const struct snd_soc_codec_driver sta32x_codec = { static const struct regmap_config sta32x_regmap = { .reg_bits = 8, .val_bits = 8, - .max_register = STA32X_FDRC2, + .max_register = STA32X_STATUS, .reg_defaults = sta32x_regs, .num_reg_defaults = ARRAY_SIZE(sta32x_regs), .cache_type = REGCACHE_RBTREE, diff --git a/sound/soc/codecs/sta32x.h b/sound/soc/codecs/sta32x.h index d8e32a6..4ccc722 100644 --- a/sound/soc/codecs/sta32x.h +++ b/sound/soc/codecs/sta32x.h @@ -67,7 +67,7 @@ #define STA32X_Reserved 0x2a #define STA32X_FDRC1 0x2b #define STA32X_FDRC2 0x2c -/* Reserved 0x2d */ +#define STA32X_STATUS 0x2d
/* STA326 register field definitions */
On Thu, Jan 22, 2015 at 12:01:56AM +0100, Thomas Niederprüm wrote:
The sta32x defines a read-only status register at address 0x2d.
+++ b/sound/soc/codecs/sta32x.c @@ -101,6 +101,7 @@ static const struct reg_default sta32x_regs[] = { { 0x28, 0xc0 }, { 0x2b, 0x00 }, { 0x2c, 0x0c },
- { 0x2d, 0x0f },
};
If it's a status register I'd expect it to be volatile and not have a default, otherwise it's not terribly useful.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 87 +++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 55 deletions(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 77be160..4858194 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -555,17 +555,12 @@ static struct { };
/* MCLK to fs clock ratios */ -static struct { - int ratio; - int mcs; -} mclk_ratios[3][7] = { - { { 768, 0 }, { 512, 1 }, { 384, 2 }, { 256, 3 }, - { 128, 4 }, { 576, 5 }, { 0, 0 } }, - { { 384, 2 }, { 256, 3 }, { 192, 4 }, { 128, 5 }, {64, 0 }, { 0, 0 } }, - { { 384, 2 }, { 256, 3 }, { 192, 4 }, { 128, 5 }, {64, 0 }, { 0, 0 } }, +static int mcs_ratio_table[3][7] = { + { 768, 512, 384, 256, 128, 576, 0 }, + { 384, 256, 192, 128, 64, 0 }, + { 384, 256, 192, 128, 64, 0 }, };
- /** * sta32x_set_dai_sysclk - configure MCLK * @codec_dai: the codec DAI @@ -590,46 +585,10 @@ static int sta32x_set_dai_sysclk(struct snd_soc_dai *codec_dai, { struct snd_soc_codec *codec = codec_dai->codec; struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); - int i, j, ir, fs; - unsigned int rates = 0; - unsigned int rate_min = -1; - unsigned int rate_max = 0;
- pr_debug("mclk=%u\n", freq); + dev_dbg(codec->dev, "mclk=%u\n", freq); sta32x->mclk = freq;
- if (sta32x->mclk) { - for (i = 0; i < ARRAY_SIZE(interpolation_ratios); i++) { - ir = interpolation_ratios[i].ir; - fs = interpolation_ratios[i].fs; - for (j = 0; mclk_ratios[ir][j].ratio; j++) { - if (mclk_ratios[ir][j].ratio * fs == freq) { - rates |= snd_pcm_rate_to_rate_bit(fs); - if (fs < rate_min) - rate_min = fs; - if (fs > rate_max) - rate_max = fs; - break; - } - } - } - /* FIXME: soc should support a rate list */ - rates &= ~SNDRV_PCM_RATE_KNOT; - - if (!rates) { - dev_err(codec->dev, "could not find a valid sample rate\n"); - return -EINVAL; - } - } else { - /* enable all possible rates */ - rates = STA32X_RATES; - rate_min = 32000; - rate_max = 192000; - } - - codec_dai->driver->playback.rates = rates; - codec_dai->driver->playback.rate_min = rate_min; - codec_dai->driver->playback.rate_max = rate_max; return 0; }
@@ -695,26 +654,44 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream, { struct snd_soc_codec *codec = dai->codec; struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); - unsigned int rate; - int i, mcs = -1, ir = -1; + int i, mcs = -EINVAL, ir = -EINVAL; unsigned int confa, confb; + unsigned int rate, ratio; + int ret; + + if (!sta32x->mclk) { + dev_err(codec->dev, + "sta32x->mclk is unset. Unable to determine ratio\n"); + return -EIO; + }
rate = params_rate(params); - pr_debug("rate: %u\n", rate); - for (i = 0; i < ARRAY_SIZE(interpolation_ratios); i++) + ratio = sta32x->mclk / rate; + dev_dbg(codec->dev, "rate: %u, ratio: %u\n", rate, ratio); + + for (i = 0; i < ARRAY_SIZE(interpolation_ratios); i++) { if (interpolation_ratios[i].fs == rate) { ir = interpolation_ratios[i].ir; break; } - if (ir < 0) + } + + if (ir < 0) { + dev_err(codec->dev, "Unsupported samplerate: %u\n", rate); return -EINVAL; - for (i = 0; mclk_ratios[ir][i].ratio; i++) - if (mclk_ratios[ir][i].ratio * rate == sta32x->mclk) { - mcs = mclk_ratios[ir][i].mcs; + } + + for (i = 0; i < 6; i++) { + if (mcs_ratio_table[ir][i] == ratio) { + mcs = i; break; } - if (mcs < 0) + } + + if (mcs < 0) { + dev_err(codec->dev, "Unresolvable ratio: %u\n", ratio); return -EINVAL; + }
confa = (ir << STA32X_CONFA_IR_SHIFT) | (mcs << STA32X_CONFA_MCS_SHIFT);
make the sta32x driver usable with device tree configs. Code is heavily based on the sta350 driver.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- .../devicetree/bindings/sound/st,sta32x.txt | 92 ++++++++++++++++++ include/sound/sta32x.h | 18 +++- sound/soc/codecs/sta32x.c | 108 ++++++++++++++++++++- 3 files changed, 211 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/st,sta32x.txt
diff --git a/Documentation/devicetree/bindings/sound/st,sta32x.txt b/Documentation/devicetree/bindings/sound/st,sta32x.txt new file mode 100644 index 0000000..255de3a --- /dev/null +++ b/Documentation/devicetree/bindings/sound/st,sta32x.txt @@ -0,0 +1,92 @@ +STA32X audio CODEC + +The driver for this device only supports I2C. + +Required properties: + + - compatible: "st,sta32x" + - reg: the I2C address of the device for I2C + - reset-gpios: a GPIO spec for the reset pin. If specified, it will be + deasserted before communication to the codec starts. + + - power-down-gpios: a GPIO spec for the power down pin. If specified, + it will be deasserted before communication to the codec + starts. + + - Vdda-supply: regulator spec, providing 3.3V + - Vdd3-supply: regulator spec, providing 3.3V + - Vcc-supply: regulator spec, providing 5V - 26V + +Optional properties: + + - st,output-conf: number, Selects the output configuration: + 0: 2-channel (full-bridge) power, 2-channel data-out + 1: 2 (half-bridge). 1 (full-bridge) on-board power + 2: 2 Channel (Full-Bridge) Power, 1 Channel FFX + 3: 1 Channel Mono-Parallel + If parameter is missing, mode 0 will be enabled. + This property has to be specified as '/bits/ 8' value. + + - st,ch1-output-mapping: Channel 1 output mapping + - st,ch2-output-mapping: Channel 2 output mapping + - st,ch3-output-mapping: Channel 3 output mapping + 0: Channel 1 + 1: Channel 2 + 2: Channel 3 + If parameter is missing, channel 1 is chosen. + This properties have to be specified as '/bits/ 8' values. + + - st,thermal-warning-recover: + If present, thermal warning recovery is enabled. + + - st,thermal-warning-adjustment: + If present, thermal warning adjustment is enabled. + + - st,fault-detect-recovery: + If present, then fault recovery will be enabled. + + - st,drop-compensation-ns: number + Only required for "st,ffx-power-output-mode" == + "variable-drop-compensation". + Specifies the drop compensation in nanoseconds. + The value must be in the range of 0..300, and only + multiples of 20 are allowed. Default is 140ns. + + - st,max-power-use-mpcc: + If present, then MPCC bits are used for MPC coefficients, + otherwise standard MPC coefficients are used. + + - st,max-power-corr: + If present, power bridge correction for THD reduction near maximum + power output is enabled. + + - st,am-reduction-mode: + If present, FFX mode runs in AM reduction mode, otherwise normal + FFX mode is used. + + - st,odd-pwm-speed-mode: + If present, PWM speed mode run on odd speed mode (341.3 kHz) on all + channels. If not present, normal PWM spped mode (384 kHz) will be used. + + - st,invalid-input-detect-mute: + If present, automatic invalid input detect mute is enabled. + +Example: + +codec: sta32x@38 { + compatible = "st,sta32x"; + reg = <0x1c>; + reset-gpios = <&gpio1 19 0>; + power-down-gpios = <&gpio1 16 0>; + st,output-conf = /bits/ 8 <0x3>; // set output to 2-channel + // (full-bridge) power, + // 2-channel data-out + st,ch1-output-mapping = /bits/ 8 <0>; // set channel 1 output ch 1 + st,ch2-output-mapping = /bits/ 8 <0>; // set channel 2 output ch 1 + st,ch3-output-mapping = /bits/ 8 <0>; // set channel 3 output ch 1 + st,max-power-correction; // enables power bridge + // correction for THD reduction + // near maximum power output + st,invalid-input-detect-mute; // mute if no valid digital + // audio signal is provided. +}; diff --git a/include/sound/sta32x.h b/include/sound/sta32x.h index 8d93b03..a894f7d 100644 --- a/include/sound/sta32x.h +++ b/include/sound/sta32x.h @@ -24,12 +24,20 @@ #define STA32X_THERMAL_RECOVERY_ENABLE 2
struct sta32x_platform_data { - int output_conf; - int ch1_output_mapping; - int ch2_output_mapping; - int ch3_output_mapping; - int thermal_conf; + u8 output_conf; + u8 ch1_output_mapping; + u8 ch2_output_mapping; + u8 ch3_output_mapping; int needs_esd_watchdog; + u8 drop_compensation_ns; + unsigned int thermal_warning_recovery:1; + unsigned int thermal_warning_adjustment:1; + unsigned int fault_detect_recovery:1; + unsigned int max_power_use_mpcc:1; + unsigned int max_power_correction:1; + unsigned int am_reduction_mode:1; + unsigned int odd_pwm_speed_mode:1; + unsigned int invalid_input_detect_mute:1; };
#endif /* __LINUX_SND__STA32X_H */ diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 4858194..f12628e 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -24,6 +24,8 @@ #include <linux/delay.h> #include <linux/pm.h> #include <linux/i2c.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/gpio/consumer.h> @@ -911,15 +913,49 @@ static int sta32x_probe(struct snd_soc_codec *codec) dev_err(codec->dev, "Failed to startup device\n"); return ret; } - /* set thermal warning adjustment and recovery */ + + /* CONFA */ if (!pdata->thermal_warning_recovery) thermal |= STA32X_CONFA_TWAB; if (!pdata->thermal_warning_adjustment) thermal |= STA32X_CONFA_TWRB; + if (!pdata->fault_detect_recovery) + thermal |= STA32X_CONFA_FDRB; regmap_update_bits(sta32x->regmap, STA32X_CONFA, - STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, + STA32X_CONFA_TWAB | STA32X_CONFA_TWRB | + STA32X_CONFA_FDRB, thermal);
+ /* CONFC */ + regmap_update_bits(sta32x->regmap, STA32X_CONFC, + STA32X_CONFC_CSZ_MASK, + pdata->drop_compensation_ns + << STA32X_CONFC_CSZ_SHIFT); + + /* CONFE */ + regmap_update_bits(sta32x->regmap, STA32X_CONFE, + STA32X_CONFE_MPCV, + pdata->max_power_use_mpcc ? + STA32X_CONFE_MPCV : 0); + regmap_update_bits(sta32x->regmap, STA32X_CONFE, + STA32X_CONFE_MPC, + pdata->max_power_correction ? + STA32X_CONFE_MPC : 0); + regmap_update_bits(sta32x->regmap, STA32X_CONFE, + STA32X_CONFE_AME, + pdata->am_reduction_mode ? + STA32X_CONFE_AME : 0); + regmap_update_bits(sta32x->regmap, STA32X_CONFE, + STA32X_CONFE_PWMS, + pdata->odd_pwm_speed_mode ? + STA32X_CONFE_PWMS : 0); + + /* CONFF */ + regmap_update_bits(sta32x->regmap, STA32X_CONFF, + STA32X_CONFF_IDE, + pdata->invalid_input_detect_mute ? + STA32X_CONFF_IDE : 0); + /* select output configuration */ regmap_update_bits(sta32x->regmap, STA32X_CONFF, STA32X_CONFF_OCFG_MASK, @@ -997,7 +1033,66 @@ static const struct regmap_config sta32x_regmap = { .rd_table = &sta32x_read_regs, .volatile_table = &sta32x_volatile_regs, }; + +#ifdef CONFIG_OF +static const struct of_device_id st32x_dt_ids[] = { + { .compatible = "st,sta32x", }, + { } }; +MODULE_DEVICE_TABLE(of, st32x_dt_ids); + +static int sta32x_probe_dt(struct device *dev, struct sta32x_priv *sta32x) +{ + struct device_node *np = dev->of_node; + struct sta32x_platform_data *pdata; + u16 tmp; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + of_property_read_u8(np, "st,output-conf", + &pdata->output_conf); + of_property_read_u8(np, "st,ch1-output-mapping", + &pdata->ch1_output_mapping); + of_property_read_u8(np, "st,ch2-output-mapping", + &pdata->ch2_output_mapping); + of_property_read_u8(np, "st,ch3-output-mapping", + &pdata->ch3_output_mapping); + + if (of_get_property(np, "st,thermal-warning-recovery", NULL)) + pdata->thermal_warning_recovery = 1; + if (of_get_property(np, "st,thermal-warning-adjustment", NULL)) + pdata->thermal_warning_adjustment = 1; + if (of_get_property(np, "st,needs_esd_watchdog", NULL)) + pdata->needs_esd_watchdog = 1; + + tmp = 140; + of_property_read_u16(np, "st,drop-compensation-ns", &tmp); + pdata->drop_compensation_ns = clamp_t(u16, tmp, 0, 300) / 20; + + /* CONFE */ + if (of_get_property(np, "st,max-power-use-mpcc", NULL)) + pdata->max_power_use_mpcc = 1; + + if (of_get_property(np, "st,max-power-correction", NULL)) + pdata->max_power_correction = 1; + + if (of_get_property(np, "st,am-reduction-mode", NULL)) + pdata->am_reduction_mode = 1; + + if (of_get_property(np, "st,odd-pwm-speed-mode", NULL)) + pdata->odd_pwm_speed_mode = 1; + + /* CONFF */ + if (of_get_property(np, "st,invalid-input-detect-mute", NULL)) + pdata->invalid_input_detect_mute = 1; + + sta32x->pdata = pdata; + + return 0; +} +#endif
static int sta32x_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) @@ -1014,6 +1109,14 @@ static int sta32x_i2c_probe(struct i2c_client *i2c, mutex_init(&sta32x->coeff_lock); sta32x->pdata = dev_get_platdata(dev);
+#ifdef CONFIG_OF + if (dev->of_node) { + ret = sta32x_probe_dt(dev, sta32x); + if (ret < 0) + return ret; + } +#endif + /* GPIOs */ sta32x->gpiod_nreset = devm_gpiod_get(dev, "reset"); if (IS_ERR(sta32x->gpiod_nreset)) { @@ -1071,6 +1174,7 @@ static struct i2c_driver sta32x_i2c_driver = { .driver = { .name = "sta32x", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(st32x_dt_ids), }, .probe = sta32x_i2c_probe, .remove = sta32x_i2c_remove,
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index f12628e..b2ce7cf 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -701,10 +701,10 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream,
switch (params_width(params)) { case 24: - pr_debug("24bit\n"); + dev_dbg(codec->dev, "24bit\n"); /* fall through */ case 32: - pr_debug("24bit or 32bit\n"); + dev_dbg(codec->dev, "24bit or 32bit\n"); switch (sta32x->format) { case SND_SOC_DAIFMT_I2S: confb |= 0x0; @@ -719,7 +719,7 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream,
break; case 20: - pr_debug("20bit\n"); + dev_dbg(codec->dev, "20bit\n"); switch (sta32x->format) { case SND_SOC_DAIFMT_I2S: confb |= 0x4; @@ -734,7 +734,7 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream,
break; case 18: - pr_debug("18bit\n"); + dev_dbg(codec->dev, "18bit\n"); switch (sta32x->format) { case SND_SOC_DAIFMT_I2S: confb |= 0x8; @@ -749,7 +749,7 @@ static int sta32x_hw_params(struct snd_pcm_substream *substream,
break; case 16: - pr_debug("16bit\n"); + dev_dbg(codec->dev, "16bit\n"); switch (sta32x->format) { case SND_SOC_DAIFMT_I2S: confb |= 0x0; @@ -809,7 +809,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, int ret; struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec);
- pr_debug("level = %d\n", level); + dev_dbg(codec->dev, "level = %d\n", level); switch (level) { case SND_SOC_BIAS_ON: break;
- Add description for the driver - Add dependency on the I2C module
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index a2683a0..1e09d32 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -557,7 +557,8 @@ config SND_SOC_SSM4567 depends on I2C
config SND_SOC_STA32X - tristate + tristate "STA326, STA328 and STA329 speaker amplifier" + depends on I2C select REGMAP_I2C
config SND_SOC_STA350
The IDE bit in the CONFF register is the third bit not the fourth.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/sta32x.h b/sound/soc/codecs/sta32x.h index 4ccc722..36fb371 100644 --- a/sound/soc/codecs/sta32x.h +++ b/sound/soc/codecs/sta32x.h @@ -131,7 +131,7 @@ #define STA32X_CONFF_OCFG_MASK 0x03 #define STA32X_CONFF_OCFG_SHIFT 0 #define STA32X_CONFF_IDE 0x04 -#define STA32X_CONFF_IDE_SHIFT 3 +#define STA32X_CONFF_IDE_SHIFT 2 #define STA32X_CONFF_BCLE 0x08 #define STA32X_CONFF_ECLE 0x20 #define STA32X_CONFF_PWDN 0x40
On Thu, Jan 22, 2015 at 12:02:01AM +0100, Thomas Niederprüm wrote:
The IDE bit in the CONFF register is the third bit not the fourth.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de
This is a bug fix, it should have been sent at the start of the series - doing this ensures there are no dependencies on feature additions so fixes can be sent to Linus and stable if required.
Signed-off-by: Thomas Niederprüm niederp@physik.uni-kl.de --- sound/soc/codecs/sta32x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index b2ce7cf..bc6edb1 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -868,7 +868,7 @@ static const struct snd_soc_dai_ops sta32x_dai_ops = { };
static struct snd_soc_dai_driver sta32x_dai = { - .name = "STA32X", + .name = "sta32x-hifi", .playback = { .stream_name = "Playback", .channels_min = 2,
On 01/22/2015 12:01 AM, Thomas Niederprüm wrote:
This patchset is the result of comparing the sta32x driver with the sta350 driver with the goal to make the sta32x driver device tree capable. In the merging process the coding style of the sta350 driver was adopted since it seemed more up to date. I did my best to break up the changes in logical chunks. Additionally some small fixes were added.
The comparison with the sta350 lead to the following changes:
- Switch to direct usage of the regmap API
- Use the kernel gpio framework to control the reset pin of the sta32x
- adopt calculation of mlck divider and extrapolation ratio from sta350
- add device tree bindings
- Change the Codecs DAI name to be coherent with the sta350 driver
Moreover some minor fixes were added:
- Add the status register
- Fix bit shift value for the IDE bit in the CONFF register
- Add description for the driver and I2C dependency in Kconfig
Good stuff.
Do you think we can actually merge the the two drivers into one. They seem to be 90% line-by-line identical.
Thanks, - Lars
On 01/22/2015 02:05 PM, Lars-Peter Clausen wrote:
On 01/22/2015 12:01 AM, Thomas Niederprüm wrote:
This patchset is the result of comparing the sta32x driver with the sta350 driver with the goal to make the sta32x driver device tree capable. In the merging process the coding style of the sta350 driver was adopted since it seemed more up to date. I did my best to break up the changes in logical chunks. Additionally some small fixes were added.
The comparison with the sta350 lead to the following changes:
- Switch to direct usage of the regmap API
- Use the kernel gpio framework to control the reset pin of the sta32x
- adopt calculation of mlck divider and extrapolation ratio from sta350
- add device tree bindings
- Change the Codecs DAI name to be coherent with the sta350 driver
Moreover some minor fixes were added:
- Add the status register
- Fix bit shift value for the IDE bit in the CONFF register
- Add description for the driver and I2C dependency in Kconfig
Good stuff.
Jup, looks all good to me, too. Thanks for working on this.
Do you think we can actually merge the the two drivers into one. They seem to be 90% line-by-line identical.
That could work, yes, but it's not straight forward. When I compared the register sets some months ago, they seem to resemble largely, but there were lots of subtle differences. All of them would need to be handled at runtime.
I think the only way to figure whether it's worth it is to do the merge and have a look :)
Daniel
Am Thu, 22 Jan 2015 14:12:57 +0100 schrieb Daniel Mack daniel@zonque.org:
On 01/22/2015 02:05 PM, Lars-Peter Clausen wrote:
On 01/22/2015 12:01 AM, Thomas Niederprüm wrote:
This patchset is the result of comparing the sta32x driver with the sta350 driver with the goal to make the sta32x driver device tree capable. In the merging process the coding style of the sta350 driver was adopted since it seemed more up to date. I did my best to break up the changes in logical chunks. Additionally some small fixes were added.
The comparison with the sta350 lead to the following changes:
- Switch to direct usage of the regmap API
- Use the kernel gpio framework to control the reset pin of the
sta32x
- adopt calculation of mlck divider and extrapolation ratio from
sta350
- add device tree bindings
- Change the Codecs DAI name to be coherent with the sta350 driver
Moreover some minor fixes were added:
- Add the status register
- Fix bit shift value for the IDE bit in the CONFF register
- Add description for the driver and I2C dependency in Kconfig
Good stuff.
Jup, looks all good to me, too. Thanks for working on this.
Do you think we can actually merge the the two drivers into one. They seem to be 90% line-by-line identical.
Indeed! Creating this patch-set involved a lot of copy and paste
That could work, yes, but it's not straight forward. When I compared the register sets some months ago, they seem to resemble largely, but there were lots of subtle differences. All of them would need to be handled at runtime.
I agree on all points.
I think the only way to figure whether it's worth it is to do the merge and have a look :)
I agree that it was sensible to get this merged first and then try to see if the drivers can be combined.
Daniel
On Thu, Jan 22, 2015 at 12:01:52AM +0100, Thomas Niederprüm wrote:
This patchset is the result of comparing the sta32x driver with the sta350 driver with the goal to make the sta32x driver device tree capable. In the merging process the coding style of the sta350 driver was adopted since it seemed more up to date. I did my best to break up the changes in logical chunks. Additionally some small fixes were added.
Applied all except for the patch adding the status register, thanks.
participants (4)
-
Daniel Mack -
Lars-Peter Clausen -
Mark Brown -
Thomas Niederprüm