linux-kernel - Re: [PATCH 4/5] ASoC: SDCA: Add missing function type names

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2038977-4f5b-4a9a-bd5f-89e6839e1703@linux.dev>
Date: Thu, 2 Jan 2025 16:01:19 -0600
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>, broonie@...nel.org
Cc: lgirdwood@...il.com, peter.ujfalusi@...ux.intel.com,
 yung-chuan.liao@...ux.intel.com, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH 4/5] ASoC: SDCA: Add missing function type names



On 12/20/24 11:35, Charles Keepax wrote:
> It is not helpful to error out on some SDCA function types, we
> might as well report the correct name and let the driver core
> simply not bind a driver to those functions for which the code
> lacks support. Also given no functions currently have support,
> it seems odd to select some as unsupported.
> 
> Signed-off-by: Charles Keepax <ckeepax@...nsource.cirrus.com>
> ---
>   include/sound/sdca_function.h   |  1 +
>   sound/soc/sdca/sdca_functions.c | 12 ++++++++++--
>   2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
> index 6943df0851a9..89e42db6d591 100644
> --- a/include/sound/sdca_function.h
> +++ b/include/sound/sdca_function.h
> @@ -42,6 +42,7 @@ enum sdca_function_type {
>   #define	SDCA_FUNCTION_TYPE_RJ_NAME		"RJ"
>   #define	SDCA_FUNCTION_TYPE_SIMPLE_NAME		"SimpleJack"
>   #define	SDCA_FUNCTION_TYPE_HID_NAME		"HID"
> +#define	SDCA_FUNCTION_TYPE_IMP_DEF_NAME		"ImplementationDefined"
>   
>   enum sdca_entity0_controls {
>   	SDCA_CONTROL_ENTITY_0_COMMIT_GROUP_MASK		= 0x01,
> diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
> index a69fdb9c8b15..400763e056fa 100644
> --- a/sound/soc/sdca/sdca_functions.c
> +++ b/sound/soc/sdca/sdca_functions.c
> @@ -75,12 +75,20 @@ static int patch_sdca_function_type(struct device *dev,
>   		*function_name = SDCA_FUNCTION_TYPE_HID_NAME;
>   		break;
>   	case SDCA_FUNCTION_TYPE_SIMPLE_AMP:
> +		*function_name = SDCA_FUNCTION_TYPE_SIMPLE_AMP_NAME;
> +		break;
>   	case SDCA_FUNCTION_TYPE_SIMPLE_MIC:
> +		*function_name = SDCA_FUNCTION_TYPE_SIMPLE_MIC_NAME;
> +		break;
>   	case SDCA_FUNCTION_TYPE_SPEAKER_MIC:
> +		*function_name = SDCA_FUNCTION_TYPE_SPEAKER_MIC_NAME;
> +		break;
>   	case SDCA_FUNCTION_TYPE_RJ:
> +		*function_name = SDCA_FUNCTION_TYPE_RJ_NAME;
> +		break;

the changes above are fine...

>   	case SDCA_FUNCTION_TYPE_IMP_DEF:
> -		dev_warn(dev, "unsupported SDCA function type %d\n", *function_type);
> -		return -EINVAL;
> +		*function_name = SDCA_FUNCTION_TYPE_IMP_DEF_NAME;
> +		break;

but this one is controversial. We really have no idea what to do in this 
case. There could be completely different 'imp-def' functions that 
require different drivers. I don't know how the entire binding mechanism 
would work. Probably best to err on the side of throwing a 'not 
supported' error?

>   	default:
>   		dev_err(dev, "invalid SDCA function type %d\n", *function_type);
>   		return -EINVAL;