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

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

On Mon, Apr 01, 2024 at 04:17:53PM +0200, Oswald Buddenhagen wrote:
> On Mon, Apr 01, 2024 at 02:34:20PM +0100, Russell King (Oracle) wrote:
> > Oh FFS. So you generated a patch based on the contents of a mere
> > comment? That is NOT how kernel development should be done. Do not do
> > this.
> > 
> 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.

> > 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.

"As we only support 16-bit samples"

That is also a fact - the driver doesn't support anything but 16-bit
samples.

"this is twice the FIFO depth irrespective of whether it's in compact
mode or not."

The only ambiguity there is what "compact" mode is, and one can find
that out by reading the documentation referenced at the top of the file
which is a public document.

Why should the comment go into all the nitty gritty that is described
in the _public_ document, like "the FIFO is shared between all channels"
and "the FIFO has a fixed width". Maybe it should also state that
compact mode is reading 32-bits per transfer and thus takes up two FIFO
entries. Maybe it should describe that each 32-bit transfer contains
two samples. Maybe it should describe the bit order of those samples.
Maybe it should describe what a computer is as well?

At some point, knowledge has to be assumed. I assume that, because the
public document is referenced at the top of the file, anyone who wishes
to patch this driver should refer to the public documentation to get an
understanding of the hardware first.

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