[Sound-open-firmware] can stream-box size be 0?

[Guennadi Liakhovetski]

On Wed, Apr 03, 2019 at 04:22:42PM +0100, Liam Girdwood wrote:
> On Wed, 2019-04-03 at 17:18 +0200, Guennadi Liakhovetski wrote:
> > On Wed, Apr 03, 2019 at 03:56:22PM +0100, Liam Girdwood wrote:
> > > On Wed, 2019-04-03 at 16:49 +0200, Guennadi Liakhovetski wrote:
> > > > On Wed, Apr 03, 2019 at 03:39:02PM +0100, Liam Girdwood wrote:
> > > > > On Wed, 2019-04-03 at 15:33 +0200, Guennadi Liakhovetski wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > In kernel ipc.c I see "if (sdev->stream_box.size == 0)" checks at
> > > > > > exactly 4 locations, but all currently supported by SOF platforms set
> > > > > > the size to 0x200. Do we need those checks or can we safely assume,
> > > > > > that
> > > > > > the size is never 0?
> > > > > 
> > > > > The size can depend on FW platform and how much space they allocate for
> > > > > mmap()
> > > > > stream position information. i.e. ALSA can read pipeline stream position
> > > > > stats
> > > > > directly from mailbox rather than via IPC.
> > > > 
> > > > Does that mean, that I misunderstood something? Could you point me out
> > > > to the code, that can set that size to anything other than 0x200, as
> > > > supplied by the firmware?
> > > > 
> > > 
> > > This size can be anything, even 0 is valid. Look at the extended boot data
> > > sent
> > > by the FW at boot in platform.c (e.g. src/platform/baytrail/platform.c)
> > 
> > Ok, right, I understand that. BTW, a correction: it's actually 0x1000
> > (4096) on cAVS platforms. But anyway: I understand that it's
> > configurable by the firmware and at least in theory it's allowed to be
> > 0. But do we have any platforms where it is 0? I don't see any. Have we
> > ever tested such a case? The kernel has code that checks it for 0 (see
> > above), but if we don't have any such platforms it means, those paths
> > are never entered and never tested. Looking at them their handling also
> > doesn't seem entirely clear to me. So my question is: do we really want
> > and need to be able at least in theory to use 0 there and keep that code
> > and hope, that it works and never breaks, or can we remove that code and
> > put a check and an failure if it's 0?
> > 
> 
> Keep the check, some platforms may not have a large enough mailbox and depend on
> IPC for all stream position updates.

Ok, I'll try to test the 0-size case.

Thanks
Guennadi