Re: [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux

[Peter Rosin]

On 2019-02-26 07:08, Pankaj Bansal wrote:
>>> +	if (IS_ERR_OR_NULL(regmap)) {
>>> +		ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : -
>> ENODEV;
>>
>> The above is not correct, this should be better (untested):
>>
>> 		ret = PTR_ERR(regmap) ?: -ENODEV;
> 
> Omitting the second operand in ternary operator is not standard. https://stackoverflow.com/questions/34559705/ternary-operator-without-the-middle-expression

Irrelevant, a great many non-standard features are used in the kernel.

> Although, it *has been* used in kernel in many places
> https://livegrep.com/search/linux?q=file%3A%5C.c%24%20%5C%3F%5C%3A&fold_case=auto&regex=true&context=true

Yes, a local grep suggests that there are plenty of instances in the
kernel. In fact, there are multiple instances even in the kernel/
directory so it's not some fringe thing. My conclusion is that it
makes for readable code and is perfectly safe to use in the kernel,
which apparently does not care about compilers that do not understand
the construct.

$ git grep '?:' *.[ch] | wc
    773    4830   54975

I you still don't buy that and don't want to sign-off on it, then
write

		ret = IS_ERR(regmap) ? PTR_ERR(regmap) : -ENODEV;

But that's just wordy and less readable, methinks.

However, the best thing to do, according to your maintainer, is still
the original suggestion to fixup the NULL as early as possible with

		regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV);

And leave the error handling block alone. Can you please just do that
and be done with it?

Cheers,
Peter