On Mon, Sep 02, 2024 at 02:20:27PM +0800, Shenghao Ding wrote:
> Add calibration related kcontrol for speaker impedance calibration and
> speaker leakage check for Chromebook
Missing grammatical period at the end.
...
It's possible to create a macro and reduce the below _a lot_.
Look at the include/linux/propery.h on how to have a similar one.
> +static const struct bulk_reg_val tas2563_cali_start_reg[] = {
...
> +static const struct bulk_reg_val tas2781_cali_start_reg[] = {
> + {
> + .reg = TAS2781_PRM_INT_MASK_REG,
> + .val = { 0xfe },
> + .val_len = 1,
> + .is_locked = false
Please, mind the trailing comma in these cases.
> + },
> +};
...
> + rc = tasdevice_dev_bulk_read(tas_priv, i, reg, &dst[1],
> + 4);
This reads much better in a single line.
...
> +static int tasdevice_active_num_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
> + struct tasdevice_priv *tas_priv = snd_soc_component_get_drvdata(codec);
> + int dev_id = ucontrol->value.integer.value[0];
> + int max = tas_priv->ndev - 1, rc;
> +
> + dev_id = clamp(dev_id, 0, max);
> +
> + guard(mutex)(&tas_priv->codec_lock);
> + rc = tasdev_chn_switch(tas_priv, dev_id);
> +
> + return rc;
int dev_id;
dev_id = clamp(ucontrol->value.integer.value[0], 0, tas_priv->ndev - 1);
guard(mutex)(&tas_priv->codec_lock);
return tasdev_chn_switch(tas_priv, dev_id);
> +}
...
> + rc = snd_soc_add_component_controls(tas_priv->codec, cali_ctrls,
> + num_controls);
Single line?
...
> + /*
> + * Alloc kcontrol via devm_kzalloc, which don't manually
devm_kzalloc()
> + * free the kcontrol
Missing period.
> + */
...
> + /*
> + * package structure for tas2781 ftc start:
Package
> + * Pkg len (1 byte)
> + * Reg id (1 byte, constant 'r')
> + * book, page, register for pilot threshold, pilot tone
> + * and sine gain (12 bytes)
> + * for (i = 0; i < Device-Sum; i++) {
> + * Device #i index_info (1 byte)
> + * Sine gain for Device #i (8 bytes)
> + * }
> + */
...
> + rc = snd_soc_add_component_controls(tas_priv->codec, cali_ctrls,
> + num_controls < i ? num_controls : i);
> +
> + return rc;
return snd_soc_add_component_controls(tas_priv->codec, cali_ctrls,
num_controls < i ? num_controls : i);
Can num_controls ever be bigger than i?
> +}
--
With Best Regards,
Andy Shevchenko
