Received-SPF: pass (google.com: domain of linux-kernel+bounces-195283-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Content-Type: multipart/signed;
 boundary=b04e15e85d503ad3de99a862da1f51c734f1e4825c9724c1f5249d05531d;
 micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: bulk
Mime-Version: 1.0
Date: Thu, 30 May 2024 13:02:59 +0200
Message-Id: <D1MXG0PGXQW8.2GBRKOVBQM49H@gmail.com>
Cc: <linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
 <lgirdwood@gmail.com>, <jonathanh@nvidia.com>, <mkumard@nvidia.com>
Subject: Re: [PATCH v2 1/2] ASoC: simple-card-utils: Split
 simple_fixup_sample_fmt func
From: "Thierry Reding" <thierry.reding@gmail.com>
To: "Sameer Pujar" <spujar@nvidia.com>, <broonie@kernel.org>,
 <linux-sound@vger.kernel.org>, <alsa-devel@alsa-project.org>
References: <20240527125608.2461300-1-spujar@nvidia.com>
 <20240527125608.2461300-2-spujar@nvidia.com>
In-Reply-To: <20240527125608.2461300-2-spujar@nvidia.com>

--b04e15e85d503ad3de99a862da1f51c734f1e4825c9724c1f5249d05531d
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8

On Mon May 27, 2024 at 2:56 PM CEST, Sameer Pujar wrote:
> From: Mohan Kumar <mkumard@nvidia.com>
>
> Split the simple_fixup_sample_fmt() into two functions by adding
> one more function named simple_util_get_sample_fmt() to return
> the sample format value.
>
> This is useful for drivers that wish to simply get the sample format
> without setting the mask.
>
> Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  include/sound/simple_card_utils.h     |  2 ++
>  sound/soc/generic/simple-card-utils.c | 26 ++++++++++++++++++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_car=
d_utils.h
> index ad67957b7b48..2c2279d082ec 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -174,6 +174,8 @@ void simple_util_parse_convert(struct device_node *np=
, char *prefix,
>  			       struct simple_util_data *data);
>  bool simple_util_is_convert_required(const struct simple_util_data *data=
);
> =20
> +int simple_util_get_sample_fmt(struct simple_util_data *data);
> +
>  int simple_util_parse_routing(struct snd_soc_card *card,
>  				      char *prefix);
>  int simple_util_parse_widgets(struct snd_soc_card *card,
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/si=
mple-card-utils.c
> index 81077d16d22f..f1f5a1c025fc 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -13,12 +13,11 @@
>  #include <sound/pcm_params.h>
>  #include <sound/simple_card_utils.h>
> =20
> -static void simple_fixup_sample_fmt(struct simple_util_data *data,
> -					 struct snd_pcm_hw_params *params)
> +int simple_util_get_sample_fmt(struct simple_util_data *data)
>  {
>  	int i;
> -	struct snd_mask *mask =3D hw_param_mask(params,
> -					      SNDRV_PCM_HW_PARAM_FORMAT);
> +	int val =3D -EINVAL;
> +
>  	struct {
>  		char *fmt;
>  		u32 val;
> @@ -33,11 +32,26 @@ static void simple_fixup_sample_fmt(struct simple_uti=
l_data *data,
>  	for (i =3D 0; i < ARRAY_SIZE(of_sample_fmt_table); i++) {
>  		if (!strcmp(data->convert_sample_format,
>  			    of_sample_fmt_table[i].fmt)) {
> -			snd_mask_none(mask);
> -			snd_mask_set(mask, of_sample_fmt_table[i].val);
> +			val =3D of_sample_fmt_table[i].val;
>  			break;
>  		}
>  	}
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(simple_util_get_sample_fmt);
> +
> +static void simple_fixup_sample_fmt(struct simple_util_data *data,
> +				    struct snd_pcm_hw_params *params)
> +{
> +	int val;
> +	struct snd_mask *mask =3D hw_param_mask(params,
> +					      SNDRV_PCM_HW_PARAM_FORMAT);
> +
> +	val =3D simple_util_get_sample_fmt(data);
> +	if (val >=3D 0) {

Maybe nothing that we need to worry about, but this could be potentially
a problem if the snd_pcm_format_t ever outgrows the non-negative number
space. snd_pcm_format_t is defined to be an int as well, so any very
large number would wrap into a negative value and then would be
considered an error in this check.

Then again, if that ever were to happen, we'd likely get a compiler
error for the snd_pcm_format_t overflowing, so we'd probably notice and
rewrite at that point.

So I suppose:

Reviewed-by: Thierry Reding <treding@nvidia.com>

--b04e15e85d503ad3de99a862da1f51c734f1e4825c9724c1f5249d05531d
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmZYXOMACgkQ3SOs138+
s6Hq1Q/+O1LuOBIU5Ga0AXMC62PY26RnEsxrv6TJtxcqwaolgwRpZlyOUDAzVvsg
ltAPTkzGEK3kTTeilIIMN2ZXK6fmMYH9qwF8AebVBUtNhRqIo/UnQsz9O6gzxPMa
ZbpfwO/MZGm06ZcKidnTDmLtRotFEx/6o0LW/tJhabOow998nCrD1yGdq3oXGFRC
lqovd9U1RoeI5IP4r3zJpJ5GbYSc/a9K04vqwq9nkvClx2xNoA03KuHce2FtkjAj
R6xknIHgcSKHIoh82lQE6pDOPt26+/Fd80b5UwqRtr74IRiUmJOboRxWKMqoJNiY
TQgIE5PN0R1OGKzrBLb/0kMLXM+VYa5LJKQMxI9FWTWNVx0B79Vyrb1lE8hMpcgE
yRuvzBHWeYedzriZDJgwEdVVayBf2u2rAoXTRGRtuBAmywtoek3oakbPxYArfnXb
fjtgV3wq9x/37DJZtPVKBU57IOxu/Pbz2SqWdw81b8DfT931IPeq4sMn+0rnv15m
zXuIMuqKefis9QMHAEmLjsCfPCnDmZCdXQdELspUMXNBprZNYrOmFctd4xStilvs
WKsKuAk0Kgox0qGDKM8fPH2F8LexSIZc41UT1EgLTlO1bZ9keROIUCoFpReQg0ET
xv/16iwcHORl2WGuLvkvBIWQ/AISHDJB8czKV3OB15j2vWTwxz8=
=dpDb
-----END PGP SIGNATURE-----

--b04e15e85d503ad3de99a862da1f51c734f1e4825c9724c1f5249d05531d--