Re: [PATCH] ASoC: rt5660: add rt5660 codec driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 23, 2016 at 01:33:43PM +0800, Oder Chiou wrote:

> +- realtek,use-ldo2

This would more normally be done by making LDO2 a regulator via the
normal regulator framework - look at how the arizona drivers handle
DCVDD for an example, it provides a default supply mapping which makes
things really easy for users.  Right now it's not clear to me looking at
the code that the driver works for systems that don't use an internal
LDO as there's nothing that enables MICVDD.

> +static int rt5660_probe(struct snd_soc_codec *codec)
> +{
> +	struct rt5660_priv *rt5660 = snd_soc_codec_get_drvdata(codec);
> +
> +	/* Check if MCLK provided */
> +	rt5660->mclk = devm_clk_get(codec->dev, "mclk");
> +	if (PTR_ERR(rt5660->mclk) == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;

This should be in the I2C level probe so we don't have the device
showing as probing there without the resources it needs being present as
that makes things confusing for people.

> +	if (rt5660->pdata.dmic1_data_pin) {
> +		regmap_update_bits(rt5660->regmap, RT5660_GPIO_CTRL1,
> +			RT5660_GP1_PIN_MASK, RT5660_GP1_PIN_DMIC1_SCL);
> +
> +		if (rt5660->pdata.dmic1_data_pin == RT5660_DMIC1_DATA_GPIO2)
> +			regmap_update_bits(rt5660->regmap, RT5660_DMIC_CTRL1,
> +				RT5660_SEL_DMIC_DATA_MASK,
> +				RT5660_SEL_DMIC_DATA_GPIO2);
> +		else if (rt5660->pdata.dmic1_data_pin == RT5660_DMIC1_DATA_IN1P)
> +			regmap_update_bits(rt5660->regmap, RT5660_DMIC_CTRL1,
> +				RT5660_SEL_DMIC_DATA_MASK,
> +				RT5660_SEL_DMIC_DATA_IN1P);
> +	}

We do a reset when we unregister at the CODEC level - that'll overwrite
these settings won't it?

Otherwise this looks good.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux