[PATCH] phy: Fix generic_setup_phy return value on power on failure

[Jonas Karlman]

On 2023-07-12 03:48, Marek Vasut wrote:
> On 7/12/23 02:06, Jonas Karlman wrote:
>> On 2023-07-12 01:59, Marek Vasut wrote:
>>> On 7/12/23 01:48, Jonas Karlman wrote:
>>>> generic_setup_phy may mask a power on failure due to the return value
>>>> from generic_phy_exit, typically 0, is being used as return value.
>>>>
>>>> Fix this by ignoring the return value of the generic_phy_exit call,
>>>> also remove an unnecessary check for -ENOENT.
>>>
>>> Why is it unnecessary ?
>>> If I recall it right, the ENOENT check is to not fail if the PHY is not
>>> present in DT, which may not be a failure.
>>
>> In the current state ret is returned any way, so this check is currently
>> pointless.
> 
> I don't think so, the test is:
> 
> if (ret && ret != -ENOENT)
>    return ret;
> 
> i.e. the ENOENT is a special case.

Sure, but in this function such check does not affect any outcome.
The function look like:

  if (ret) {
      if (ret != -ENOENT)
          return ret;
  } else {
	  // init and power on
  }
  return ret;

As seen above ret is always returned unless it is 0 in the initial check,
making the ENOENT check unnecessary and misleading in this function.

The old ehci_setup_phy function, see [1], returned 0 instead of ret as
the final return statement, making the ENOENT check meaningful.

This change in behavior may have been unintentional in [2]. Because this
new behavior was merged in v2023.01-rc1, I did not see it appropriate to
revert back to the old ehci_setup_phy behavior and instead just removed
the unnecessary ENOENT check.

I can include a check for ENOENT at the generic_setup_phy call sites if
we want to restore the old behavior?

[1] https://source.denx.de/u-boot/u-boot/-/commit/75341e9c16aa10929a1616060a3b565ecf6e1418
[2] https://source.denx.de/u-boot/u-boot/-/commit/083f8aa978a837542ffa53e5247d36d2b1734e44

Regards,
Jonas

> 
>> Looking back at the old originating code for generic_setup_phy the
>> return value was 0 for similar ENOENT check.
>>
>> Maybe this is something that should be restored or such check be moved
>> to caller of generic_setup_phy.
> 
>