Re: [RFC PATCH v1 22/57] sound: Remove PAGE_SIZE compile-time constant assumption

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

On Mon, Oct 14, 2024 at 01:24:02PM +0100, Ryan Roberts wrote:
> On 14/10/2024 12:38, Mark Brown wrote:
> > On Mon, Oct 14, 2024 at 11:58:29AM +0100, Ryan Roberts wrote:

> >> ***NOTE***
> >> Any confused maintainers may want to read the cover note here for context:
> >> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/

> > As documented in submitting-patches.rst please send patches to the 
> > maintainers for the code you would like to change.  The normal kernel
> > workflow is that people apply patches from their inboxes, if they aren't
> > copied they are likely to not see the patch at all and it is much more
> > difficult to apply patches.

> Sure. I think you're implying that you would have liked to be in To: for this
> patch? I went to quite a lot of trouble to ensure all maintainers were at least
> in the To: field for patches touching their code. But get_maintainer.pl lists
> you as a supporter, not a maintainer when I ran this patch through. Could you
> clarify what would have been the correct thing to do? I could include all
> reviewers and supporters as well as maintainers but then I'd be banging up
> against the limits for some of the patches.

The entry in MAINTAINERS for me is a M:, supporter is just the usual
get_maintainers noise.  Supported is exactly equivalent to a maintainer.
Generally if you're going to filter people you should be filtering less
specific matches out rather than more and if you're looking to filter
very aggressively look at who actually commits changes to whatever
you're trying to change, less specific maintainers will generally
delegate down to the more specific ones.

> > It's probably better to just use PAGE_SIZE_MAX here and avoid the
> > deferred patching, like the comment says we don't particularly care what
> > the value actually is here given that it's a dummy.

> OK, so would that be:

> 	.buffer_bytes_max	= 128*1024,
> 	.period_bytes_min	= PAGE_SIZE_MAX,      <<<<<
> 	.period_bytes_max	= PAGE_SIZE_MAX*2,    <<<<<
> 	.periods_min		= 2,
> 	.periods_max		= 128,

> It's not really clear to me how all the parameters interact; the buffer size
> 128K, which, if PAGE_SIZE_MAX is 64K, would hold 1 period of the maximum size.
> But periods_min is 2. So not sure that works? Or perhaps I'm trying to apply too
> much meaning to the param names...

Like Takashi says just using absolute numbers here is probably just as
sensible, the numbers are there to stop userspace tripping over itself
but like I say it shouldn't ever get as far as actually using them for
anything.  So long as we end up with some numbers that don't need any
late init patching the specifics aren't super important, the use of
PAGE_SIZE was kind of random.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]