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

[Sakari Ailus <sakari.ailus@linux.intel.com>]

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.

-- 
Kind regards,

Sakari Ailus