On 2023-09-19 12:07 PM, Jaroslav Kysela wrote:
On 19. 09. 23 11:28, Cezary Rojewski wrote:
...
2) if you copy 90% of my code, I don't think that Suggested-by: tag
is fine
Could you do your work on top of my patch?
I'm afraid this isn't a fair claim. The feature is driven by validation
and this has been conducted be me or my folks entirely. Given the scarce
guidance provided in [1] I still provided a valid WIP in [2] and
expected to iterate over it given the feedback. Closing the discussion
by taking a single patch away from the series and re-authoring it is not
a welcoming way to do a review. Perhaps Co-developed-by: then?
I don't follow you. My patch can be also changed - I've not heard any
review except cosmetic changes. I am just telling you that the patch is
a good base for all other changes. I think that the best way is to
finish the base extension in sound/core at first without any helpers and
so on and then work on other parts.
(save)
Please note that the code differs. I do believe that splitting the API
and the constrains into separate patches is a better approach from
maintenance point of view.
It does not make sense to extend API without constraints. The splitting
does not help here.
Proposed readability improvements have also
been applied in v2. For reasons provided in previous paragraphs, I
chose > to avoid the chicken-bit and treat subformat constraints in
generic
fashion. Also, validation shows that without updating
snd_pcm_subformat_names[] in pcm.c the code ends with UBSAN during
Yes, I missed that. I can put it to my v3 when we agree on the constraints.
runtime. I've already addressed that, even in v1.
I'm happy to continue the technical discussion as there are still points
to discuss. Let's do so in v2 of the series [3].
Unfortunately, you are not working with the technical discussion anyway.
Changing comments adding empty lines, renaming variables to make you
happy is not a nice way to co-operate with others and then act as the
author of the CODE (not comments) is really bad. Everyone has own coding
style and you're forcing your opinion.
We have a different view on the subject. Readability shall be treated on
the same level as performance is. It should not be marginalized.
When taking a look at RFC series [1] and the WIP [2] there's a clear
resemblance to the code here. I believe the differences between this and
v2 [3] are also being skipped over. And as stated previously, closing
the review by separating a patch from the series and sending it without
any references to the original series and modifying the CC list is not nice.
What I propose is: leave the MSBITS-introduction and MSBITS-constraints
patches split. Have my authorship for the former and yours for the
latter with relevant Co-developed-by: tags added in both of them. Is
this proposal acceptable from your perspective?
Kind regards,
Czarek
[1]:
https://lore.kernel.org/alsa-devel/20230811164853.1797547-1-cezary.rojewski@xxxxxxxxx/
[2]:
https://lore.kernel.org/alsa-devel/8d76a1d8-e85c-b699-34a0-ecea6edc2fe1@xxxxxxxxx/
[3]:
https://lore.kernel.org/alsa-devel/20230918133940.3676091-1-cezary.rojewski@xxxxxxxxx/
Also, I though about {} end-of-array mark (remove
SNDRV_PCM_FORMAT_CONSTRAINT_END), but I found that I prefer to have the
possibility to skip the MSBITS_MAX settings for the given format. It may
make sense in a situation when the MSBITS configuration is too rare to
be added as the API bit.
