On Mon, Jan 25, 2016 at 05:14:43PM -0600, tim.howe@xxxxxxxxxx wrote:
> From: Tim Howe <tim.howe@xxxxxxxxxx>
I'd have got to this sooner but there was an unreplied mail from the
0day bot... anyway, a few fairly minor issues here:
> + default:
> + pr_err("Invalid event = 0x%x\n", event);
> + return -EINVAL;
> + }
dev_err().
> + regmap_read(priv->regmap, CS53L30_MCLKCTL, &mclk_ctl);
> + mclk_ctl &= ~MCLK_DIV;
> + mclk_ctl |= cs53l30_mclkx_coeffs[mclkx_coeff].mclkdiv;
> +
> + regmap_write(priv->regmap, CS53L30_MCLKCTL, mclk_ctl);
This is open coding regmap_update_bits(), several other bits of the
driver seem to be doing something similar.
> + regmap_read(priv->regmap, CS53L30_IS, ®); /* Clr status */
> + for (i = 0; i < inter_max_check; i++) {
> + usleep_range(1000, 1000);
> + msleep(1);
So we do a usleep_range() for 1ms then a msleep() for 1ms... why not
just do one or the other for 2ms? It at least looks weird and needs a
comment.
> +/* CS53L30_ADCDMIC1_CTL1 */
> +#define ADC1B_PDN (1 << 7)
> +#define ADC1A_PDN (1 << 6)
These constants really need namespacing - a lot of them have pretty
generic names so look like they might end up colliding with a kernel
header.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
