On Thu, Jun 17, 2021 at 9:37 AM Sai Krishna Potthuri
[off-list ref] wrote:

Ping!

Do not top-post.
Do not ping for the sake of pings.

quoted

From: Sai Krishna Potthuri <redacted>
Sent: Tuesday, May 11, 2021 6:08 PM

quoted

From: Sai Krishna Potthuri
Sent: Wednesday, April 28, 2021 11:04 AM

quoted

From: Andy Shevchenko <redacted>
Sent: Monday, April 26, 2021 7:35 PM

...

quoted

quoted

quoted

quoted

quoted

quoted

+               ret = zynqmp_pm_pinctrl_get_config(pin, param, &arg);
+               if (arg != PM_PINCTRL_BIAS_PULL_UP)
+                       return -EINVAL;

Error code being shadowed. Instead check it here properly.

quoted

Are you mentioning the case where ret is also a non-zero?
If yes, then I will update this check to if (!ret && arg !=
PM_PINCTRL_BIAS_PULL_UP)
        return -EINVAL;

No, this is wrong in the same way.

quoted

ret non-zero case, we are handling at the end of switch case.

I meant that you need to pass the real return code to the (upper) caller.

Here we are checking for valid argument and not the return value of the

API.

quoted

If the read value(argument) is not valid and return value of the API
is zero (SUCCESS) then framework expects driver to be returned with
'-EINVAL' and it is a legal error code in this case.

Do you agree on this?
I am ready with the other changes, will send out the patch to address your
comments.

If you are ready, drop a new version (note, ~1w is a good time to send
a new version if no one answered the doubts in your previous one,
which means "silent agreement").

-- 
With Best Regards,
Andy Shevchenko