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.