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!