Re: [PATCH v2 2/8] ASoC: codecs: wcd: add common helper for wcd codecs

[Dmitry Baryshkov]

On Fri, Aug 22, 2025 at 11:49:21AM +0100, Srinivas Kandagatla wrote:
> All the Qualcomm WCD codecs have most of its code duplicated across all
> these 3/4 drivers. This patch is an attempt to remove those duplicate

See Documentation/process/submitting-patches.rst, "This patch"

> parts by adding a common helper library for these codecs.
> 
> To start with move all the micbias parsing and voltage settings these
> are identical in WCD934x, WCD937x, WCD938x and WCD939x codec driver.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxxxxxxxx>
> ---
>  sound/soc/codecs/Kconfig      |  8 ++++
>  sound/soc/codecs/Makefile     |  2 +
>  sound/soc/codecs/wcd-common.c | 66 ++++++++++++++++++++++++++
>  sound/soc/codecs/wcd-common.h | 24 ++++++++++
>  sound/soc/codecs/wcd934x.c    | 82 +++++++++-----------------------
>  sound/soc/codecs/wcd937x.c    | 75 +++++++----------------------
>  sound/soc/codecs/wcd937x.h    |  6 +--
>  sound/soc/codecs/wcd938x.c    | 87 +++++++---------------------------
>  sound/soc/codecs/wcd938x.h    |  1 +
>  sound/soc/codecs/wcd939x.c    | 89 +++++++----------------------------
>  sound/soc/codecs/wcd939x.h    |  1 +
>  11 files changed, 176 insertions(+), 265 deletions(-)
>  create mode 100644 sound/soc/codecs/wcd-common.c
>  create mode 100644 sound/soc/codecs/wcd-common.h
> 
> diff --git a/sound/soc/codecs/wcd-common.h b/sound/soc/codecs/wcd-common.h
> new file mode 100644
> index 000000000000..08c8e7ce9814
> --- /dev/null
> +++ b/sound/soc/codecs/wcd-common.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.

Please use the correct copyright according to guidelines.

> + */
> +
> +#ifndef __WCD_COMMON_H__
> +#define __WCD_COMMON_H___
> +
> +#define WCD_MIN_MICBIAS_MV	1000
> +#define WCD_DEF_MICBIAS_MV	1800
> +#define WCD_MAX_MICBIAS_MV	2850

These do not belong to a public header.

> +#define WCD_MAX_MICBIAS		4
> +
> +struct wcd_common {
> +	struct device *dev;
> +	int max_bias;
> +	u32 micb_mv[WCD_MAX_MICBIAS];
> +	u32 micb_vout[WCD_MAX_MICBIAS];
> +};
> +
> +int wcd_get_micb_vout_ctl_val(struct device *dev, u32 micb_mv);
> +int wcd_dt_parse_micbias_info(struct wcd_common *common);
> +
> +#endif /* __WCD_COMMON_H___  */

> diff --git a/sound/soc/codecs/wcd939x.h b/sound/soc/codecs/wcd939x.h
> index e70445b1a4bc..f587cf22f86f 100644
> --- a/sound/soc/codecs/wcd939x.h
> +++ b/sound/soc/codecs/wcd939x.h
> @@ -8,6 +8,7 @@
>  #define __WCD939X_H__
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_type.h>
> +#include "wcd-common.h"

No need to. Please include headers where they are actually used.

>  
>  #define WCD939X_BASE					   (0x3000)
>  #define WCD939X_ANA_PAGE				   (0x3000)
> -- 
> 2.50.0
> 

-- 
With best wishes
Dmitry