Re: [PATCH 5/6] media: platform: Use str_enable_disable-like helpers

[Hans Verkuil <hverkuil@xs4all.nl>]

On 1/15/25 08:47, Tomasz Figa wrote:
> On Wed, Jan 15, 2025 at 4:39 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
>>
>> Hi Laurent,
>>
>> On Tue, Jan 14, 2025 at 10:42:40PM +0200, Laurent Pinchart wrote:
>>> Hi Krzysztof,
>>>
>>> Thank you for the patch.
>>>
>>> On Tue, Jan 14, 2025 at 08:46:21PM +0100, Krzysztof Kozlowski wrote:
>>>> Replace ternary (condition ? "enable" : "disable") syntax with helpers
>>>> from string_choices.h because:
>>>> 1. Simple function call with one argument is easier to read.  Ternary
>>>>    operator has three arguments and with wrapping might lead to quite
>>>>    long code.
>>>
>>> It's more difficult to read for me.
>>
>> I don't have any issue in using the ternary operator either. Using these
>> helpers makes the lines generally 3 characters shorter.
>>
>>>
>>>> 2. Is slightly shorter thus also easier to read.
>>>> 3. It brings uniformity in the text - same string.
>>>> 4. Allows deduping by the linker, which results in a smaller binary
>>>>    file.
>>>
>>> I don't see why the linker can't de-dup string in the current code.
>>
>> In fact the functions are static inline so from that point of view I don't
>> think there's any difference.
>>
>>>
>>> I'm sorry, I just don't see the point in doing this. I'd like to avoid
>>> those changes in the Linux media subsystem, or at the very least in
>>> drivers I maintain.
>>
>> I don't have much of an opinion, perhaps I slightly prefer using these as
>> the rest of the kernel does, too. Yet if we choose not to use these
>> helpers, we continue to be occasional targets of largish patchsets "fixing"
>> this.
> 
> To put one more aspect on the scales:
> 
> These kinds of patches actually make it more difficult to backport
> changes (e.g. fixes) to stable kernels, so my preference would be to
> only use the new helpers in new drivers.

I agree with Tomasz. Now, if the whole kernel is converting to these
new functions, then I guess we should follow, but from what I can tell
that doesn't appear to be the case.

I'll reject this series. It can always be resurrected if there is
sufficient demand for this.

Regards,

	Hans

> 
> Best regards,
> Tomasz
>