Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1227178 > unrolled thread

[PATCH 0/3] ASoC: Add support for DA7219 audio codec

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] ASoC: Add support for DA7219 audio codec Adam Thomson <Adam.Thomson.Opensource@diasemi.com> - 2015-09-17 18:10 +0200
    Re: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver Mark Brown <broonie@kernel.org> - 2015-09-20 02:40 +0200
      RE: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> - 2015-09-21 17:10 +0200
        Re: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver Mark Brown <broonie@kernel.org> - 2015-09-21 21:30 +0200

#1227178 — [PATCH 0/3] ASoC: Add support for DA7219 audio codec

This patch set adds support for the DA7219 audio codec with built-in
advanced accessory detection functionality. Patch set includes codec driver,
associated DT bindings documentation and MAINTAINERS file updates to cover new
bindings.

Adam Thomson (3):
  ASoC: codecs: Add da7219 codec driver
  ASoC: da7219: Add bindings documentation for DA7219 audio codec
  MAINTAINERS: da7219: Add entry to cover DA7219 bindings document

 Documentation/devicetree/bindings/sound/da7219.txt |  106 ++
 MAINTAINERS                                        |    1 +
 include/sound/da7219-aad.h                         |   99 ++
 include/sound/da7219.h                             |   76 +
 sound/soc/codecs/Kconfig                           |    4 +
 sound/soc/codecs/Makefile                          |    2 +
 sound/soc/codecs/da7219-aad.c                      |  800 +++++++++
 sound/soc/codecs/da7219-aad.h                      |  209 +++
 sound/soc/codecs/da7219.c                          | 1877 ++++++++++++++++++++
 sound/soc/codecs/da7219.h                          |  807 +++++++++
 10 files changed, 3981 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/da7219.txt
 create mode 100644 include/sound/da7219-aad.h
 create mode 100644 include/sound/da7219.h
 create mode 100644 sound/soc/codecs/da7219-aad.c
 create mode 100644 sound/soc/codecs/da7219-aad.h
 create mode 100644 sound/soc/codecs/da7219.c
 create mode 100644 sound/soc/codecs/da7219.h

--
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1228801 — Re: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 17, 2015 at 05:01:14PM +0100, Adam Thomson wrote:

> +	do {
> +		statusa = snd_soc_read(codec, DA7219_ACCDET_STATUS_A);
> +		if (statusa & DA7219_MICBIAS_UP_STS_MASK)
> +			micbias_up = true;
> +	} while (!micbias_up);

This could go into an inifinite loop.

> +static void da7219_aad_hptest_work(struct work_struct *work)
> +{
> +	struct da7219_aad_priv *da7219_aad =
> +		container_of(work, struct da7219_aad_priv, hptest_work);
> +	struct snd_soc_codec *codec = da7219_aad->codec;
> +	struct da7219_priv *da7219 = snd_soc_codec_get_drvdata(codec);
> +
> +	u8 tonegen_cfg1, tonegen_cfg2, tonegen_onper;
> +	u16 tonegen_freq1, tonegen_freq_hptest;
> +	u8 hpl_gain, hpr_gain, dacl_gain, dacr_gain, dac_filters1, dac_filters4;
> +	u8 dac_filters5, cp_ctrl, routing_dac, dacl_ctrl, dacr_ctrl;
> +	u8 mixoutl_sel, mixoutr_sel, st_outfilt_1l, st_outfilt_1r;
> +	u8 mixoutl_ctrl, mixoutr_ctrl, hpl_ctrl, hpr_ctrl, accdet_cfg8;
> +	int report = 0;
> +
> +	/* Save current settings */

This is obviously a massive reconfiguration of the device.  I'm not
seeing anything here which prevents userspace coming in and change the
configuration while we're in this function, that would obviously have
serious issues.

I'm also wondering if this might be more elegantly implemented by going
into cache bypass mode, doing the test and then using a cache resync to
restore the initial configuration.  That will at least avoid issues with
updates adding a new register but not modifying it.

> +			if (statusa & DA7219_JACK_TYPE_STS_MASK) {
> +				report |= SND_JACK_HEADSET;
> +				mask |=	SND_JACK_HEADSET | SND_JACK_LINEOUT;
> +				schedule_work(&da7219_aad->btn_det_work);
> +			} else {
> +				schedule_work(&da7219_aad->hptest_work);
> +			}

Why are we scheduling work - we're already in thread context?

> +			/* Un-drive headphones/lineout */
> +			snd_soc_update_bits(codec, DA7219_HP_R_CTRL,
> +					    DA7219_HP_R_AMP_OE_MASK, 0);
> +			snd_soc_update_bits(codec, DA7219_HP_L_CTRL,
> +					    DA7219_HP_L_AMP_OE_MASK, 0);

This looks like DAPM?

> +static enum da7219_aad_jack_ins_deb da7219_aad_of_jack_ins_deb(u32 val)
> +{
> +	switch (val) {
> +	case 5:
> +		return DA7219_AAD_JACK_INS_DEB_5MS;
> +	case 10:
> +		return DA7219_AAD_JACK_INS_DEB_10MS;
> +	case 20:
> +		return DA7219_AAD_JACK_INS_DEB_20MS;
> +	case 50:
> +		return DA7219_AAD_JACK_INS_DEB_50MS;
> +	case 100:
> +		return DA7219_AAD_JACK_INS_DEB_100MS;
> +	case 200:
> +		return DA7219_AAD_JACK_INS_DEB_200MS;
> +	case 500:
> +		return DA7219_AAD_JACK_INS_DEB_500MS;
> +	case 1000:
> +		return DA7219_AAD_JACK_INS_DEB_1S;
> +	default:
> +		return DA7219_AAD_JACK_INS_DEB_20MS;

This isn't an error?

> +/* Input/Output Enums */
> +static const char * const da7219_gain_ramp_rate_txt[] = {
> +	"nominal rate * 8", "nominal rate", "nominal rate / 8",
> +	"nominal rate / 16"
> +};

The ALSA ABI generally capitalises words.

> +/* ToneGen */
> +static int da7219_tonegen_freq_get(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
> +	struct da7219_priv *da7219 = snd_soc_codec_get_drvdata(codec);
> +	struct soc_mixer_control *mixer_ctrl =
> +		(struct soc_mixer_control *) kcontrol->private_value;
> +	unsigned int reg = mixer_ctrl->reg;
> +	u16 val;
> +	int ret;
> +
> +	ret = regmap_bulk_read(da7219->regmap, reg, &val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	ucontrol->value.integer.value[0] = le16_to_cpu(val);

This is *weird*.  We do a bulk read for a single register using an API
that returns CPU endian data then make it CPU endian (without any
annotations on variables...).  Why not use regmap_read()?  Why swap?
Why not use raw I/O?

> +static int da7219_hpf_put(struct snd_kcontrol *kcontrol,
> +			  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
> +	struct soc_enum *enum_ctrl = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int reg = enum_ctrl->reg;
> +	unsigned int sel = ucontrol->value.integer.value[0];
> +	unsigned int bits;
> +
> +	switch (sel) {
> +	case DA7219_HPF_MODE_DISABLED:
> +		bits = DA7219_HPF_DISABLED;
> +		break;
> +	case DA7219_HPF_MODE_AUDIO:
> +		bits = DA7219_HPF_AUDIO_EN;
> +		break;
> +	case DA7219_HPF_MODE_VOICE:
> +		bits = DA7219_HPF_VOICE_EN;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	snd_soc_update_bits(codec, reg, DA7219_HPF_MODE_MASK, bits);
> +
> +	return 0;
> +}

This looks like a standard enumeration with a simple mapping to the
register map?

> +
> +	/* ADCs */
> +	SOC_SINGLE_TLV("ADC Volume", DA7219_ADC_L_GAIN,
> +		       DA7219_ADC_L_DIGITAL_GAIN_SHIFT,
> +		       DA7219_ADC_L_DIGITAL_GAIN_MAX, DA7219_NO_INVERT,
> +		       da7219_adc_dig_gain_tlv),
> +	SOC_SINGLE("ADC Switch", DA7219_ADC_L_CTRL,
> +		   DA7219_ADC_L_MUTE_EN_SHIFT, DA7219_SWITCH_EN_MAX,
> +		   DA7219_INVERT),
> +	SOC_SINGLE("ADC Gain Ramp Switch", DA7219_ADC_L_CTRL,
> +		   DA7219_ADC_L_RAMP_EN_SHIFT, DA7219_SWITCH_EN_MAX,
> +		   DA7219_NO_INVERT),

Capture Digital rather than ADC.

> +	SOC_SINGLE_TLV("ALC Max Attenuation", DA7219_ALC_GAIN_LIMITS,
> +		       DA7219_ALC_ATTEN_MAX_SHIFT, DA7219_ALC_ATTEN_GAIN_MAX,
> +		       DA7219_NO_INVERT, da7219_alc_gain_tlv),
> +	SOC_SINGLE_TLV("ALC Max Gain", DA7219_ALC_GAIN_LIMITS,
> +		       DA7219_ALC_GAIN_MAX_SHIFT, DA7219_ALC_ATTEN_GAIN_MAX,
> +		       DA7219_NO_INVERT, da7219_alc_gain_tlv),
> +	SOC_SINGLE_RANGE_TLV("ALC Min Analog Gain", DA7219_ALC_ANA_GAIN_LIMITS,
> +			     DA7219_ALC_ANA_GAIN_MIN_SHIFT,
> +			     DA7219_ALC_ANA_GAIN_MIN, DA7219_ALC_ANA_GAIN_MAX,
> +			     DA7219_NO_INVERT, da7219_alc_ana_gain_tlv),
> +	SOC_SINGLE_RANGE_TLV("ALC Max Analog Gain", DA7219_ALC_ANA_GAIN_LIMITS,
> +			     DA7219_ALC_ANA_GAIN_MAX_SHIFT,
> +			     DA7219_ALC_ANA_GAIN_MIN, DA7219_ALC_ANA_GAIN_MAX,
> +			     DA7219_NO_INVERT, da7219_alc_ana_gain_tlv),

Volume not Gain.

> +	SOC_SINGLE("ToneGen DTMF Key", DA7219_TONE_GEN_CFG1,
> +		   DA7219_DTMF_REG_SHIFT, DA7219_DTMF_REG_MAX,
> +		   DA7219_NO_INVERT),

Should this be an enumeration with the DTMF digits in it (# and * aren't
numbers)?

> +	/* DACs */
> +	SOC_DOUBLE_R_TLV("DAC Volume", DA7219_DAC_L_GAIN, DA7219_DAC_R_GAIN,
> +			 DA7219_DAC_L_DIGITAL_GAIN_SHIFT,
> +			 DA7219_DAC_DIGITAL_GAIN_MAX, DA7219_NO_INVERT,
> +			 da7219_dac_dig_gain_tlv),
> +	SOC_DOUBLE_R("DAC Switch", DA7219_DAC_L_CTRL, DA7219_DAC_R_CTRL,
> +		     DA7219_DAC_L_MUTE_EN_SHIFT, DA7219_SWITCH_EN_MAX,
> +		     DA7219_INVERT),
> +	SOC_DOUBLE_R("DAC Gain Ramp Switch", DA7219_DAC_L_CTRL,
> +		     DA7219_DAC_R_CTRL, DA7219_DAC_L_RAMP_EN_SHIFT,
> +		     DA7219_SWITCH_EN_MAX, DA7219_NO_INVERT),


All the DAC controls should probably be Playback Digital instead.

> +
> +static int da7219_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> +				 int clk_id, unsigned int freq, int dir)
> +{
> +	struct snd_soc_codec *codec = codec_dai->codec;
> +	struct da7219_priv *da7219 = snd_soc_codec_get_drvdata(codec);
> +
> +	if (da7219->mclk_rate == freq)
> +		return 0;

Given that we also have source selection is this safe - should we also
be checking the source?

> +	if (da7219->mclk) {
> +		freq = clk_round_rate(da7219->mclk, freq);
> +		clk_set_rate(da7219->mclk, freq);
> +	}

Missing error checking.

> +			/* Internal LDO */
> +			if (da7219->use_int_ldo)
> +				snd_soc_update_bits(codec, DA7219_LDO_CTRL,
> +						    DA7219_LDO_EN_MASK,
> +						    DA7219_LDO_EN_MASK);

If there is an option to use an external supply I would expect to see
the regulator API used to discover the external LDO (and ideally also to
configure the integrated LDO).  If the driver works outside the
frameworks then it is likely this will lead to integration issues later
on.

> +		/* Internal LDO */
> +		da7219->use_int_ldo = pdata->use_internal_ldo;

This is likely to lead to surprises for example...

> +	/* Check if MCLK provided, if not the clock is NULL */
> +	da7219->mclk = devm_clk_get(codec->dev, "mclk");
> +	if (IS_ERR(da7219->mclk)) {
> +		if (PTR_ERR(da7219->mclk) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		da7219->mclk = NULL;
> +	}

This should specifically look for the error code that corresponds to a
clock not being mapped and only continue silently in that case rather
than silently accepting all failures to obtain a clock - if we silently
accept all failures that will mean that actual errors will be ignored.

> +	/*
> +	 * There are multiple control bits for the input mixer.
> +	 * The following can be enabled now as it's not power related.
> +	 */
> +	snd_soc_update_bits(codec, DA7219_MIXIN_L_CTRL,
> +			    DA7219_MIXIN_L_MIX_EN_MASK,
> +			    DA7219_MIXIN_L_MIX_EN_MASK);

So the chip designers just put these in for randomness?  Fun.  It'd be
more idiomatic to do something like making these supply widgets so
they're controlled via DAPM even if they don't matter much.

> +	else if (device_may_wakeup(codec->dev))
> +		disable_irq_wake(da7219->aad->irq);

You can use dev_pm_set_wake_irq() and skip having to manage this stuff
explicitly in the driver.

[toc] | [prev] | [next] | [standalone]

#1229450 — RE: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver

T24gU2VwdGVtYmVyIDE5LCAyMDE1IDE4OjQ0LCBNYXJrIEJyb3duIHdyb3RlOg0KDQo+ID4gKwlk
byB7DQo+ID4gKwkJc3RhdHVzYSA9IHNuZF9zb2NfcmVhZChjb2RlYywgREE3MjE5X0FDQ0RFVF9T
VEFUVVNfQSk7DQo+ID4gKwkJaWYgKHN0YXR1c2EgJiBEQTcyMTlfTUlDQklBU19VUF9TVFNfTUFT
SykNCj4gPiArCQkJbWljYmlhc191cCA9IHRydWU7DQo+ID4gKwl9IHdoaWxlICghbWljYmlhc191
cCk7DQo+IA0KPiBUaGlzIGNvdWxkIGdvIGludG8gYW4gaW5pZmluaXRlIGxvb3AuDQo+IA0KDQpP
bmx5IGlmIHRoZSBIVyBpcyB1bnJlc3BvbnNpdmUuIEhvd2V2ZXIsIGl0J3MgcHJvYmFibHkgYmV0
dGVyIHRvIGFkZCBhIHRpbWVvdXQNCmFuZCBzb21lIGRlYnVnIGluIHRoZSB1bmxpa2VseSBldmVu
dCB0aGF0IGRvZXMgaGFwcGVuLg0KDQo+ID4gK3N0YXRpYyB2b2lkIGRhNzIxOV9hYWRfaHB0ZXN0
X3dvcmsoc3RydWN0IHdvcmtfc3RydWN0ICp3b3JrKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgZGE3
MjE5X2FhZF9wcml2ICpkYTcyMTlfYWFkID0NCj4gPiArCQljb250YWluZXJfb2Yod29yaywgc3Ry
dWN0IGRhNzIxOV9hYWRfcHJpdiwgaHB0ZXN0X3dvcmspOw0KPiA+ICsJc3RydWN0IHNuZF9zb2Nf
Y29kZWMgKmNvZGVjID0gZGE3MjE5X2FhZC0+Y29kZWM7DQo+ID4gKwlzdHJ1Y3QgZGE3MjE5X3By
aXYgKmRhNzIxOSA9IHNuZF9zb2NfY29kZWNfZ2V0X2RydmRhdGEoY29kZWMpOw0KPiA+ICsNCj4g
PiArCXU4IHRvbmVnZW5fY2ZnMSwgdG9uZWdlbl9jZmcyLCB0b25lZ2VuX29ucGVyOw0KPiA+ICsJ
dTE2IHRvbmVnZW5fZnJlcTEsIHRvbmVnZW5fZnJlcV9ocHRlc3Q7DQo+ID4gKwl1OCBocGxfZ2Fp
biwgaHByX2dhaW4sIGRhY2xfZ2FpbiwgZGFjcl9nYWluLCBkYWNfZmlsdGVyczEsIGRhY19maWx0
ZXJzNDsNCj4gPiArCXU4IGRhY19maWx0ZXJzNSwgY3BfY3RybCwgcm91dGluZ19kYWMsIGRhY2xf
Y3RybCwgZGFjcl9jdHJsOw0KPiA+ICsJdTggbWl4b3V0bF9zZWwsIG1peG91dHJfc2VsLCBzdF9v
dXRmaWx0XzFsLCBzdF9vdXRmaWx0XzFyOw0KPiA+ICsJdTggbWl4b3V0bF9jdHJsLCBtaXhvdXRy
X2N0cmwsIGhwbF9jdHJsLCBocHJfY3RybCwgYWNjZGV0X2NmZzg7DQo+ID4gKwlpbnQgcmVwb3J0
ID0gMDsNCj4gPiArDQo+ID4gKwkvKiBTYXZlIGN1cnJlbnQgc2V0dGluZ3MgKi8NCj4gDQo+IFRo
aXMgaXMgb2J2aW91c2x5IGEgbWFzc2l2ZSByZWNvbmZpZ3VyYXRpb24gb2YgdGhlIGRldmljZS4g
IEknbSBub3QNCj4gc2VlaW5nIGFueXRoaW5nIGhlcmUgd2hpY2ggcHJldmVudHMgdXNlcnNwYWNl
IGNvbWluZyBpbiBhbmQgY2hhbmdlIHRoZQ0KPiBjb25maWd1cmF0aW9uIHdoaWxlIHdlJ3JlIGlu
IHRoaXMgZnVuY3Rpb24sIHRoYXQgd291bGQgb2J2aW91c2x5IGhhdmUNCj4gc2VyaW91cyBpc3N1
ZXMuDQo+IA0KPiBJJ20gYWxzbyB3b25kZXJpbmcgaWYgdGhpcyBtaWdodCBiZSBtb3JlIGVsZWdh
bnRseSBpbXBsZW1lbnRlZCBieSBnb2luZw0KPiBpbnRvIGNhY2hlIGJ5cGFzcyBtb2RlLCBkb2lu
ZyB0aGUgdGVzdCBhbmQgdGhlbiB1c2luZyBhIGNhY2hlIHJlc3luYyB0bw0KPiByZXN0b3JlIHRo
ZSBpbml0aWFsIGNvbmZpZ3VyYXRpb24uICBUaGF0IHdpbGwgYXQgbGVhc3QgYXZvaWQgaXNzdWVz
IHdpdGgNCj4gdXBkYXRlcyBhZGRpbmcgYSBuZXcgcmVnaXN0ZXIgYnV0IG5vdCBtb2RpZnlpbmcg
aXQuDQoNCkluIGEgc3lzdGVtIHNjZW5hcmlvIHRoZSBsaWtlbGlob29kIG9mIHRoYXQgaGFwcGVu
aW5nIGlzIHNtYWxsIGFzIHlvdQ0KY2Fubm90IHVzZSB0aGUgbWljIG9yIGhlYWRwaG9uZXMgdW50
aWwgdGhleSd2ZSBiZWVuIGluc2VydGVkLiBUaGUgc3lzdGVtIGlzDQpvbmx5IGxpa2VseSB0byBh
Y3QgYWZ0ZXIgdGhlIGphY2sgaW5zZXJ0aW9uIGV2ZW50cyBoYXZlIG9jY3VycmVkLiBIb3dldmVy
LCBpdA0Kd291bGQgYmUgYmV0dGVyIHRvIHByZXZlbnQgYW55IGNoYW5jZSBvZiBjb25jdXJyZW50
IGFjY2Vzcy4gVGhlIHByb2JsZW0gaXMgaG93DQpiZXN0IHRvIGxvY2sgdGhlIEtjb250cm9scyB3
aGlsc3QgdGhlIHRlc3QgcHJvY2VkdXJlIGluIHByb2dyZXNzLiBBdCB0aGUgbW9tZW50DQp0aGUg
b25seSB3YXkgSSBjYW4gc2VlIGlzIHRvIGFkZCBleHBsaWNpdCBjb250cm9sIHNldCgpIGZ1bmN0
aW9ucyB3aGljaCB3b3VsZA0KdXNlIGEgbG9jayB0aGF0IGNhbiBhbHNvIGJlIGNvbnRyb2xsZWQg
YnkgdGhlIEhQIHRlc3QgY29kZS4gRG9lcyB0aGlzIG1ha2Ugc2Vuc2UNCnRvIHlvdSBvciBkbyB5
b3Uga25vdyBvZiBhIHNpbXBsZXIgbWV0aG9kPyBPYnZpb3VzbHkgZGFwbSBoYXMgZnVuY3Rpb24g
dG8gbG9jaw0KYXMgcmVxdWlyZWQuDQoNCkZvciB0aGUgY2FjaGUgcmVzeW5jIGlkZWEsIGluIHRl
cm1zIG9mIGNvZGUsIGl0IHdpbGwgbG9vayBjbGVhbmVyLCBidXQgeW91IGFyZQ0KdGFsa2luZyBh
Ym91dCA0IHRvIDUgdGltZXMgdGhlIG51bWJlciBvZiBJMkMgYWNjZXNzZXMgdG8gdGhlIGRldmlj
ZSwgdG8gcmVzdG9yZQ0KY29uZmlndXJhdGlvbi4gRG9lcyB0aGF0IG5vdCBzZWVtIGxpa2UgYSBi
aXQgdG9vIG11Y2ggb3ZlcmhlYWQ/DQogDQo+ID4gKwkJCWlmIChzdGF0dXNhICYgREE3MjE5X0pB
Q0tfVFlQRV9TVFNfTUFTSykgew0KPiA+ICsJCQkJcmVwb3J0IHw9IFNORF9KQUNLX0hFQURTRVQ7
DQo+ID4gKwkJCQltYXNrIHw9CVNORF9KQUNLX0hFQURTRVQgfA0KPiBTTkRfSkFDS19MSU5FT1VU
Ow0KPiA+ICsJCQkJc2NoZWR1bGVfd29yaygmZGE3MjE5X2FhZC0+YnRuX2RldF93b3JrKTsNCj4g
PiArCQkJfSBlbHNlIHsNCj4gPiArCQkJCXNjaGVkdWxlX3dvcmsoJmRhNzIxOV9hYWQtPmhwdGVz
dF93b3JrKTsNCj4gPiArCQkJfQ0KPiANCj4gV2h5IGFyZSB3ZSBzY2hlZHVsaW5nIHdvcmsgLSB3
ZSdyZSBhbHJlYWR5IGluIHRocmVhZCBjb250ZXh0Pw0KDQpocHRlc3Qgd2lsbCB0YWtlIHNvbWUg
dGltZSB0byBjb21wbGV0ZSAob3ZlciAxMDBtcyksIGFuZCBpbiB0aGF0IHRpbWUgaXQncw0KcGxh
dXNpYmxlIHRoYXQgYSB1c2VyIGNvdWxkIHJlbW92ZSB0aGUgamFjay4gSWYgd2UgZGVhbCB3aXRo
IHRoaXMgaW4gdGhlIElSUQ0KdGhyZWFkLCB3ZSB3b24ndCBiZSBhd2FyZSBvZiBqYWNrIHJlbW92
YWwgZHVyaW5nIHRoZSBwcm9jZXNzLCBhbmQgd2lsbCBzZW5kIGENCnJlcG9ydCByZWdhcmRsZXNz
LCB3aGljaCB3aWxsIGFsbW9zdCBkZWZpbml0ZWx5IGJlIGluY29ycmVjdCwgYW5kIHVubmVjZXNz
YXJ5Lg0KQnkgc3Bhd25pbmcgb2ZmIHdvcmssIGl0IGFsbG93cyB0aGUgcmVtb3ZhbCB0byBiZSBk
ZWFsdCB3aXRoIGlmIHRoZSBocHRlc3Qgd29yaw0KcHJvY2VkdXJlIGlzIGN1cnJlbnRseSBpbiBw
cm9jZXNzLCBhbmQgdGhlbiB3ZSBjYW4gYXZvaWQgc2VuZGluZyBpbmNvcnJlY3QgamFjaw0KcmVw
b3J0cyBhdCB0aGUgZW5kLg0KDQpGb3IgYnV0dG9uIGRldGVjdGlvbiwgZm9yIGNlcnRhaW4gbWlj
cyBpdCdzIHJlcXVpcmVkIHRvIHB1bHNlIHRoZSBtaWNiaWFzIHRvIGENCmhpZ2hlciB2b2x0YWdl
LCBmb3IgYSBkZWZpbmVkIHBlcmlvZCBvZiB0aW1lLCB0byBlbmFibGUgdGhlIG1pYy4gQWdhaW4g
dGhpcw0KcGVyaW9kIGlzIGxpa2VseSB0byB0YWtlIG1heWJlIDEwMG1zLCBkZXBlbmRpbmcgb24g
dGhlIG1pYywgc28gaXQgbWFkZSBmYXIgbW9yZQ0Kc2Vuc2UgdG8gbWUgdG8gZG8gdGhpcyBvdXRz
aWRlIG9mIHRoZSBJUlEgdGhyZWFkLiBIb3dldmVyLCBpZiB5b3UgaGF2ZSBhIGJldHRlcg0KcHJv
cG9zYWwgZm9yIHRoaXMgdGhlbiBhbSBoYXBweSB0byB0YWtlIGl0IG9uIGJvYXJkLg0KIA0KPiA+
ICsJCQkvKiBVbi1kcml2ZSBoZWFkcGhvbmVzL2xpbmVvdXQgKi8NCj4gPiArCQkJc25kX3NvY191
cGRhdGVfYml0cyhjb2RlYywgREE3MjE5X0hQX1JfQ1RSTCwNCj4gPiArCQkJCQkgICAgREE3MjE5
X0hQX1JfQU1QX09FX01BU0ssIDApOw0KPiA+ICsJCQlzbmRfc29jX3VwZGF0ZV9iaXRzKGNvZGVj
LCBEQTcyMTlfSFBfTF9DVFJMLA0KPiA+ICsJCQkJCSAgICBEQTcyMTlfSFBfTF9BTVBfT0VfTUFT
SywgMCk7DQo+IA0KPiBUaGlzIGxvb2tzIGxpa2UgREFQTT8NCg0KVGhlIGNvbnRyb2wgb2YgZHJp
dmluZyB0aGUgaGVhZHBob25lcyBvciBtYWtpbmcgdGhlbSBoaWdoIGltcGVkYW5jZSBpcyBoYW5k
bGVkDQppbiB0aGUgQUFEIGNvZGUgYmVjYXVzZSB3ZSBjYW5ub3QgaGF2ZSB0aGUgaGVhZHBob25l
cyBkcml2ZW4gYmVmb3JlIGEgamFjayBpcw0KaW5zZXJ0ZWQgYXMgaXQgd2lsbCBhZmZlY3QgdGhl
IHBvbGUgZGV0ZWN0aW9uLiBBZGRpbmcgaXQgdG8gREFQTSBzZWVtZWQgbGlrZSBpdA0Kd291bGQg
Y2F1c2UgbW9yZSBwcm9ibGVtcyBpbiB0ZXJtcyBvZiBjb250cm9sbGluZyB3aGVuIGl0IHdvdWxk
IGFuZCB3b3VsZG4ndCBiZQ0KZW5hYmxlZC4NCiANCj4gPiArc3RhdGljIGVudW0gZGE3MjE5X2Fh
ZF9qYWNrX2luc19kZWIgZGE3MjE5X2FhZF9vZl9qYWNrX2luc19kZWIodTMyIHZhbCkNCj4gPiAr
ew0KPiA+ICsJc3dpdGNoICh2YWwpIHsNCj4gPiArCWNhc2UgNToNCj4gPiArCQlyZXR1cm4gREE3
MjE5X0FBRF9KQUNLX0lOU19ERUJfNU1TOw0KPiA+ICsJY2FzZSAxMDoNCj4gPiArCQlyZXR1cm4g
REE3MjE5X0FBRF9KQUNLX0lOU19ERUJfMTBNUzsNCj4gPiArCWNhc2UgMjA6DQo+ID4gKwkJcmV0
dXJuIERBNzIxOV9BQURfSkFDS19JTlNfREVCXzIwTVM7DQo+ID4gKwljYXNlIDUwOg0KPiA+ICsJ
CXJldHVybiBEQTcyMTlfQUFEX0pBQ0tfSU5TX0RFQl81ME1TOw0KPiA+ICsJY2FzZSAxMDA6DQo+
ID4gKwkJcmV0dXJuIERBNzIxOV9BQURfSkFDS19JTlNfREVCXzEwME1TOw0KPiA+ICsJY2FzZSAy
MDA6DQo+ID4gKwkJcmV0dXJuIERBNzIxOV9BQURfSkFDS19JTlNfREVCXzIwME1TOw0KPiA+ICsJ
Y2FzZSA1MDA6DQo+ID4gKwkJcmV0dXJuIERBNzIxOV9BQURfSkFDS19JTlNfREVCXzUwME1TOw0K
PiA+ICsJY2FzZSAxMDAwOg0KPiA+ICsJCXJldHVybiBEQTcyMTlfQUFEX0pBQ0tfSU5TX0RFQl8x
UzsNCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcmV0dXJuIERBNzIxOV9BQURfSkFDS19JTlNfREVC
XzIwTVM7DQo+IA0KPiBUaGlzIGlzbid0IGFuIGVycm9yPw0KDQpPcHRlZCBmb3IgSFcgZGVmYXVs
dCBpbiBjYXNlIG9mIGludmFsaWQgdmFsdWVzIHByb3ZpZGVkLiBNYXliZSBhIGRldl93YXJuKCkN
CndvdWxkIGJlIHVzZWZ1bCB0aG91Z2ggdG8gaW5kaWNhdGUgdGhpcyBpcyB0aGUgY2FzZT8NCiAN
Cj4gPiArLyogSW5wdXQvT3V0cHV0IEVudW1zICovDQo+ID4gK3N0YXRpYyBjb25zdCBjaGFyICog
Y29uc3QgZGE3MjE5X2dhaW5fcmFtcF9yYXRlX3R4dFtdID0gew0KPiA+ICsJIm5vbWluYWwgcmF0
ZSAqIDgiLCAibm9taW5hbCByYXRlIiwgIm5vbWluYWwgcmF0ZSAvIDgiLA0KPiA+ICsJIm5vbWlu
YWwgcmF0ZSAvIDE2Ig0KPiA+ICt9Ow0KPiANCj4gVGhlIEFMU0EgQUJJIGdlbmVyYWxseSBjYXBp
dGFsaXNlcyB3b3Jkcy4NCg0KT2ssIG5vIHByb2JsZW0uIFdpbGwgYWRqdXN0IGFjY29yZGluZ2x5
Lg0KIA0KPiA+ICsvKiBUb25lR2VuICovDQo+ID4gK3N0YXRpYyBpbnQgZGE3MjE5X3RvbmVnZW5f
ZnJlcV9nZXQoc3RydWN0IHNuZF9rY29udHJvbCAqa2NvbnRyb2wsDQo+ID4gKwkJCQkgICBzdHJ1
Y3Qgc25kX2N0bF9lbGVtX3ZhbHVlICp1Y29udHJvbCkNCj4gPiArew0KPiA+ICsJc3RydWN0IHNu
ZF9zb2NfY29kZWMgKmNvZGVjID0gc25kX3NvY19rY29udHJvbF9jb2RlYyhrY29udHJvbCk7DQo+
ID4gKwlzdHJ1Y3QgZGE3MjE5X3ByaXYgKmRhNzIxOSA9IHNuZF9zb2NfY29kZWNfZ2V0X2RydmRh
dGEoY29kZWMpOw0KPiA+ICsJc3RydWN0IHNvY19taXhlcl9jb250cm9sICptaXhlcl9jdHJsID0N
Cj4gPiArCQkoc3RydWN0IHNvY19taXhlcl9jb250cm9sICopIGtjb250cm9sLT5wcml2YXRlX3Zh
bHVlOw0KPiA+ICsJdW5zaWduZWQgaW50IHJlZyA9IG1peGVyX2N0cmwtPnJlZzsNCj4gPiArCXUx
NiB2YWw7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCXJldCA9IHJlZ21hcF9idWxrX3Jl
YWQoZGE3MjE5LT5yZWdtYXAsIHJlZywgJnZhbCwgc2l6ZW9mKHZhbCkpOw0KPiA+ICsJaWYgKHJl
dCkNCj4gPiArCQlyZXR1cm4gcmV0Ow0KPiA+ICsNCj4gPiArCXVjb250cm9sLT52YWx1ZS5pbnRl
Z2VyLnZhbHVlWzBdID0gbGUxNl90b19jcHUodmFsKTsNCj4gDQo+IFRoaXMgaXMgKndlaXJkKi4g
IFdlIGRvIGEgYnVsayByZWFkIGZvciBhIHNpbmdsZSByZWdpc3RlciB1c2luZyBhbiBBUEkNCj4g
dGhhdCByZXR1cm5zIENQVSBlbmRpYW4gZGF0YSB0aGVuIG1ha2UgaXQgQ1BVIGVuZGlhbiAod2l0
aG91dCBhbnkNCj4gYW5ub3RhdGlvbnMgb24gdmFyaWFibGVzLi4uKS4gIFdoeSBub3QgdXNlIHJl
Z21hcF9yZWFkKCk/ICBXaHkgc3dhcD8NCj4gV2h5IG5vdCB1c2UgcmF3IEkvTz8NCg0KVGhlIGRl
dmljZSBpcyA4LWJpdCByZWdpc3RlciBhY2Nlc3Mgb25seSwgYW5kIHRoZSB2YWx1ZSBzcGFucyB0
d28gcmVnaXN0ZXJzLA0KaGVuY2Ugd2h5IHRoaXMgaXMgZG9uZSBoZXJlLiBUaGUgcmVnaXN0ZXIg
ZGVmaW5lZCBmb3IgdGhlIEtjb250cm9sIGlzIHRoZSBmaXJzdA0KaW4gdGhlIHNlcXVlbmNlIG9m
IHR3byByZWdpc3RlcnMgKGZpcnN0IGxvd2VyIGJ5dGUsIHNlY29uZCB1cHBlciBieXRlKS4gVGhv
dWdodA0KdGhpcyB3YXMgY2xlYW5lciB0aGFuIGhhdmluZyB0d28gc2VwYXJhdGUgY29udHJvbHMg
dG8gY29uZmlndXJlIHVwcGVyIGFuZA0KbG93ZXIgYnl0ZXMuDQoNCj4gDQo+ID4gK3N0YXRpYyBp
bnQgZGE3MjE5X2hwZl9wdXQoc3RydWN0IHNuZF9rY29udHJvbCAqa2NvbnRyb2wsDQo+ID4gKwkJ
CSAgc3RydWN0IHNuZF9jdGxfZWxlbV92YWx1ZSAqdWNvbnRyb2wpDQo+ID4gK3sNCj4gPiArCXN0
cnVjdCBzbmRfc29jX2NvZGVjICpjb2RlYyA9IHNuZF9zb2Nfa2NvbnRyb2xfY29kZWMoa2NvbnRy
b2wpOw0KPiA+ICsJc3RydWN0IHNvY19lbnVtICplbnVtX2N0cmwgPSAoc3RydWN0IHNvY19lbnVt
ICopa2NvbnRyb2wtPnByaXZhdGVfdmFsdWU7DQo+ID4gKwl1bnNpZ25lZCBpbnQgcmVnID0gZW51
bV9jdHJsLT5yZWc7DQo+ID4gKwl1bnNpZ25lZCBpbnQgc2VsID0gdWNvbnRyb2wtPnZhbHVlLmlu
dGVnZXIudmFsdWVbMF07DQo+ID4gKwl1bnNpZ25lZCBpbnQgYml0czsNCj4gPiArDQo+ID4gKwlz
d2l0Y2ggKHNlbCkgew0KPiA+ICsJY2FzZSBEQTcyMTlfSFBGX01PREVfRElTQUJMRUQ6DQo+ID4g
KwkJYml0cyA9IERBNzIxOV9IUEZfRElTQUJMRUQ7DQo+ID4gKwkJYnJlYWs7DQo+ID4gKwljYXNl
IERBNzIxOV9IUEZfTU9ERV9BVURJTzoNCj4gPiArCQliaXRzID0gREE3MjE5X0hQRl9BVURJT19F
TjsNCj4gPiArCQlicmVhazsNCj4gPiArCWNhc2UgREE3MjE5X0hQRl9NT0RFX1ZPSUNFOg0KPiA+
ICsJCWJpdHMgPSBEQTcyMTlfSFBGX1ZPSUNFX0VOOw0KPiA+ICsJCWJyZWFrOw0KPiA+ICsJZGVm
YXVsdDoNCj4gPiArCQlyZXR1cm4gLUVJTlZBTDsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwlzbmRf
c29jX3VwZGF0ZV9iaXRzKGNvZGVjLCByZWcsIERBNzIxOV9IUEZfTU9ERV9NQVNLLCBiaXRzKTsN
Cj4gPiArDQo+ID4gKwlyZXR1cm4gMDsNCj4gPiArfQ0KPiANCj4gVGhpcyBsb29rcyBsaWtlIGEg
c3RhbmRhcmQgZW51bWVyYXRpb24gd2l0aCBhIHNpbXBsZSBtYXBwaW5nIHRvIHRoZQ0KPiByZWdp
c3RlciBtYXA/DQo+IA0KDQpUaGUgYml0IHZhbHVlcyBhcmVuJ3Qgc2VxdWVudGlhbCBhcyBwZXIg
YSBub3JtYWwgZW51bWVyYXRpb24sIHdoaWNoIGlzIHdoeSBJDQphZGRlZCB0aGlzIGFwcHJvYWNo
IHRvIHNldHRpbmcgdGhlIGNvcnJlY3QgYml0cy4gSG93ZXZlciwgSSd2ZSBqdXN0IHNwb3R0ZWQg
dGhlDQpTT0NfVkFMVUVfRU5VTV9TSU5HTEUgbWFjcm8gd2hpY2ggbG9va3MgbGlrZSBpdCB3aWxs
IGRvIHRoZSBqb2IsIHNvIEknbGwgdXNlDQp0aGF0IGluc3RlYWQuIFRoYW5rcy4NCg0KPiA+ICsN
Cj4gPiArCS8qIEFEQ3MgKi8NCj4gPiArCVNPQ19TSU5HTEVfVExWKCJBREMgVm9sdW1lIiwgREE3
MjE5X0FEQ19MX0dBSU4sDQo+ID4gKwkJICAgICAgIERBNzIxOV9BRENfTF9ESUdJVEFMX0dBSU5f
U0hJRlQsDQo+ID4gKwkJICAgICAgIERBNzIxOV9BRENfTF9ESUdJVEFMX0dBSU5fTUFYLCBEQTcy
MTlfTk9fSU5WRVJULA0KPiA+ICsJCSAgICAgICBkYTcyMTlfYWRjX2RpZ19nYWluX3RsdiksDQo+
ID4gKwlTT0NfU0lOR0xFKCJBREMgU3dpdGNoIiwgREE3MjE5X0FEQ19MX0NUUkwsDQo+ID4gKwkJ
ICAgREE3MjE5X0FEQ19MX01VVEVfRU5fU0hJRlQsIERBNzIxOV9TV0lUQ0hfRU5fTUFYLA0KPiA+
ICsJCSAgIERBNzIxOV9JTlZFUlQpLA0KPiA+ICsJU09DX1NJTkdMRSgiQURDIEdhaW4gUmFtcCBT
d2l0Y2giLCBEQTcyMTlfQURDX0xfQ1RSTCwNCj4gPiArCQkgICBEQTcyMTlfQURDX0xfUkFNUF9F
Tl9TSElGVCwgREE3MjE5X1NXSVRDSF9FTl9NQVgsDQo+ID4gKwkJICAgREE3MjE5X05PX0lOVkVS
VCksDQo+IA0KPiBDYXB0dXJlIERpZ2l0YWwgcmF0aGVyIHRoYW4gQURDLg0KDQpPaywgZmluZS4g
SXMgdGhpcyBub3cgdGhlIGNvbW1vbiBuYW1pbmcgdG8gYmUgdXNlZCBmb3IgYWxsIGZ1dHVyZSBj
b2RlY3M/DQoNCj4gDQo+ID4gKwlTT0NfU0lOR0xFX1RMVigiQUxDIE1heCBBdHRlbnVhdGlvbiIs
IERBNzIxOV9BTENfR0FJTl9MSU1JVFMsDQo+ID4gKwkJICAgICAgIERBNzIxOV9BTENfQVRURU5f
TUFYX1NISUZULA0KPiBEQTcyMTlfQUxDX0FUVEVOX0dBSU5fTUFYLA0KPiA+ICsJCSAgICAgICBE
QTcyMTlfTk9fSU5WRVJULCBkYTcyMTlfYWxjX2dhaW5fdGx2KSwNCj4gPiArCVNPQ19TSU5HTEVf
VExWKCJBTEMgTWF4IEdhaW4iLCBEQTcyMTlfQUxDX0dBSU5fTElNSVRTLA0KPiA+ICsJCSAgICAg
ICBEQTcyMTlfQUxDX0dBSU5fTUFYX1NISUZULA0KPiBEQTcyMTlfQUxDX0FUVEVOX0dBSU5fTUFY
LA0KPiA+ICsJCSAgICAgICBEQTcyMTlfTk9fSU5WRVJULCBkYTcyMTlfYWxjX2dhaW5fdGx2KSwN
Cj4gPiArCVNPQ19TSU5HTEVfUkFOR0VfVExWKCJBTEMgTWluIEFuYWxvZyBHYWluIiwNCj4gREE3
MjE5X0FMQ19BTkFfR0FJTl9MSU1JVFMsDQo+ID4gKwkJCSAgICAgREE3MjE5X0FMQ19BTkFfR0FJ
Tl9NSU5fU0hJRlQsDQo+ID4gKwkJCSAgICAgREE3MjE5X0FMQ19BTkFfR0FJTl9NSU4sDQo+IERB
NzIxOV9BTENfQU5BX0dBSU5fTUFYLA0KPiA+ICsJCQkgICAgIERBNzIxOV9OT19JTlZFUlQsIGRh
NzIxOV9hbGNfYW5hX2dhaW5fdGx2KSwNCj4gPiArCVNPQ19TSU5HTEVfUkFOR0VfVExWKCJBTEMg
TWF4IEFuYWxvZyBHYWluIiwNCj4gREE3MjE5X0FMQ19BTkFfR0FJTl9MSU1JVFMsDQo+ID4gKwkJ
CSAgICAgREE3MjE5X0FMQ19BTkFfR0FJTl9NQVhfU0hJRlQsDQo+ID4gKwkJCSAgICAgREE3MjE5
X0FMQ19BTkFfR0FJTl9NSU4sDQo+IERBNzIxOV9BTENfQU5BX0dBSU5fTUFYLA0KPiA+ICsJCQkg
ICAgIERBNzIxOV9OT19JTlZFUlQsIGRhNzIxOV9hbGNfYW5hX2dhaW5fdGx2KSwNCj4gDQo+IFZv
bHVtZSBub3QgR2Fpbi4NCg0KRmluZSwgd2lsbCB1cGRhdGUuDQoNCj4gPiArCVNPQ19TSU5HTEUo
IlRvbmVHZW4gRFRNRiBLZXkiLCBEQTcyMTlfVE9ORV9HRU5fQ0ZHMSwNCj4gPiArCQkgICBEQTcy
MTlfRFRNRl9SRUdfU0hJRlQsIERBNzIxOV9EVE1GX1JFR19NQVgsDQo+ID4gKwkJICAgREE3MjE5
X05PX0lOVkVSVCksDQo+IA0KPiBTaG91bGQgdGhpcyBiZSBhbiBlbnVtZXJhdGlvbiB3aXRoIHRo
ZSBEVE1GIGRpZ2l0cyBpbiBpdCAoIyBhbmQgKiBhcmVuJ3QNCj4gbnVtYmVycyk/DQoNClllcywg
YWdyZWVkLiBXaWxsIHVwZGF0ZS4NCg0KPiA+ICsJLyogREFDcyAqLw0KPiA+ICsJU09DX0RPVUJM
RV9SX1RMVigiREFDIFZvbHVtZSIsIERBNzIxOV9EQUNfTF9HQUlOLA0KPiBEQTcyMTlfREFDX1Jf
R0FJTiwNCj4gPiArCQkJIERBNzIxOV9EQUNfTF9ESUdJVEFMX0dBSU5fU0hJRlQsDQo+ID4gKwkJ
CSBEQTcyMTlfREFDX0RJR0lUQUxfR0FJTl9NQVgsIERBNzIxOV9OT19JTlZFUlQsDQo+ID4gKwkJ
CSBkYTcyMTlfZGFjX2RpZ19nYWluX3RsdiksDQo+ID4gKwlTT0NfRE9VQkxFX1IoIkRBQyBTd2l0
Y2giLCBEQTcyMTlfREFDX0xfQ1RSTCwNCj4gREE3MjE5X0RBQ19SX0NUUkwsDQo+ID4gKwkJICAg
ICBEQTcyMTlfREFDX0xfTVVURV9FTl9TSElGVCwgREE3MjE5X1NXSVRDSF9FTl9NQVgsDQo+ID4g
KwkJICAgICBEQTcyMTlfSU5WRVJUKSwNCj4gPiArCVNPQ19ET1VCTEVfUigiREFDIEdhaW4gUmFt
cCBTd2l0Y2giLCBEQTcyMTlfREFDX0xfQ1RSTCwNCj4gPiArCQkgICAgIERBNzIxOV9EQUNfUl9D
VFJMLCBEQTcyMTlfREFDX0xfUkFNUF9FTl9TSElGVCwNCj4gPiArCQkgICAgIERBNzIxOV9TV0lU
Q0hfRU5fTUFYLCBEQTcyMTlfTk9fSU5WRVJUKSwNCj4gDQo+IA0KPiBBbGwgdGhlIERBQyBjb250
cm9scyBzaG91bGQgcHJvYmFibHkgYmUgUGxheWJhY2sgRGlnaXRhbCBpbnN0ZWFkLg0KDQpPaywg
c2ltaWxhciBuYW1pbmcgY29udmVudGlvbiBhcyBmb3IgQ2FwdHVyZSBEaWdpdGFsLiBXaWxsIHVw
ZGF0ZS4NCg0KPiANCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgZGE3MjE5X3NldF9kYWlfc3lzY2xr
KHN0cnVjdCBzbmRfc29jX2RhaSAqY29kZWNfZGFpLA0KPiA+ICsJCQkJIGludCBjbGtfaWQsIHVu
c2lnbmVkIGludCBmcmVxLCBpbnQgZGlyKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3Qgc25kX3NvY19j
b2RlYyAqY29kZWMgPSBjb2RlY19kYWktPmNvZGVjOw0KPiA+ICsJc3RydWN0IGRhNzIxOV9wcml2
ICpkYTcyMTkgPSBzbmRfc29jX2NvZGVjX2dldF9kcnZkYXRhKGNvZGVjKTsNCj4gPiArDQo+ID4g
KwlpZiAoZGE3MjE5LT5tY2xrX3JhdGUgPT0gZnJlcSkNCj4gPiArCQlyZXR1cm4gMDsNCj4gDQo+
IEdpdmVuIHRoYXQgd2UgYWxzbyBoYXZlIHNvdXJjZSBzZWxlY3Rpb24gaXMgdGhpcyBzYWZlIC0g
c2hvdWxkIHdlIGFsc28NCj4gYmUgY2hlY2tpbmcgdGhlIHNvdXJjZT8NCg0KR29vZCBzcG90LiBX
aWxsIGFtZW5kLg0KDQo+ID4gKwlpZiAoZGE3MjE5LT5tY2xrKSB7DQo+ID4gKwkJZnJlcSA9IGNs
a19yb3VuZF9yYXRlKGRhNzIxOS0+bWNsaywgZnJlcSk7DQo+ID4gKwkJY2xrX3NldF9yYXRlKGRh
NzIxOS0+bWNsaywgZnJlcSk7DQo+ID4gKwl9DQo+IA0KPiBNaXNzaW5nIGVycm9yIGNoZWNraW5n
Lg0KDQpZZXMsIHdpbGwgYWRkIGluIGNoZWNraW5nLg0KDQo+IA0KPiA+ICsJCQkvKiBJbnRlcm5h
bCBMRE8gKi8NCj4gPiArCQkJaWYgKGRhNzIxOS0+dXNlX2ludF9sZG8pDQo+ID4gKwkJCQlzbmRf
c29jX3VwZGF0ZV9iaXRzKGNvZGVjLCBEQTcyMTlfTERPX0NUUkwsDQo+ID4gKwkJCQkJCSAgICBE
QTcyMTlfTERPX0VOX01BU0ssDQo+ID4gKwkJCQkJCSAgICBEQTcyMTlfTERPX0VOX01BU0spOw0K
PiANCj4gSWYgdGhlcmUgaXMgYW4gb3B0aW9uIHRvIHVzZSBhbiBleHRlcm5hbCBzdXBwbHkgSSB3
b3VsZCBleHBlY3QgdG8gc2VlDQo+IHRoZSByZWd1bGF0b3IgQVBJIHVzZWQgdG8gZGlzY292ZXIg
dGhlIGV4dGVybmFsIExETyAoYW5kIGlkZWFsbHkgYWxzbyB0bw0KPiBjb25maWd1cmUgdGhlIGlu
dGVncmF0ZWQgTERPKS4gIElmIHRoZSBkcml2ZXIgd29ya3Mgb3V0c2lkZSB0aGUNCj4gZnJhbWV3
b3JrcyB0aGVuIGl0IGlzIGxpa2VseSB0aGlzIHdpbGwgbGVhZCB0byBpbnRlZ3JhdGlvbiBpc3N1
ZXMgbGF0ZXINCj4gb24uDQoNCkdpdmVuIHRoZSBzaW1wbGlzdGljIG5hdHVyZSBvZiB0aGUgaW50
ZXJuYWwgTERPLCBJIGRpZG4ndCB0aGluayBpdCB3b3VsZCBiZQ0KbmVjZXNzYXJ5IHRvIHVzZSB0
aGUgZnJhbWV3b3JrIGFzIGl0IHNlZW1lZCBvdmVya2lsbC4gSSBhc3N1bWUgeW91IG1lYW4NCmZv
bGxvd2luZyBzb21ldGhpbmcgbGlrZSB3aGF0IGlzIGRvbmUgaW4gdGhlIHNndGw1MDAwIGNvZGVj
Pw0KDQo+ID4gKwkJLyogSW50ZXJuYWwgTERPICovDQo+ID4gKwkJZGE3MjE5LT51c2VfaW50X2xk
byA9IHBkYXRhLT51c2VfaW50ZXJuYWxfbGRvOw0KPiANCj4gVGhpcyBpcyBsaWtlbHkgdG8gbGVh
ZCB0byBzdXJwcmlzZXMgZm9yIGV4YW1wbGUuLi4NCj4gDQo+ID4gKwkvKiBDaGVjayBpZiBNQ0xL
IHByb3ZpZGVkLCBpZiBub3QgdGhlIGNsb2NrIGlzIE5VTEwgKi8NCj4gPiArCWRhNzIxOS0+bWNs
ayA9IGRldm1fY2xrX2dldChjb2RlYy0+ZGV2LCAibWNsayIpOw0KPiA+ICsJaWYgKElTX0VSUihk
YTcyMTktPm1jbGspKSB7DQo+ID4gKwkJaWYgKFBUUl9FUlIoZGE3MjE5LT5tY2xrKSA9PSAtRVBS
T0JFX0RFRkVSKQ0KPiA+ICsJCQlyZXR1cm4gLUVQUk9CRV9ERUZFUjsNCj4gPiArCQlkYTcyMTkt
Pm1jbGsgPSBOVUxMOw0KPiA+ICsJfQ0KPiANCj4gVGhpcyBzaG91bGQgc3BlY2lmaWNhbGx5IGxv
b2sgZm9yIHRoZSBlcnJvciBjb2RlIHRoYXQgY29ycmVzcG9uZHMgdG8gYQ0KPiBjbG9jayBub3Qg
YmVpbmcgbWFwcGVkIGFuZCBvbmx5IGNvbnRpbnVlIHNpbGVudGx5IGluIHRoYXQgY2FzZSByYXRo
ZXINCj4gdGhhbiBzaWxlbnRseSBhY2NlcHRpbmcgYWxsIGZhaWx1cmVzIHRvIG9idGFpbiBhIGNs
b2NrIC0gaWYgd2Ugc2lsZW50bHkNCj4gYWNjZXB0IGFsbCBmYWlsdXJlcyB0aGF0IHdpbGwgbWVh
biB0aGF0IGFjdHVhbCBlcnJvcnMgd2lsbCBiZSBpZ25vcmVkLg0KIA0KT2ssIEkgd2lsbCBhZGQg
ZnVydGhlciBlcnJvciBjaGVja2luZyB0byBjb3ZlciB0aGlzLg0KDQo+ID4gKwkvKg0KPiA+ICsJ
ICogVGhlcmUgYXJlIG11bHRpcGxlIGNvbnRyb2wgYml0cyBmb3IgdGhlIGlucHV0IG1peGVyLg0K
PiA+ICsJICogVGhlIGZvbGxvd2luZyBjYW4gYmUgZW5hYmxlZCBub3cgYXMgaXQncyBub3QgcG93
ZXIgcmVsYXRlZC4NCj4gPiArCSAqLw0KPiA+ICsJc25kX3NvY191cGRhdGVfYml0cyhjb2RlYywg
REE3MjE5X01JWElOX0xfQ1RSTCwNCj4gPiArCQkJICAgIERBNzIxOV9NSVhJTl9MX01JWF9FTl9N
QVNLLA0KPiA+ICsJCQkgICAgREE3MjE5X01JWElOX0xfTUlYX0VOX01BU0spOw0KPiANCj4gU28g
dGhlIGNoaXAgZGVzaWduZXJzIGp1c3QgcHV0IHRoZXNlIGluIGZvciByYW5kb21uZXNzPyAgRnVu
LiAgSXQnZCBiZQ0KPiBtb3JlIGlkaW9tYXRpYyB0byBkbyBzb21ldGhpbmcgbGlrZSBtYWtpbmcg
dGhlc2Ugc3VwcGx5IHdpZGdldHMgc28NCj4gdGhleSdyZSBjb250cm9sbGVkIHZpYSBEQVBNIGV2
ZW4gaWYgdGhleSBkb24ndCBtYXR0ZXIgbXVjaC4NCg0KRmlndXJlZCB3ZSdkIGJlIHNhdmluZyBv
biBhZGRpdGlvbmFsIEkyQyBhY2Nlc3NlcyBpZiBpdCdzIGp1c3QgZG9uZSB0aGUgb25jZS4NCkRv
IHlvdSByZWFsbHkgdGhpbmsgaXQgbmVlZHMgdG8gYmUgYSB3aWRnZXQgYXMgaXQgc2VlbXMgYSBs
aXR0bGUgdW5uZWNlc3NhcnkNCmVuYWJsaW5nIGFuZCBkaXNhYmxpbmcgZXZlcnkgdGltZSB0aGF0
IHBhdGggaXMgcG93ZXJlZCB1cCBhbmQgZG93bj8NCg0KPiA+ICsJZWxzZSBpZiAoZGV2aWNlX21h
eV93YWtldXAoY29kZWMtPmRldikpDQo+ID4gKwkJZGlzYWJsZV9pcnFfd2FrZShkYTcyMTktPmFh
ZC0+aXJxKTsNCj4gDQo+IFlvdSBjYW4gdXNlIGRldl9wbV9zZXRfd2FrZV9pcnEoKSBhbmQgc2tp
cCBoYXZpbmcgdG8gbWFuYWdlIHRoaXMgc3R1ZmYNCj4gZXhwbGljaXRseSBpbiB0aGUgZHJpdmVy
Lg0KDQpBaCwgZGlkbid0IGtub3cgYWJvdXQgdGhhdC4gV2lsbCBjb252ZXJ0IG92ZXIgdG8gdXNp
bmcgdGhhdC4gVGhhbmtzLg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229686 — Re: [PATCH 1/3] ASoC: codecs: Add da7219 codec driver

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 03:08:03PM +0000, Opensource [Adam Thomson] wrote:
> On September 19, 2015 18:44, Mark Brown wrote:

> > This is obviously a massive reconfiguration of the device.  I'm not
> > seeing anything here which prevents userspace coming in and change the
> > configuration while we're in this function, that would obviously have
> > serious issues.

> In a system scenario the likelihood of that happening is small as you
> cannot use the mic or headphones until they've been inserted. The system is
> only likely to act after the jack insertion events have occurred. However, it

This really isn't an OK approach here, you're making a whole bunch of
assumptions about how the system is implemented that aren't robust and
will lead to loss of audio if things go wrong which is a pretty serious
consequence.  Are you *sure* there's going to be a quick enough response
to cover all jack inserts and remove (especially under load), or that
userspace even bothers paying attention given that there's no other
input and output devices?

> would be better to prevent any chance of concurrent access. The problem is how
> best to lock the Kcontrols whilst the test procedure in progress. At the moment
> the only way I can see is to add explicit control set() functions which would
> use a lock that can also be controlled by the HP test code. Does this make sense
> to you or do you know of a simpler method? Obviously dapm has function to lock
> as required.

Yes, you're going to have to do something like that if you want to do
this - you'll also need to lock the reads since otherwise userspace will
see the intermediate control states.

> For the cache resync idea, in terms of code, it will look cleaner, but you are
> talking about 4 to 5 times the number of I2C accesses to the device, to restore
> configuration. Does that not seem like a bit too much overhead?

There's regcache_sync_region().

> > Why are we scheduling work - we're already in thread context?

> hptest will take some time to complete (over 100ms), and in that time it's
> plausible that a user could remove the jack. If we deal with this in the IRQ
> thread, we won't be aware of jack removal during the process, and will send a
> report regardless, which will almost definitely be incorrect, and unnecessary.
> By spawning off work, it allows the removal to be dealt with if the hptest work
> procedure is currently in process, and then we can avoid sending incorrect jack
> reports at the end.

OK, please document this then.

> > > +			/* Un-drive headphones/lineout */
> > > +			snd_soc_update_bits(codec, DA7219_HP_R_CTRL,
> > > +					    DA7219_HP_R_AMP_OE_MASK, 0);
> > > +			snd_soc_update_bits(codec, DA7219_HP_L_CTRL,
> > > +					    DA7219_HP_L_AMP_OE_MASK, 0);

> > This looks like DAPM?

> The control of driving the headphones or making them high impedance is handled
> in the AAD code because we cannot have the headphones driven before a jack is
> inserted as it will affect the pole detection. Adding it to DAPM seemed like it
> would cause more problems in terms of controlling when it would and wouldn't be
> enabled.

IIRC you had some DAPM updates in adjacent code?

> > > +	default:
> > > +		return DA7219_AAD_JACK_INS_DEB_20MS;

> > This isn't an error?

> Opted for HW default in case of invalid values provided. Maybe a dev_warn()
> would be useful though to indicate this is the case?

Yes - the user has explicitly tried to set something and the driver is
ignoring it.

> > > +	ret = regmap_bulk_read(da7219->regmap, reg, &val, sizeof(val));
> > > +	if (ret)
> > > +		return ret;

> > > +	ucontrol->value.integer.value[0] = le16_to_cpu(val);

> > This is *weird*.  We do a bulk read for a single register using an API
> > that returns CPU endian data then make it CPU endian (without any
> > annotations on variables...).  Why not use regmap_read()?  Why swap?
> > Why not use raw I/O?

> The device is 8-bit register access only, and the value spans two registers,
> hence why this is done here. The register defined for the Kcontrol is the first
> in the sequence of two registers (first lower byte, second upper byte). Thought
> this was cleaner than having two separate controls to configure upper and
> lower bytes.

Again some documentation would help, and also using raw reads rather
than bulk reads (which imply that all endianness issues will be taken
care of).  If you're doing a bulk read and handling endianness that's
worrying.  You should also have an endianness annotation for val.

> > This looks like a standard enumeration with a simple mapping to the
> > register map?

> The bit values aren't sequential as per a normal enumeration, which is why I
> added this approach to setting the correct bits. However, I've just spotted the
> SOC_VALUE_ENUM_SINGLE macro which looks like it will do the job, so I'll use
> that instead. Thanks.

Yes, you want a VALUE_ENUM.

> > > +	/* ADCs */
> > > +	SOC_SINGLE_TLV("ADC Volume", DA7219_ADC_L_GAIN,
> > > +		       DA7219_ADC_L_DIGITAL_GAIN_SHIFT,
> > > +		       DA7219_ADC_L_DIGITAL_GAIN_MAX, DA7219_NO_INVERT,
> > > +		       da7219_adc_dig_gain_tlv),
> > > +	SOC_SINGLE("ADC Switch", DA7219_ADC_L_CTRL,
> > > +		   DA7219_ADC_L_MUTE_EN_SHIFT, DA7219_SWITCH_EN_MAX,
> > > +		   DA7219_INVERT),
> > > +	SOC_SINGLE("ADC Gain Ramp Switch", DA7219_ADC_L_CTRL,
> > > +		   DA7219_ADC_L_RAMP_EN_SHIFT, DA7219_SWITCH_EN_MAX,
> > > +		   DA7219_NO_INVERT),

> > Capture Digital rather than ADC.

> Ok, fine. Is this now the common naming to be used for all future codecs?

It's always been the naming in ControlNames.txt - we don't generally
worry about it on devices with more flexible routing which mean that the
associated meaning won't always be really true but for this device it
seems that the options are sufficiently limited to allow userspace to
use the standard name.

> > > +			/* Internal LDO */
> > > +			if (da7219->use_int_ldo)
> > > +				snd_soc_update_bits(codec, DA7219_LDO_CTRL,
> > > +						    DA7219_LDO_EN_MASK,
> > > +						    DA7219_LDO_EN_MASK);

> > If there is an option to use an external supply I would expect to see
> > the regulator API used to discover the external LDO (and ideally also to
> > configure the integrated LDO).  If the driver works outside the
> > frameworks then it is likely this will lead to integration issues later
> > on.

> Given the simplistic nature of the internal LDO, I didn't think it would be
> necessary to use the framework as it seemed overkill. I assume you mean
> following something like what is done in the sgtl5000 codec?

That should work I think.  The point here isn't really the control of
the LDO itself, it's making sure that the integration with external
supplies works well - the key bit is that how we figure out that we
don't have an external supply connected should be joined up with how we
normally integrate external supplies.

> > > +	/*
> > > +	 * There are multiple control bits for the input mixer.
> > > +	 * The following can be enabled now as it's not power related.
> > > +	 */
> > > +	snd_soc_update_bits(codec, DA7219_MIXIN_L_CTRL,
> > > +			    DA7219_MIXIN_L_MIX_EN_MASK,
> > > +			    DA7219_MIXIN_L_MIX_EN_MASK);

> > So the chip designers just put these in for randomness?  Fun.  It'd be
> > more idiomatic to do something like making these supply widgets so
> > they're controlled via DAPM even if they don't matter much.

> Figured we'd be saving on additional I2C accesses if it's just done the once.
> Do you really think it needs to be a widget as it seems a little unnecessary
> enabling and disabling every time that path is powered up and down?

It seems likely to be more robust against someone realising that the
register bits actually do something useful and need toggling and it
raises less eyebrows code wise.

If you're worried about the register writes you should also be able to
arrange to map these in as mixer widgets which would mean that the the
core will combine the writes with the main power controls.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web