[PATCH] ASoC: rt5682: split i2c driver into separate module

Mark Brown broonie at kernel.org
Wed May 27 22:56:58 CEST 2020


On Wed, May 27, 2020 at 09:34:13PM +0200, Arnd Bergmann wrote:

> index 000000000000..c891aa80757f
> --- /dev/null
> +++ b/sound/soc/codecs/rt5682-i2c.c
> @@ -0,0 +1,341 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * rt5682.c  --  RT5682 ALSA SoC audio component driver
> + *

Please keep the entire comment C++ so things look more intentional.

> +static const char *rt5682_supply_names[RT5682_NUM_SUPPLIES] = {
> +	"AVDD",
> +	"MICVDD",
> +	"VBAT",
> +};

I'm *fairly* sure the device needs power even with Soundwire?

> +static void rt5682_jd_check_handler(struct work_struct *work)
> +{
> +	struct rt5682_priv *rt5682 = container_of(work, struct rt5682_priv,
> +		jd_check_work.work);

> +static int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev)
> +{
> +
> +	device_property_read_u32(dev, "realtek,dmic1-data-pin",
> +		&rt5682->pdata.dmic1_data_pin);
> +	device_property_read_u32(dev, "realtek,dmic1-clk-pin",
> +		&rt5682->pdata.dmic1_clk_pin);

This doesn't look very I2C specific either, nor do chunks of the rest of
the code.  The usual pattern with this stuff is to have the bus specific
code do bus specific stuff like setting up the regmap and then call into
a common init function for the shared parts of the chip.  I'd expect a
bit more unshared code here but not this much.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20200527/4adb2315/attachment.sig>


More information about the Alsa-devel mailing list