On Wed, 29 Jan 2020 18:07:20 +0100,
Takashi Iwai wrote:
>
> On Wed, 29 Jan 2020 18:01:00 +0100,
> Pierre-Louis Bossart wrote:
> >
> >
> >
> > >>>> We're seeing the following sparse warning in the SOF code:
> > >>>>
> > >>>> sound/soc/sof/sof-audio.c:86:31: warning: incorrect type in assignment (different base types)
> > >>>> sound/soc/sof/sof-audio.c:86:31: expected restricted snd_pcm_state_t [usertype] state
> > >>>> sound/soc/sof/sof-audio.c:86:31: got signed int [signed] [usertype] [explicitly-signed] state
> > >>>>
> > >>>> The line under scrutiny where we assign "state" is as follows:
> > >>>>
> > >>>> state = substream->runtime->status->state;
> > >>>>
> > >>>> and it is defined as
> > >>>> snd_pcm_state_t state;
> > >>>>
> > >>>> There are other places (ex pcm_oss.c) where a similar assignment has been used
> > >>>> as well.
> > >>>>
> > >>>> What fixes the issue is a forced cast to snd_pcm_state_t as below before
> > >>>> assigning:
> > >>>> state = (__force snd_pcm_state_t)substream->runtime->status->state;
> > >>>>
> > >>>> Do you think this is acceptable? If not, could you please suggest an
> > >>>> alternative?
> > >>>
> > >>> Hm, I don't see the warning in my code. Did you merge all upstream
> > >>> stuff and still get it?
> > >>
> > >> It's been merged in the SOF tree (topic/sof-dev) and it'll be in the
> > >> next batch I send. It's not upstream just yet because we want to
> > >> remove the warning to make the patch nice and shiny :-)
> > >
> > > I still wonder why I couldn't get it on my tree. I guess the code
> > > around that hasn't changed, right?
> > >
> > > Wait... Is it a test on 32bit arch? If so, it might be a new thing
> > > for y2038 support. Then we may fix the uapi definition instead.
> >
> > It's actually in your tree already, my mistake, see:
> >
> > ee1e79b72e3cf ("ASoC: SOF: partition audio-related parts from SOF core")
> >
> > It's with plain-vanilla x86_64, I can share the config.
>
> Sorry, my bad, it seems that my sparse program was too old. After
> upgrading to the latest version, I could see the warning, too.
... and the cause was what I suspected: it's a 64bit compat type that
defines the fields explicitly with __s32. I overlooked that it's
always used for __KERNEL__. The tentative fix is below.
thanks,
Takashi
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -564,13 +564,13 @@ typedef char __pad_after_uframe[sizeof(__u64) - sizeof(snd_pcm_uframes_t)];
#endif
struct __snd_pcm_mmap_status64 {
- __s32 state; /* RO: state - SNDRV_PCM_STATE_XXXX */
+ snd_pcm_state_t state; /* RO: state - SNDRV_PCM_STATE_XXXX */
__u32 pad1; /* Needed for 64 bit alignment */
__pad_before_uframe __pad1;
snd_pcm_uframes_t hw_ptr; /* RO: hw ptr (0...boundary-1) */
__pad_after_uframe __pad2;
struct __snd_timespec64 tstamp; /* Timestamp */
- __s32 suspended_state; /* RO: suspended stream state */
+ snd_pcm_state_t suspended_state; /* RO: suspended stream state */
__u32 pad3; /* Needed for 64 bit alignment */
struct __snd_timespec64 audio_tstamp; /* sample counter or wall clock */
};
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
