Re: [PATCH] ALSA: aaci: report FIFO size in frames

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Apr 02, 2024 at 01:01:24AM +0200, Oswald Buddenhagen wrote:
> On Mon, Apr 01, 2024 at 09:59:10PM +0100, Russell King (Oracle) wrote:
> > On Mon, Apr 01, 2024 at 04:17:53PM +0200, Oswald Buddenhagen wrote:
> > > i think that speculative/rfc patches are a perfectly fine way to get
> > > things clarified, and the linux kernel is no exception to that.
> > 
> > This wasn't a "speculative/rfc" patch. Such patches get marked with
> > "RFC" in the tag.
> > 
> putting an obvious disclaimer/question section after a three-dash line
> is a perfectly sufficient way to mark such a patch. at least if the
> receiver is actually interested in cooperation rather than harping on
> formalities.

Convention is it goes in the subject line, so patch automation such as
patchwork can identify the patches that aren't to be applied. It's not
just a formality as you suggest.

> > > > Comments are not always correct.
> > > >
> > > so how about taking the opportunity to fix this one?
> > 
> > I don't think this comment is incorrect.
> > 
> > "ALSA wants the byte-size of the FIFOs."
> > 
> > That is a fact when the flag you refer to is not set.
> > 
> yet the formulation also suggests that this is something that just is,
> rather than something that the code has control over.

Eh what are you going on about.

The fact that SNDRV_PCM_INFO_FIFO_IN_FRAMES is not set means fifo_size
is in bytes. That's a fact. This flag was introduced in commit
8bea869c5e56 ("ALSA: PCM midlevel: improve fifo_size handling") which
was part of v2.6.31-rc1. The driver you are modifying was introduced
in v2.6.13-rc1 *before* this flag was available, and thus from a time
when fifo_size was _only_ _ever_ specifyable in bytes. Maybe the
author of the patch introducing the ability to provide fifo_size in
frames should have gone through every driver checking to see whether
there were any comments to be updated?

> > [...]
> > At some point, knowledge has to be assumed.
> > 
> the problem is the omission of specific information that is in this
> context at least as pertinent as the information that _was_ supplied.
> 
> the code is also somewhat special in that it implements an interface,
> which makes it more likely to be "visited" by outsiders than some
> implementation details. it makes sense to take that into account in
> related comments.

The code is not "somewhat special". The code does what it does and has
done so since before specifying fifo_size in frames was possible.

I think it is your understanding of ALSA that is the problem, and you've
decided that passing fifo_size as bytes is now "somewhat special". It
isn't.

I am still left wondering if this is some perverse April Fool's joke
on your part, designed to provoke a negative reaction. You clearly
don't believe in doing any research. You don't follow the process
described in submitting-patches.rst. You just create broken patches
and send them in a form where they could well be picked up and merged
into mainline causing breakage.

This makes you dangerous, which is why I'm calling you out for it.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!