[PATCH -next 0/5] ASoC: fsl: Fix sparse warning

[PATCH -next 0/5] ASoC: fsl: Fix sparse warning

[Shengjiu Wang]

Fix sparse warning

Shengjiu Wang (5):
  ASoC: fsl_sai: Don't use plain integer as NULL pointer
  ASoC: fsl_asrc: force cast the asrc_format type
  ASoC: fsl-asoc-card: force cast the asrc_format type
  ASoC: fsl_easrc: use snd_pcm_format_t type for sample_format
  ASoC: imx-card: use snd_pcm_format_t type for asrc_format

 sound/soc/fsl/fsl-asoc-card.c | 2 +-
 sound/soc/fsl/fsl_asrc.c      | 4 ++--
 sound/soc/fsl/fsl_easrc.c     | 7 ++++---
 sound/soc/fsl/fsl_easrc.h     | 2 +-
 sound/soc/fsl/fsl_sai.c       | 2 +-
 sound/soc/fsl/imx-card.c      | 7 ++++---
 6 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH -next 1/5] ASoC: fsl_sai: Don't use plain integer as NULL pointer

[Shengjiu Wang]

Fix sparse warning:
sound/soc/fsl/fsl_sai.c:64:39: sparse: warning: Using plain integer as NULL pointer

Fixes: b4ee8a913e61 ("ASoc: fsl_sai: Add pinctrl operation for PDM and DSD")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 974ba0780b19..7523bb944b21 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -61,7 +61,7 @@ static inline bool fsl_sai_dir_is_synced(struct fsl_sai *sai, int dir)
 
 static struct pinctrl_state *fsl_sai_get_pins_state(struct fsl_sai *sai, u32 bclk)
 {
-	struct pinctrl_state *state = 0;
+	struct pinctrl_state *state = NULL;
 
 	if (sai->is_pdm_mode) {
 		/* DSD512@44.1kHz, DSD512@48kHz */
-- 
2.34.1

[PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

Fix sparse warning:
sound/soc/fsl/fsl_asrc.c:1177:60: sparse: warning: incorrect type in argument 3 (different base types)
sound/soc/fsl/fsl_asrc.c:1177:60: sparse:    expected unsigned int [usertype] *out_value
sound/soc/fsl/fsl_asrc.c:1177:60: sparse:    got restricted snd_pcm_format_t *
sound/soc/fsl/fsl_asrc.c:1200:47: sparse: warning: restricted snd_pcm_format_t degrades to integer

Fixes: 4520af41fd21 ("ASoC: fsl_asrc: Support new property fsl,asrc-format")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 20a9f8e924b3..544395efd605 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1174,7 +1174,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
+	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
 	if (ret) {
 		ret = of_property_read_u32(np, "fsl,asrc-width", &width);
 		if (ret) {
@@ -1197,7 +1197,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (!(FSL_ASRC_FORMATS & (1ULL << asrc->asrc_format))) {
+	if (!(FSL_ASRC_FORMATS & (1ULL << (__force u32)asrc->asrc_format))) {
 		dev_warn(&pdev->dev, "unsupported width, use default S24_LE\n");
 		asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;
 	}
-- 
2.34.1

[PATCH -next 3/5] ASoC: fsl-asoc-card: force cast the asrc_format type

[Shengjiu Wang]

Fix sparse warning:
sound/soc/fsl/fsl-asoc-card.c:833:45: sparse: warning: incorrect type in argument 3 (different base types)
sound/soc/fsl/fsl-asoc-card.c:833:45: sparse:    expected unsigned int [usertype] *out_value
sound/soc/fsl/fsl-asoc-card.c:833:45: sparse:    got restricted snd_pcm_format_t *

Fixes: 859e364302c5 ("ASoC: fsl-asoc-card: Support new property fsl, asrc-format")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index d9a0d4768c4d..c63f7d6517cf 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -830,7 +830,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		}
 
 		ret = of_property_read_u32(asrc_np, "fsl,asrc-format",
-					   &priv->asrc_format);
+					   (u32 *)&priv->asrc_format);
 		if (ret) {
 			/* Fallback to old binding; translate to asrc_format */
 			ret = of_property_read_u32(asrc_np, "fsl,asrc-width",
-- 
2.34.1

[PATCH -next 4/5] ASoC: fsl_easrc: use snd_pcm_format_t type for sample_format

[Shengjiu Wang]

Fix sparse warning:
sound/soc/fsl/fsl_easrc.c:562:33: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:563:34: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:565:38: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:566:39: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:608:33: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:609:34: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:615:40: sparse: warning: restricted snd_pcm_format_t degrades to integer
sound/soc/fsl/fsl_easrc.c:616:41: sparse: warning: restricted snd_pcm_format_t degrades to integer

sound/soc/fsl/fsl_easrc.c:1465:51: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/fsl_easrc.c:1465:51: sparse:    expected unsigned int sample_format
sound/soc/fsl/fsl_easrc.c:1465:51: sparse:    got restricted snd_pcm_format_t [usertype] format
sound/soc/fsl/fsl_easrc.c:1467:52: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/fsl_easrc.c:1467:52: sparse:    expected unsigned int sample_format
sound/soc/fsl/fsl_easrc.c:1467:52: sparse:    got restricted snd_pcm_format_t [usertype] asrc_format
sound/soc/fsl/fsl_easrc.c:1470:52: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/fsl_easrc.c:1470:52: sparse:    expected unsigned int sample_format
sound/soc/fsl/fsl_easrc.c:1470:52: sparse:    got restricted snd_pcm_format_t [usertype] format
sound/soc/fsl/fsl_easrc.c:1472:51: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/fsl_easrc.c:1472:51: sparse:    expected unsigned int sample_format
sound/soc/fsl/fsl_easrc.c:1472:51: sparse:    got restricted snd_pcm_format_t [usertype] asrc_format
sound/soc/fsl/fsl_easrc.c:1484:41: sparse: warning: incorrect type in argument 2 (different base types)
sound/soc/fsl/fsl_easrc.c:1484:41: sparse:    expected restricted snd_pcm_format_t [usertype] *in_raw_format
sound/soc/fsl/fsl_easrc.c:1484:41: sparse:    got unsigned int *
sound/soc/fsl/fsl_easrc.c:1485:41: sparse: warning: incorrect type in argument 3 (different base types)
sound/soc/fsl/fsl_easrc.c:1485:41: sparse:    expected restricted snd_pcm_format_t [usertype] *out_raw_format
sound/soc/fsl/fsl_easrc.c:1485:41: sparse:    got unsigned int *
sound/soc/fsl/fsl_easrc.c:1937:60: sparse: warning: incorrect type in argument 3 (different base types)
sound/soc/fsl/fsl_easrc.c:1937:60: sparse:    expected unsigned int [usertype] *out_value
sound/soc/fsl/fsl_easrc.c:1937:60: sparse:    got restricted snd_pcm_format_t *
sound/soc/fsl/fsl_easrc.c:1943:49: sparse: warning: restricted snd_pcm_format_t degrades to integer

Fixes: 955ac624058f ("ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_easrc.c | 7 ++++---
 sound/soc/fsl/fsl_easrc.h | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index ea96b0fb6b20..3f301f1a0acc 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -476,7 +476,8 @@ static int fsl_easrc_prefilter_config(struct fsl_asrc *easrc,
 	struct fsl_asrc_pair *ctx;
 	struct device *dev;
 	u32 inrate, outrate, offset = 0;
-	u32 in_s_rate, out_s_rate, in_s_fmt, out_s_fmt;
+	u32 in_s_rate, out_s_rate;
+	snd_pcm_format_t in_s_fmt, out_s_fmt;
 	int ret, i;
 
 	if (!easrc)
@@ -1934,13 +1935,13 @@ static int fsl_easrc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = of_property_read_u32(np, "fsl,asrc-format", &easrc->asrc_format);
+	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&easrc->asrc_format);
 	if (ret) {
 		dev_err(dev, "failed to asrc format\n");
 		return ret;
 	}
 
-	if (!(FSL_EASRC_FORMATS & (1ULL << easrc->asrc_format))) {
+	if (!(FSL_EASRC_FORMATS & (1ULL << (__force u32)easrc->asrc_format))) {
 		dev_warn(dev, "unsupported format, switching to S24_LE\n");
 		easrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;
 	}
diff --git a/sound/soc/fsl/fsl_easrc.h b/sound/soc/fsl/fsl_easrc.h
index 86d5c360d4f5..7c70dac52713 100644
--- a/sound/soc/fsl/fsl_easrc.h
+++ b/sound/soc/fsl/fsl_easrc.h
@@ -569,7 +569,7 @@ struct fsl_easrc_io_params {
 	unsigned int access_len;
 	unsigned int fifo_wtmk;
 	unsigned int sample_rate;
-	unsigned int sample_format;
+	snd_pcm_format_t sample_format;
 	unsigned int norm_rate;
 };
 
-- 
2.34.1

[PATCH -next 5/5] ASoC: imx-card: use snd_pcm_format_t type for asrc_format

[Shengjiu Wang]

Fix sparse warning:
sound/soc/fsl/imx-card.c:653:59: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/imx-card.c:653:59: sparse:    expected unsigned int [usertype] asrc_format
sound/soc/fsl/imx-card.c:653:59: sparse:    got restricted snd_pcm_format_t [usertype]
sound/soc/fsl/imx-card.c:655:59: sparse: warning: incorrect type in assignment (different base types)
sound/soc/fsl/imx-card.c:655:59: sparse:    expected unsigned int [usertype] asrc_format
sound/soc/fsl/imx-card.c:655:59: sparse:    got restricted snd_pcm_format_t [usertype]

Fixes: aa736700f42f ("ASoC: imx-card: Add imx-card machine driver")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/imx-card.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/imx-card.c b/sound/soc/fsl/imx-card.c
index ccc4194dc5e7..bd7e489e96a1 100644
--- a/sound/soc/fsl/imx-card.c
+++ b/sound/soc/fsl/imx-card.c
@@ -118,7 +118,7 @@ struct imx_card_data {
 	struct snd_soc_card card;
 	int num_dapm_routes;
 	u32 asrc_rate;
-	u32 asrc_format;
+	snd_pcm_format_t asrc_format;
 };
 
 static struct imx_akcodec_fs_mul ak4458_fs_mul[] = {
@@ -474,7 +474,7 @@ static int be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 
 	mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
 	snd_mask_none(mask);
-	snd_mask_set(mask, data->asrc_format);
+	snd_mask_set(mask, (__force unsigned int)data->asrc_format);
 
 	return 0;
 }
@@ -639,7 +639,8 @@ static int imx_card_parse_of(struct imx_card_data *data)
 				goto err;
 			}
 
-			ret = of_property_read_u32(args.np, "fsl,asrc-format", &data->asrc_format);
+			ret = of_property_read_u32(args.np, "fsl,asrc-format",
+						   (u32 *)&data->asrc_format);
 			if (ret) {
 				/* Fallback to old binding; translate to asrc_format */
 				ret = of_property_read_u32(args.np, "fsl,asrc-width", &width);
-- 
2.34.1

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

From: Shengjiu Wang
> Sent: 19 July 2022 10:28
> 
> Fix sparse warning:
> sound/soc/fsl/fsl_asrc.c:1177:60: sparse: warning: incorrect type in argument 3 (different base types)
> sound/soc/fsl/fsl_asrc.c:1177:60: sparse:    expected unsigned int [usertype] *out_value
> sound/soc/fsl/fsl_asrc.c:1177:60: sparse:    got restricted snd_pcm_format_t *
> sound/soc/fsl/fsl_asrc.c:1200:47: sparse: warning: restricted snd_pcm_format_t degrades to integer
> 
> Fixes: 4520af41fd21 ("ASoC: fsl_asrc: Support new property fsl,asrc-format")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/fsl_asrc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 20a9f8e924b3..544395efd605 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -1174,7 +1174,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
> 
> -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);

Ugg, you really shouldn't need to do that.
It means that something is badly wrong somewhere.
Casting pointers to integer types is just asking for a bug.

>  	if (ret) {
>  		ret = of_property_read_u32(np, "fsl,asrc-width", &width);
>  		if (ret) {
> @@ -1197,7 +1197,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  		}
>  	}
> 
> -	if (!(FSL_ASRC_FORMATS & (1ULL << asrc->asrc_format))) {
> +	if (!(FSL_ASRC_FORMATS & (1ULL << (__force u32)asrc->asrc_format))) {

Ditto.

	David

>  		dev_warn(&pdev->dev, "unsupported width, use default S24_LE\n");
>  		asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;
>  	}
> --
> 2.34.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> From: Shengjiu Wang

> > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);

> Ugg, you really shouldn't need to do that.
> It means that something is badly wrong somewhere.
> Casting pointers to integer types is just asking for a bug.

That's casting one pointer type to another pointer type.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> From: Shengjiu Wang

> > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);

> Ugg, you really shouldn't need to do that.
> It means that something is badly wrong somewhere.
> Casting pointers to integer types is just asking for a bug.

That's casting one pointer type to another pointer type.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> From: Shengjiu Wang

> > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);

> Ugg, you really shouldn't need to do that.
> It means that something is badly wrong somewhere.
> Casting pointers to integer types is just asking for a bug.

That's casting one pointer type to another pointer type.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

From: Mark Brown
> Sent: 19 July 2022 11:17
> 
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > From: Shengjiu Wang
> 
> > > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
> 
> > Ugg, you really shouldn't need to do that.
> > It means that something is badly wrong somewhere.
> > Casting pointers to integer types is just asking for a bug.
> 
> That's casting one pointer type to another pointer type.

It is casting the address of some type to a 'u32 *'.
This will then be dereferenced by the called function.
So the original type better be 32 bits.

I'm also guessing that sparse was complaining about endianness?
It isn't at all clear that these casts actually fix it.

(Mark: You'll be glad to hear that the office aircon is
broken again - two weeks lead time on the spare part.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

From: Mark Brown
> Sent: 19 July 2022 11:17
> 
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > From: Shengjiu Wang
> 
> > > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
> 
> > Ugg, you really shouldn't need to do that.
> > It means that something is badly wrong somewhere.
> > Casting pointers to integer types is just asking for a bug.
> 
> That's casting one pointer type to another pointer type.

It is casting the address of some type to a 'u32 *'.
This will then be dereferenced by the called function.
So the original type better be 32 bits.

I'm also guessing that sparse was complaining about endianness?
It isn't at all clear that these casts actually fix it.

(Mark: You'll be glad to hear that the office aircon is
broken again - two weeks lead time on the spare part.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

From: Mark Brown
> Sent: 19 July 2022 11:17
> 
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > From: Shengjiu Wang
> 
> > > -	ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > > +	ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
> 
> > Ugg, you really shouldn't need to do that.
> > It means that something is badly wrong somewhere.
> > Casting pointers to integer types is just asking for a bug.
> 
> That's casting one pointer type to another pointer type.

It is casting the address of some type to a 'u32 *'.
This will then be dereferenced by the called function.
So the original type better be 32 bits.

I'm also guessing that sparse was complaining about endianness?
It isn't at all clear that these casts actually fix it.

(Mark: You'll be glad to hear that the office aircon is
broken again - two weeks lead time on the spare part.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
wrote:

> From: Mark Brown
> > Sent: 19 July 2022 11:17
> >
> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > > From: Shengjiu Wang
> >
> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
> &asrc->asrc_format);
> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
> *)&asrc->asrc_format);
> >
> > > Ugg, you really shouldn't need to do that.
> > > It means that something is badly wrong somewhere.
> > > Casting pointers to integer types is just asking for a bug.
> >
> > That's casting one pointer type to another pointer type.
>
> It is casting the address of some type to a 'u32 *'.
> This will then be dereferenced by the called function.
> So the original type better be 32 bits.
>
> I'm also guessing that sparse was complaining about endianness?
> It isn't at all clear that these casts actually fix it.
>
> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.

The code in include/sound/pcm.h also does such __force cast.
#define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
int)SNDRV_PCM_FORMAT_##fmt)

The change I have made does not cause an issue.

Best regards
Wang shengjiu


> (Mark: You'll be glad to hear that the office aircon is
> broken again - two weeks lead time on the spare part.)
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
>

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
wrote:

> From: Mark Brown
> > Sent: 19 July 2022 11:17
> >
> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > > From: Shengjiu Wang
> >
> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
> &asrc->asrc_format);
> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
> *)&asrc->asrc_format);
> >
> > > Ugg, you really shouldn't need to do that.
> > > It means that something is badly wrong somewhere.
> > > Casting pointers to integer types is just asking for a bug.
> >
> > That's casting one pointer type to another pointer type.
>
> It is casting the address of some type to a 'u32 *'.
> This will then be dereferenced by the called function.
> So the original type better be 32 bits.
>
> I'm also guessing that sparse was complaining about endianness?
> It isn't at all clear that these casts actually fix it.
>
> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.

The code in include/sound/pcm.h also does such __force cast.
#define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
int)SNDRV_PCM_FORMAT_##fmt)

The change I have made does not cause an issue.

Best regards
Wang shengjiu


> (Mark: You'll be glad to hear that the office aircon is
> broken again - two weeks lead time on the spare part.)
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
>

[-- Attachment #2: Type: text/html, Size: 2294 bytes --]

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

grrr... top-posting because outluck is really stupid :-(

The definition seems to be:
typedef int __bitwise<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
#define SNDRV_PCM_FORMAT_S8<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>    ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
#define SNDRV_PCM_FORMAT_U8<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>    ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
#define SNDRV_PCM_FORMAT_S16_LE<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>        ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
...
(goes away and looks up __bitwIse)

I think I’d add:
#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
and use that to remove most of the casts.
But the ones where you have (u32 *)&xxx are only valid because u32 and int are the same size.
That does sort of happen to be true, but someone might look at all the values and
decide that u8 is big enough.
After which the code will still compile, but the data areas get corrupted.
So you really need to use a u32 ‘temp’ variable.

It would all be slightly less problematic if the ‘force’ casts could be sparse only
(ie not seen by the compiler) – so the compiler would do the type checking.

                David

From: Shengjiu Wang <shengjiu.wang@gmail.com>
Sent: 19 July 2022 12:07
To: David Laight <David.Laight@ACULAB.COM>
Cc: Mark Brown <broonie@kernel.org>; Shengjiu Wang <shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com; nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz; tiwai@suse.com; alsa-devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type



On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com<mailto:David.Laight@aculab.com>> wrote:
From: Mark Brown
> Sent: 19 July 2022 11:17
>
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > From: Shengjiu Wang
>
> > > - ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
>
> > Ugg, you really shouldn't need to do that.
> > It means that something is badly wrong somewhere.
> > Casting pointers to integer types is just asking for a bug.
>
> That's casting one pointer type to another pointer type.

It is casting the address of some type to a 'u32 *'.
This will then be dereferenced by the called function.
So the original type better be 32 bits.

I'm also guessing that sparse was complaining about endianness?
It isn't at all clear that these casts actually fix it.
The sparse is complaining about the snd_pcm_format_t cast to u32/int type.

The code in include/sound/pcm.h also does such __force cast.
#define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)

The change I have made does not cause an issue.

Best regards
Wang shengjiu

(Mark: You'll be glad to hear that the office aircon is
broken again - two weeks lead time on the spare part.)

        David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[David Laight]

[-- Attachment #1: Type: text/plain, Size: 3873 bytes --]

grrr... top-posting because outluck is really stupid :-(

The definition seems to be:
typedef int __bitwise<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
#define SNDRV_PCM_FORMAT_S8<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>    ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
#define SNDRV_PCM_FORMAT_U8<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>    ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
#define SNDRV_PCM_FORMAT_S16_LE<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>        ((__force<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force> snd_pcm_format_t<https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
...
(goes away and looks up __bitwIse)

I think I’d add:
#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
and use that to remove most of the casts.
But the ones where you have (u32 *)&xxx are only valid because u32 and int are the same size.
That does sort of happen to be true, but someone might look at all the values and
decide that u8 is big enough.
After which the code will still compile, but the data areas get corrupted.
So you really need to use a u32 ‘temp’ variable.

It would all be slightly less problematic if the ‘force’ casts could be sparse only
(ie not seen by the compiler) – so the compiler would do the type checking.

                David

From: Shengjiu Wang <shengjiu.wang@gmail.com>
Sent: 19 July 2022 12:07
To: David Laight <David.Laight@ACULAB.COM>
Cc: Mark Brown <broonie@kernel.org>; Shengjiu Wang <shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com; nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz; tiwai@suse.com; alsa-devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type



On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com<mailto:David.Laight@aculab.com>> wrote:
From: Mark Brown
> Sent: 19 July 2022 11:17
>
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > From: Shengjiu Wang
>
> > > - ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);
> > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);
>
> > Ugg, you really shouldn't need to do that.
> > It means that something is badly wrong somewhere.
> > Casting pointers to integer types is just asking for a bug.
>
> That's casting one pointer type to another pointer type.

It is casting the address of some type to a 'u32 *'.
This will then be dereferenced by the called function.
So the original type better be 32 bits.

I'm also guessing that sparse was complaining about endianness?
It isn't at all clear that these casts actually fix it.
The sparse is complaining about the snd_pcm_format_t cast to u32/int type.

The code in include/sound/pcm.h also does such __force cast.
#define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)

The change I have made does not cause an issue.

Best regards
Wang shengjiu

(Mark: You'll be glad to hear that the office aircon is
broken again - two weeks lead time on the spare part.)

        David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[-- Attachment #2: Type: text/html, Size: 9836 bytes --]

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

On Tue, Jul 19, 2022 at 8:39 PM David Laight <David.Laight@aculab.com>
wrote:

> grrr... top-posting because outluck is really stupid :-(
>
>
>
> The definition seems to be:
>
> typedef int __bitwise
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
>
> #define SNDRV_PCM_FORMAT_S8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
>
> #define SNDRV_PCM_FORMAT_U8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
>
> #define SNDRV_PCM_FORMAT_S16_LE
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
>
> ...
>
> (goes away and looks up __bitwIse)
>
>
>
> I think I’d add:
>
> #define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
>

Where is this definition? Which header file?
Thanks.

Best regards
Wang Shengjiu

> and use that to remove most of the casts.
>
But the ones where you have (u32 *)&xxx are only valid because u32 and int
> are the same size.
>
> That does sort of happen to be true, but someone might look at all the
> values and
>
> decide that u8 is big enough.
>
> After which the code will still compile, but the data areas get corrupted.
>
> So you really need to use a u32 ‘temp’ variable.
>
>
>
> It would all be slightly less problematic if the ‘force’ casts could be
> sparse only
>
> (ie not seen by the compiler) – so the compiler would do the type checking.
>
>
>
>                 David
>
>
>
> *From:* Shengjiu Wang <shengjiu.wang@gmail.com>
> *Sent:* 19 July 2022 12:07
> *To:* David Laight <David.Laight@ACULAB.COM>
> *Cc:* Mark Brown <broonie@kernel.org>; Shengjiu Wang <
> shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com;
> nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz;
> tiwai@suse.com; alsa-devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> *Subject:* Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the
> asrc_format type
>
>
>
>
>
>
>
> On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
> wrote:
>
> From: Mark Brown
> > Sent: 19 July 2022 11:17
> >
> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > > From: Shengjiu Wang
> >
> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
> &asrc->asrc_format);
> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
> *)&asrc->asrc_format);
> >
> > > Ugg, you really shouldn't need to do that.
> > > It means that something is badly wrong somewhere.
> > > Casting pointers to integer types is just asking for a bug.
> >
> > That's casting one pointer type to another pointer type.
>
> It is casting the address of some type to a 'u32 *'.
> This will then be dereferenced by the called function.
> So the original type better be 32 bits.
>
> I'm also guessing that sparse was complaining about endianness?
> It isn't at all clear that these casts actually fix it.
>
> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.
>
>
>
> The code in include/sound/pcm.h also does such __force cast.
>
> #define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
> int)SNDRV_PCM_FORMAT_##fmt)
>
>
>
> The change I have made does not cause an issue.
>
>
>
> Best regards
>
> Wang shengjiu
>
>
>
> (Mark: You'll be glad to hear that the office aircon is
> broken again - two weeks lead time on the spare part.)
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
>
>
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
> P *Please consider the environment and don't print this e-mail unless you
> really need to*
>

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]

On Tue, Jul 19, 2022 at 8:39 PM David Laight <David.Laight@aculab.com>
wrote:

> grrr... top-posting because outluck is really stupid :-(
>
>
>
> The definition seems to be:
>
> typedef int __bitwise
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
>
> #define SNDRV_PCM_FORMAT_S8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
>
> #define SNDRV_PCM_FORMAT_U8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
>
> #define SNDRV_PCM_FORMAT_S16_LE
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
>
> ...
>
> (goes away and looks up __bitwIse)
>
>
>
> I think I’d add:
>
> #define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
>

Where is this definition? Which header file?
Thanks.

Best regards
Wang Shengjiu

> and use that to remove most of the casts.
>
But the ones where you have (u32 *)&xxx are only valid because u32 and int
> are the same size.
>
> That does sort of happen to be true, but someone might look at all the
> values and
>
> decide that u8 is big enough.
>
> After which the code will still compile, but the data areas get corrupted.
>
> So you really need to use a u32 ‘temp’ variable.
>
>
>
> It would all be slightly less problematic if the ‘force’ casts could be
> sparse only
>
> (ie not seen by the compiler) – so the compiler would do the type checking.
>
>
>
>                 David
>
>
>
> *From:* Shengjiu Wang <shengjiu.wang@gmail.com>
> *Sent:* 19 July 2022 12:07
> *To:* David Laight <David.Laight@ACULAB.COM>
> *Cc:* Mark Brown <broonie@kernel.org>; Shengjiu Wang <
> shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com;
> nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz;
> tiwai@suse.com; alsa-devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> *Subject:* Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the
> asrc_format type
>
>
>
>
>
>
>
> On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
> wrote:
>
> From: Mark Brown
> > Sent: 19 July 2022 11:17
> >
> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > > From: Shengjiu Wang
> >
> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
> &asrc->asrc_format);
> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
> *)&asrc->asrc_format);
> >
> > > Ugg, you really shouldn't need to do that.
> > > It means that something is badly wrong somewhere.
> > > Casting pointers to integer types is just asking for a bug.
> >
> > That's casting one pointer type to another pointer type.
>
> It is casting the address of some type to a 'u32 *'.
> This will then be dereferenced by the called function.
> So the original type better be 32 bits.
>
> I'm also guessing that sparse was complaining about endianness?
> It isn't at all clear that these casts actually fix it.
>
> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.
>
>
>
> The code in include/sound/pcm.h also does such __force cast.
>
> #define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
> int)SNDRV_PCM_FORMAT_##fmt)
>
>
>
> The change I have made does not cause an issue.
>
>
>
> Best regards
>
> Wang shengjiu
>
>
>
> (Mark: You'll be glad to hear that the office aircon is
> broken again - two weeks lead time on the spare part.)
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
>
>
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
> P *Please consider the environment and don't print this e-mail unless you
> really need to*
>

[-- Attachment #2: Type: text/html, Size: 10204 bytes --]

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

Hi David

On Tue, Jul 19, 2022 at 9:35 PM Shengjiu Wang <shengjiu.wang@gmail.com>
wrote:

>
>
> On Tue, Jul 19, 2022 at 8:39 PM David Laight <David.Laight@aculab.com>
> wrote:
>
>> grrr... top-posting because outluck is really stupid :-(
>>
>>
>>
>> The definition seems to be:
>>
>> typedef int __bitwise
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
>>
>> #define SNDRV_PCM_FORMAT_S8
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
>>
>> #define SNDRV_PCM_FORMAT_U8
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
>>
>> #define SNDRV_PCM_FORMAT_S16_LE
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
>>
>> ...
>>
>> (goes away and looks up __bitwIse)
>>
>>
>>
>> I think I’d add:
>>
>> #define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
>>
>
> Where is this definition? Which header file?
> Thanks.
>
>
Here is the change based on your proposal.
Not sure if there is misunderstanding.
Not sure if the definition can be put in pcm.h.

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 26523cfe428d..93e53b195ef9 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1477,6 +1477,8 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t
pcm_format)
        return 1ULL << (__force int) pcm_format;
 }

+#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
+
 /**
  * pcm_for_each_format - helper to iterate for each format type
  * @f: the iterator variable in snd_pcm_format_t type
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 544395efd605..dcfdfb6b3472 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1066,6 +1066,7 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
        struct resource *res;
        void __iomem *regs;
        int irq, ret, i;
+       u32 asrc_fmt = 0;
        u32 map_idx;
        char tmp[16];
        u32 width;
@@ -1174,7 +1175,8 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
                return ret;
        }

-       ret = of_property_read_u32(np, "fsl,asrc-format", (u32
*)&asrc->asrc_format);
+       ret = of_property_read_u32(np, "fsl,asrc-format", &asrc_fmt);
+       asrc->asrc_format = snd_pcm_format(asrc_fmt);
        if (ret) {
                ret = of_property_read_u32(np, "fsl,asrc-width", &width);
                if (ret) {
@@ -1197,7 +1199,7 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
                }
        }

-       if (!(FSL_ASRC_FORMATS & (1ULL << (__force u32)asrc->asrc_format)))
{
+       if (!(FSL_ASRC_FORMATS & pcm_format_to_bits(asrc->asrc_format))) {
                dev_warn(&pdev->dev, "unsupported width, use default
S24_LE\n");
                asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;

best regards
wang shengjiu

>
> and use that to remove most of the casts.
>>
> But the ones where you have (u32 *)&xxx are only valid because u32 and int
>> are the same size.
>>
>> That does sort of happen to be true, but someone might look at all the
>> values and
>>
>> decide that u8 is big enough.
>>
>> After which the code will still compile, but the data areas get corrupted.
>>
>> So you really need to use a u32 ‘temp’ variable.
>>
>>
>>
>> It would all be slightly less problematic if the ‘force’ casts could be
>> sparse only
>>
>> (ie not seen by the compiler) – so the compiler would do the type
>> checking.
>>
>>
>>
>>                 David
>>
>>
>>
>> *From:* Shengjiu Wang <shengjiu.wang@gmail.com>
>> *Sent:* 19 July 2022 12:07
>> *To:* David Laight <David.Laight@ACULAB.COM>
>> *Cc:* Mark Brown <broonie@kernel.org>; Shengjiu Wang <
>> shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com;
>> nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz;
>> tiwai@suse.com; alsa-devel@alsa-project.org;
>> linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
>> *Subject:* Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the
>> asrc_format type
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
>> wrote:
>>
>> From: Mark Brown
>> > Sent: 19 July 2022 11:17
>> >
>> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
>> > > From: Shengjiu Wang
>> >
>> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
>> &asrc->asrc_format);
>> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
>> *)&asrc->asrc_format);
>> >
>> > > Ugg, you really shouldn't need to do that.
>> > > It means that something is badly wrong somewhere.
>> > > Casting pointers to integer types is just asking for a bug.
>> >
>> > That's casting one pointer type to another pointer type.
>>
>> It is casting the address of some type to a 'u32 *'.
>> This will then be dereferenced by the called function.
>> So the original type better be 32 bits.
>>
>> I'm also guessing that sparse was complaining about endianness?
>> It isn't at all clear that these casts actually fix it.
>>
>> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.
>>
>>
>>
>> The code in include/sound/pcm.h also does such __force cast.
>>
>> #define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
>> int)SNDRV_PCM_FORMAT_##fmt)
>>
>>
>>
>> The change I have made does not cause an issue.
>>
>>
>>
>> Best regards
>>
>> Wang shengjiu
>>
>>
>>
>> (Mark: You'll be glad to hear that the office aircon is
>> broken again - two weeks lead time on the spare part.)
>>
>>         David
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
>> 1PT, UK
>> Registration No: 1397386 (Wales)
>>
>>
>>
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
>> 1PT, UK
>> Registration No: 1397386 (Wales)
>>
>> P *Please consider the environment and don't print this e-mail unless
>> you really need to*
>>
>

Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang]

[-- Attachment #1: Type: text/plain, Size: 6670 bytes --]

Hi David

On Tue, Jul 19, 2022 at 9:35 PM Shengjiu Wang <shengjiu.wang@gmail.com>
wrote:

>
>
> On Tue, Jul 19, 2022 at 8:39 PM David Laight <David.Laight@aculab.com>
> wrote:
>
>> grrr... top-posting because outluck is really stupid :-(
>>
>>
>>
>> The definition seems to be:
>>
>> typedef int __bitwise
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
>>
>> #define SNDRV_PCM_FORMAT_S8
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
>>
>> #define SNDRV_PCM_FORMAT_U8
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
>>
>> #define SNDRV_PCM_FORMAT_S16_LE
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>
>> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
>> snd_pcm_format_t
>> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
>>
>> ...
>>
>> (goes away and looks up __bitwIse)
>>
>>
>>
>> I think I’d add:
>>
>> #define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
>>
>
> Where is this definition? Which header file?
> Thanks.
>
>
Here is the change based on your proposal.
Not sure if there is misunderstanding.
Not sure if the definition can be put in pcm.h.

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 26523cfe428d..93e53b195ef9 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1477,6 +1477,8 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t
pcm_format)
        return 1ULL << (__force int) pcm_format;
 }

+#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
+
 /**
  * pcm_for_each_format - helper to iterate for each format type
  * @f: the iterator variable in snd_pcm_format_t type
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 544395efd605..dcfdfb6b3472 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1066,6 +1066,7 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
        struct resource *res;
        void __iomem *regs;
        int irq, ret, i;
+       u32 asrc_fmt = 0;
        u32 map_idx;
        char tmp[16];
        u32 width;
@@ -1174,7 +1175,8 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
                return ret;
        }

-       ret = of_property_read_u32(np, "fsl,asrc-format", (u32
*)&asrc->asrc_format);
+       ret = of_property_read_u32(np, "fsl,asrc-format", &asrc_fmt);
+       asrc->asrc_format = snd_pcm_format(asrc_fmt);
        if (ret) {
                ret = of_property_read_u32(np, "fsl,asrc-width", &width);
                if (ret) {
@@ -1197,7 +1199,7 @@ static int fsl_asrc_probe(struct platform_device
*pdev)
                }
        }

-       if (!(FSL_ASRC_FORMATS & (1ULL << (__force u32)asrc->asrc_format)))
{
+       if (!(FSL_ASRC_FORMATS & pcm_format_to_bits(asrc->asrc_format))) {
                dev_warn(&pdev->dev, "unsupported width, use default
S24_LE\n");
                asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;

best regards
wang shengjiu

>
> and use that to remove most of the casts.
>>
> But the ones where you have (u32 *)&xxx are only valid because u32 and int
>> are the same size.
>>
>> That does sort of happen to be true, but someone might look at all the
>> values and
>>
>> decide that u8 is big enough.
>>
>> After which the code will still compile, but the data areas get corrupted.
>>
>> So you really need to use a u32 ‘temp’ variable.
>>
>>
>>
>> It would all be slightly less problematic if the ‘force’ casts could be
>> sparse only
>>
>> (ie not seen by the compiler) – so the compiler would do the type
>> checking.
>>
>>
>>
>>                 David
>>
>>
>>
>> *From:* Shengjiu Wang <shengjiu.wang@gmail.com>
>> *Sent:* 19 July 2022 12:07
>> *To:* David Laight <David.Laight@ACULAB.COM>
>> *Cc:* Mark Brown <broonie@kernel.org>; Shengjiu Wang <
>> shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com;
>> nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz;
>> tiwai@suse.com; alsa-devel@alsa-project.org;
>> linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
>> *Subject:* Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the
>> asrc_format type
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
>> wrote:
>>
>> From: Mark Brown
>> > Sent: 19 July 2022 11:17
>> >
>> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
>> > > From: Shengjiu Wang
>> >
>> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
>> &asrc->asrc_format);
>> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
>> *)&asrc->asrc_format);
>> >
>> > > Ugg, you really shouldn't need to do that.
>> > > It means that something is badly wrong somewhere.
>> > > Casting pointers to integer types is just asking for a bug.
>> >
>> > That's casting one pointer type to another pointer type.
>>
>> It is casting the address of some type to a 'u32 *'.
>> This will then be dereferenced by the called function.
>> So the original type better be 32 bits.
>>
>> I'm also guessing that sparse was complaining about endianness?
>> It isn't at all clear that these casts actually fix it.
>>
>> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.
>>
>>
>>
>> The code in include/sound/pcm.h also does such __force cast.
>>
>> #define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
>> int)SNDRV_PCM_FORMAT_##fmt)
>>
>>
>>
>> The change I have made does not cause an issue.
>>
>>
>>
>> Best regards
>>
>> Wang shengjiu
>>
>>
>>
>> (Mark: You'll be glad to hear that the office aircon is
>> broken again - two weeks lead time on the spare part.)
>>
>>         David
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
>> 1PT, UK
>> Registration No: 1397386 (Wales)
>>
>>
>>
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
>> 1PT, UK
>> Registration No: 1397386 (Wales)
>>
>> P *Please consider the environment and don't print this e-mail unless
>> you really need to*
>>
>

[-- Attachment #2: Type: text/html, Size: 12856 bytes --]