On Mon, Apr 01, 2024 at 12:53:18PM +0200, Oswald Buddenhagen wrote: > On Mon, Apr 01, 2024 at 11:37:27AM +0100, Russell King (Oracle) wrote: > > runtime->hw.fifo_size is only measured in _frames_ if > > SNDRV_PCM_INFO_FIFO_IN_FRAMES is set. > > > yes, which is exactly why the other hunk in the patch sets it. > > > So now I have to ask what caused you to generate this patch. I don't > > think you've actually run this driver, so presumably it's by [] > > code inspection... > > > yes, it was a random find while trying to make sense of this parameter. The driver worked when I wrote it. The fifo_size contents was correct when I wrote it. The choice for using bytes here and not setting SNDRV_PCM_INFO_FIFO_IN_FRAMES means that ALSA correctly takes the real FIFO size (which is in bytes) and correctly translates that itself into frames. I fail to see how this is any better - in fact, I think it's a lot worse because, as you've pointed out, it doesn't deal with stereo. In fact, it only supports 16-bit mono, whereas the driver supports lots more than that. I think where the confusion comes from is that fifo_depth is the depth of the hardware FIFO in units of 16-bit quantities, which is why we multiply by two to get to bytes. 16-bit quantities does not necessarily equate to ALSA frames - it can be in specific cases but not always. As far as I'm concerned, the code is correct as it stands and your patch will introduce regressions. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
