<div dir="ltr"><div>Hi David</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jul 19, 2022 at 9:35 PM Shengjiu Wang <<a href="mailto:shengjiu.wang@gmail.com">shengjiu.wang@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jul 19, 2022 at 8:39 PM David Laight <<a href="mailto:David.Laight@aculab.com" target="_blank">David.Laight@aculab.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-GB">
<div>
<p class="MsoNormal"><span>grrr... top-posting because outluck is really stupid :-(<u></u><u></u></span></p>
<p class="MsoNormal"><span><u></u> <u></u></span></p>
<p class="MsoNormal"><span>The definition seems to be:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:10pt;font-family:"Courier New"">typedef int
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise" target="_blank">__bitwise</a>
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t" target="_blank">snd_pcm_format_t</a>;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:10pt;font-family:"Courier New"">#define
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8" target="_blank">
SNDRV_PCM_FORMAT_S8</a> ((<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force" target="_blank">__force</a>
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t" target="_blank">snd_pcm_format_t</a>) 0)<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:10pt;font-family:"Courier New"">#define
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8" target="_blank">
SNDRV_PCM_FORMAT_U8</a> ((<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force" target="_blank">__force</a>
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t" target="_blank">snd_pcm_format_t</a>) 1)<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:10pt;font-family:"Courier New"">#define
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE" target="_blank">
SNDRV_PCM_FORMAT_S16_LE</a> ((<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force" target="_blank">__force</a>
<a href="https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t" target="_blank">snd_pcm_format_t</a>) 2)<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:10pt;font-family:"Courier New"">...<u></u><u></u></span></p>
<p class="MsoNormal"><span>(goes away and looks up __bitwIse)<u></u><u></u></span></p>
<p class="MsoNormal"><span><u></u> <u></u></span></p>
<p class="MsoNormal"><span>I think I’d add:<u></u><u></u></span></p>
<p class="MsoNormal"><span>#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))</span></p></div></div></blockquote><div><br></div><div>Where is this definition? Which header file?</div><div>Thanks.</div><div><br></div></div></div></blockquote><div><br></div><div>Here is the change based on your proposal. <br></div><div>Not sure if there is misunderstanding.</div><div>Not sure if the definition can be put in pcm.h. </div><div><br></div>diff --git a/include/sound/pcm.h b/include/sound/pcm.h<br>index 26523cfe428d..93e53b195ef9 100644<br>--- a/include/sound/pcm.h<br>+++ b/include/sound/pcm.h<br>@@ -1477,6 +1477,8 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)<br> return 1ULL << (__force int) pcm_format;<br> }<br><br>+#define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))<br>+<br> /**<br> * pcm_for_each_format - helper to iterate for each format type<br> * @f: the iterator variable in snd_pcm_format_t type<br>diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c<br>index 544395efd605..dcfdfb6b3472 100644<br>--- a/sound/soc/fsl/fsl_asrc.c<br>+++ b/sound/soc/fsl/fsl_asrc.c<br>@@ -1066,6 +1066,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)<br> struct resource *res;<br> void __iomem *regs;<br> int irq, ret, i;<br>+ u32 asrc_fmt = 0;<br> u32 map_idx;<br> char tmp[16];<br> u32 width;<br>@@ -1174,7 +1175,8 @@ static int fsl_asrc_probe(struct platform_device *pdev)<br> return ret;<br> }<br><br>- ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);<br>+ ret = of_property_read_u32(np, "fsl,asrc-format", &asrc_fmt);<br>+ asrc->asrc_format = snd_pcm_format(asrc_fmt);<br> if (ret) {<br> ret = of_property_read_u32(np, "fsl,asrc-width", &width);<br> if (ret) {<br>@@ -1197,7 +1199,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)<br> }<br> }<br><br>- if (!(FSL_ASRC_FORMATS & (1ULL << (__force u32)asrc->asrc_format))) {<br>+ if (!(FSL_ASRC_FORMATS & pcm_format_to_bits(asrc->asrc_format))) {<br> dev_warn(&pdev->dev, "unsupported width, use default S24_LE\n");<br> asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE;<br><div> </div><div>best regards</div><div>wang shengjiu</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-GB"><div><p class="MsoNormal"><span><u></u><u></u></span></p>
<p class="MsoNormal"><span>and use that to remove most of the casts.</span> </p></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-GB"><div><p class="MsoNormal"><span><u></u><u></u></span></p>
<p class="MsoNormal"><span>But the ones where you have (u32 *)&xxx are only valid because u32 and int are the same size.<u></u><u></u></span></p>
<p class="MsoNormal"><span>That does sort of happen to be true, but someone might look at all the values and<u></u><u></u></span></p>
<p class="MsoNormal"><span>decide that u8 is big enough.<u></u><u></u></span></p>
<p class="MsoNormal"><span>After which the code will still compile, but the data areas get corrupted.<u></u><u></u></span></p>
<p class="MsoNormal"><span>So you really need to use a u32 ‘temp’ variable.<u></u><u></u></span></p>
<p class="MsoNormal"><span><u></u> <u></u></span></p>
<p class="MsoNormal"><span>It would all be slightly less problematic if the ‘force’ casts could be sparse only<u></u><u></u></span></p>
<p class="MsoNormal"><span>(ie not seen by the compiler) – so the compiler would do the type checking.<u></u><u></u></span></p>
<p class="MsoNormal"><span><u></u> <u></u></span></p>
<p class="MsoNormal"><span> David<u></u><u></u></span></p>
<p class="MsoNormal"><span><u></u> <u></u></span></p>
<div style="border-top:none;border-right:none;border-bottom:none;border-left:1.5pt solid blue;padding:0cm 0cm 0cm 4pt">
<div>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(225,225,225);padding:3pt 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US">From:</span></b><span lang="EN-US"> Shengjiu Wang <<a href="mailto:shengjiu.wang@gmail.com" target="_blank">shengjiu.wang@gmail.com</a>>
<br>
<b>Sent:</b> 19 July 2022 12:07<br>
<b>To:</b> David Laight <<a href="mailto:David.Laight@ACULAB.COM" target="_blank">David.Laight@ACULAB.COM</a>><br>
<b>Cc:</b> Mark Brown <<a href="mailto:broonie@kernel.org" target="_blank">broonie@kernel.org</a>>; Shengjiu Wang <<a href="mailto:shengjiu.wang@nxp.com" target="_blank">shengjiu.wang@nxp.com</a>>; <a href="mailto:Xiubo.Lee@gmail.com" target="_blank">Xiubo.Lee@gmail.com</a>; <a href="mailto:festevam@gmail.com" target="_blank">festevam@gmail.com</a>; <a href="mailto:nicoleotsuka@gmail.com" target="_blank">nicoleotsuka@gmail.com</a>; <a href="mailto:lgirdwood@gmail.com" target="_blank">lgirdwood@gmail.com</a>; <a href="mailto:perex@perex.cz" target="_blank">perex@perex.cz</a>; <a href="mailto:tiwai@suse.com" target="_blank">tiwai@suse.com</a>; <a href="mailto:alsa-devel@alsa-project.org" target="_blank">alsa-devel@alsa-project.org</a>; <a href="mailto:linuxppc-dev@lists.ozlabs.org" target="_blank">linuxppc-dev@lists.ozlabs.org</a>;
<a href="mailto:linux-kernel@vger.kernel.org" target="_blank">linux-kernel@vger.kernel.org</a><br>
<b>Subject:</b> Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type<u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Jul 19, 2022 at 6:34 PM David Laight <<a href="mailto:David.Laight@aculab.com" target="_blank">David.Laight@aculab.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal" style="margin-bottom:12pt">From: Mark Brown<br>
> Sent: 19 July 2022 11:17<br>
> <br>
> On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:<br>
> > From: Shengjiu Wang<br>
> <br>
> > > - ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format);<br>
> > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32 *)&asrc->asrc_format);<br>
> <br>
> > Ugg, you really shouldn't need to do that.<br>
> > It means that something is badly wrong somewhere.<br>
> > Casting pointers to integer types is just asking for a bug.<br>
> <br>
> That's casting one pointer type to another pointer type.<br>
<br>
It is casting the address of some type to a 'u32 *'.<br>
This will then be dereferenced by the called function.<br>
So the original type better be 32 bits.<br>
<br>
I'm also guessing that sparse was complaining about endianness?<br>
It isn't at all clear that these casts actually fix it.<u></u><u></u></p>
</blockquote>
<div>
<p class="MsoNormal">The sparse is complaining about the snd_pcm_format_t cast to u32/int type.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The code in include/sound/pcm.h also does such __force cast.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">#define _SNDRV_PCM_FMTBIT(fmt) (1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The change I have made does not cause an issue. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Best regards<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Wang shengjiu<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal" style="margin-bottom:12pt">(Mark: You'll be glad to hear that the office aircon is<br>
broken again - two weeks lead time on the spare part.)<br>
<br>
David<br>
<br>
-<br>
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK<br>
Registration No: 1397386 (Wales)<u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</div>
<br><br><div><span style="color:rgb(72,72,48);font-family:Tahoma;font-size:xx-small">Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK<br>Registration No: 1397386 (Wales)</span></div>
<div>
<p><span style="color:rgb(0,128,0);font-family:Arial"><span style="font-size:xx-small"><span style="font-family:Webdings">P </span><strong>Please consider the environment and don't print this e-mail unless you really need to</strong></span></span></p>
</div></div>
</blockquote></div></div>
</blockquote></div></div>