diff mbox series

[7/7] usb: host: ohci-exynos: Convert to devm_of_phy_optional_get()

Message ID cd685d8e4d6754c384acfc1796065d539a2c3ea8.1674036164.git.geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show
Series phy: Add devm_of_phy_optional_get() helper | expand

Commit Message

Geert Uytterhoeven Jan. 18, 2023, 10:15 a.m. UTC
Use the new devm_of_phy_optional_get() helper instead of open-coding the
same operation.

This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
handle NULL parameters fine.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

Comments

Alan Stern Jan. 18, 2023, 4:18 p.m. UTC | #1
On Wed, Jan 18, 2023 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
> 
> This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> handle NULL parameters fine.

The patch ignores a possible -ENOSYS error return.  Is it known that 
this will never happen?

Alan Stern

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
>  			return -EINVAL;
>  		}
>  
> -		phy = devm_of_phy_get(dev, child, NULL);
> +		phy = devm_of_phy_optional_get(dev, child, NULL);
>  		exynos_ohci->phy[phy_number] = phy;
>  		if (IS_ERR(phy)) {
> -			ret = PTR_ERR(phy);
> -			if (ret == -EPROBE_DEFER) {
> -				of_node_put(child);
> -				return ret;
> -			} else if (ret != -ENOSYS && ret != -ENODEV) {
> -				dev_err(dev,
> -					"Error retrieving usb2 phy: %d\n", ret);
> -				of_node_put(child);
> -				return ret;
> -			}
> +			of_node_put(child);
> +			return dev_err_probe(dev, PTR_ERR(phy),
> +					     "Error retrieving usb2 phy\n");
>  		}
>  	}
>  
> @@ -97,12 +90,10 @@ static int exynos_ohci_phy_enable(struct device *dev)
>  	int ret = 0;
>  
>  	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ohci->phy[i]))
> -			ret = phy_power_on(exynos_ohci->phy[i]);
> +		ret = phy_power_on(exynos_ohci->phy[i]);
>  	if (ret)
>  		for (i--; i >= 0; i--)
> -			if (!IS_ERR(exynos_ohci->phy[i]))
> -				phy_power_off(exynos_ohci->phy[i]);
> +			phy_power_off(exynos_ohci->phy[i]);
>  
>  	return ret;
>  }
> @@ -114,8 +105,7 @@ static void exynos_ohci_phy_disable(struct device *dev)
>  	int i;
>  
>  	for (i = 0; i < PHY_NUMBER; i++)
> -		if (!IS_ERR(exynos_ohci->phy[i]))
> -			phy_power_off(exynos_ohci->phy[i]);
> +		phy_power_off(exynos_ohci->phy[i]);
>  }
>  
>  static int exynos_ohci_probe(struct platform_device *pdev)
> -- 
> 2.34.1
>
Geert Uytterhoeven Jan. 18, 2023, 4:50 p.m. UTC | #2
Hi Alan,

On Wed, Jan 18, 2023 at 5:26 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, Jan 18, 2023 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > same operation.
> >
> > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > handle NULL parameters fine.
>
> The patch ignores a possible -ENOSYS error return.  Is it known that
> this will never happen?

While some phy_*() dummies in include/linux/phy/phy.h do return -ENOSYS
if CONFIG_GENERIC_PHY is not enabled, devm_of_phy_optional_get()
returns NULL in that case, so I think this cannot happen.

> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
> >                       return -EINVAL;
> >               }
> >
> > -             phy = devm_of_phy_get(dev, child, NULL);
> > +             phy = devm_of_phy_optional_get(dev, child, NULL);
> >               exynos_ohci->phy[phy_number] = phy;
> >               if (IS_ERR(phy)) {
> > -                     ret = PTR_ERR(phy);
> > -                     if (ret == -EPROBE_DEFER) {
> > -                             of_node_put(child);
> > -                             return ret;
> > -                     } else if (ret != -ENOSYS && ret != -ENODEV) {
> > -                             dev_err(dev,
> > -                                     "Error retrieving usb2 phy: %d\n", ret);
> > -                             of_node_put(child);
> > -                             return ret;
> > -                     }
> > +                     of_node_put(child);
> > +                     return dev_err_probe(dev, PTR_ERR(phy),
> > +                                          "Error retrieving usb2 phy\n");
> >               }
> >       }
> >

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Alan Stern Jan. 18, 2023, 5:01 p.m. UTC | #3
On Wed, Jan 18, 2023 at 05:50:00PM +0100, Geert Uytterhoeven wrote:
> Hi Alan,
> 
> On Wed, Jan 18, 2023 at 5:26 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Wed, Jan 18, 2023 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> > > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > > same operation.
> > >
> > > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > > handle NULL parameters fine.
> >
> > The patch ignores a possible -ENOSYS error return.  Is it known that
> > this will never happen?
> 
> While some phy_*() dummies in include/linux/phy/phy.h do return -ENOSYS
> if CONFIG_GENERIC_PHY is not enabled, devm_of_phy_optional_get()
> returns NULL in that case, so I think this cannot happen.

You could mention that in the patch description.  Apart from this one 
issue:

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern
Rob Herring (Arm) Jan. 18, 2023, 5:29 p.m. UTC | #4
On Wed, Jan 18, 2023 at 4:15 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> Use the new devm_of_phy_optional_get() helper instead of open-coding the
> same operation.
>
> This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> handle NULL parameters fine.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
>                         return -EINVAL;
>                 }
>
> -               phy = devm_of_phy_get(dev, child, NULL);
> +               phy = devm_of_phy_optional_get(dev, child, NULL);
>                 exynos_ohci->phy[phy_number] = phy;
>                 if (IS_ERR(phy)) {
> -                       ret = PTR_ERR(phy);
> -                       if (ret == -EPROBE_DEFER) {
> -                               of_node_put(child);
> -                               return ret;
> -                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> -                               dev_err(dev,
> -                                       "Error retrieving usb2 phy: %d\n", ret);
> -                               of_node_put(child);
> -                               return ret;
> -                       }
> +                       of_node_put(child);
> +                       return dev_err_probe(dev, PTR_ERR(phy),
> +                                            "Error retrieving usb2 phy\n");

Optional is really the only reason for the caller to decide whether to
print an error message or not. If we have both flavors of 'get', then
really the 'get' functions should print an error message.

Rob
Geert Uytterhoeven Jan. 18, 2023, 6:28 p.m. UTC | #5
Hi Rob,

On Wed, Jan 18, 2023 at 6:30 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 18, 2023 at 4:15 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > same operation.
> >
> > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > handle NULL parameters fine.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
> >  1 file changed, 7 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
> >                         return -EINVAL;
> >                 }
> >
> > -               phy = devm_of_phy_get(dev, child, NULL);
> > +               phy = devm_of_phy_optional_get(dev, child, NULL);
> >                 exynos_ohci->phy[phy_number] = phy;
> >                 if (IS_ERR(phy)) {
> > -                       ret = PTR_ERR(phy);
> > -                       if (ret == -EPROBE_DEFER) {
> > -                               of_node_put(child);
> > -                               return ret;
> > -                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> > -                               dev_err(dev,
> > -                                       "Error retrieving usb2 phy: %d\n", ret);
> > -                               of_node_put(child);
> > -                               return ret;
> > -                       }
> > +                       of_node_put(child);
> > +                       return dev_err_probe(dev, PTR_ERR(phy),
> > +                                            "Error retrieving usb2 phy\n");
>
> Optional is really the only reason for the caller to decide whether to
> print an error message or not. If we have both flavors of 'get', then
> really the 'get' functions should print an error message.

In case of a real error, both should print an error message, right?

Anyway, I understand that's a three step operation:
  1. Introduce and convert to the _optional variant,
  2. Add error printing to callees.
  3. Remove error printing from callers.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Rob Herring (Arm) Jan. 18, 2023, 7:49 p.m. UTC | #6
On Wed, Jan 18, 2023 at 12:28 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Wed, Jan 18, 2023 at 6:30 PM Rob Herring <robh@kernel.org> wrote:
> > On Wed, Jan 18, 2023 at 4:15 AM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > > same operation.
> > >
> > > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > > handle NULL parameters fine.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
> > >  1 file changed, 7 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > > index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> > > --- a/drivers/usb/host/ohci-exynos.c
> > > +++ b/drivers/usb/host/ohci-exynos.c
> > > @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
> > >                         return -EINVAL;
> > >                 }
> > >
> > > -               phy = devm_of_phy_get(dev, child, NULL);
> > > +               phy = devm_of_phy_optional_get(dev, child, NULL);
> > >                 exynos_ohci->phy[phy_number] = phy;
> > >                 if (IS_ERR(phy)) {
> > > -                       ret = PTR_ERR(phy);
> > > -                       if (ret == -EPROBE_DEFER) {
> > > -                               of_node_put(child);
> > > -                               return ret;
> > > -                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> > > -                               dev_err(dev,
> > > -                                       "Error retrieving usb2 phy: %d\n", ret);
> > > -                               of_node_put(child);
> > > -                               return ret;
> > > -                       }
> > > +                       of_node_put(child);
> > > +                       return dev_err_probe(dev, PTR_ERR(phy),
> > > +                                            "Error retrieving usb2 phy\n");
> >
> > Optional is really the only reason for the caller to decide whether to
> > print an error message or not. If we have both flavors of 'get', then
> > really the 'get' functions should print an error message.
>
> In case of a real error, both should print an error message, right?
>
> Anyway, I understand that's a three step operation:
>   1. Introduce and convert to the _optional variant,
>   2. Add error printing to callees.
>   3. Remove error printing from callers.

I think you only need 2 out of 3 steps depending on the situation. In
this case, you can add error printing in the _optional variant when
you introduce it and then convert callers to it.

Where we already have an optional variant, then you need steps 2 and 3.

Rob
Geert Uytterhoeven Jan. 20, 2023, 7:56 a.m. UTC | #7
Hi Rob,

On Wed, Jan 18, 2023 at 8:49 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 18, 2023 at 12:28 PM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Wed, Jan 18, 2023 at 6:30 PM Rob Herring <robh@kernel.org> wrote:
> > > On Wed, Jan 18, 2023 at 4:15 AM Geert Uytterhoeven
> > > <geert+renesas@glider.be> wrote:
> > > > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > > > same operation.
> > > >
> > > > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > > > handle NULL parameters fine.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > ---
> > > >  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
> > > >  1 file changed, 7 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > > > index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> > > > --- a/drivers/usb/host/ohci-exynos.c
> > > > +++ b/drivers/usb/host/ohci-exynos.c
> > > > @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
> > > >                         return -EINVAL;
> > > >                 }
> > > >
> > > > -               phy = devm_of_phy_get(dev, child, NULL);
> > > > +               phy = devm_of_phy_optional_get(dev, child, NULL);
> > > >                 exynos_ohci->phy[phy_number] = phy;
> > > >                 if (IS_ERR(phy)) {
> > > > -                       ret = PTR_ERR(phy);
> > > > -                       if (ret == -EPROBE_DEFER) {
> > > > -                               of_node_put(child);
> > > > -                               return ret;
> > > > -                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> > > > -                               dev_err(dev,
> > > > -                                       "Error retrieving usb2 phy: %d\n", ret);
> > > > -                               of_node_put(child);
> > > > -                               return ret;
> > > > -                       }
> > > > +                       of_node_put(child);
> > > > +                       return dev_err_probe(dev, PTR_ERR(phy),
> > > > +                                            "Error retrieving usb2 phy\n");
> > >
> > > Optional is really the only reason for the caller to decide whether to
> > > print an error message or not. If we have both flavors of 'get', then
> > > really the 'get' functions should print an error message.
> >
> > In case of a real error, both should print an error message, right?
> >
> > Anyway, I understand that's a three step operation:
> >   1. Introduce and convert to the _optional variant,
> >   2. Add error printing to callees.
> >   3. Remove error printing from callers.
>
> I think you only need 2 out of 3 steps depending on the situation. In
> this case, you can add error printing in the _optional variant when
> you introduce it and then convert callers to it.
>
> Where we already have an optional variant, then you need steps 2 and 3.

Right, so the error printing can be done now, while introducing
devm_of_phy_optional_get().

Vinod: Do you agree?
If yes, I can respin with that change.
If not, I'll have to respin anyway, as the bug in
am65_cpsw_init_serdes_phy() has been fixed in the meantime.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Vinod Koul Jan. 20, 2023, 8:04 a.m. UTC | #8
On 20-01-23, 08:56, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Wed, Jan 18, 2023 at 8:49 PM Rob Herring <robh@kernel.org> wrote:
> > On Wed, Jan 18, 2023 at 12:28 PM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Wed, Jan 18, 2023 at 6:30 PM Rob Herring <robh@kernel.org> wrote:
> > > > On Wed, Jan 18, 2023 at 4:15 AM Geert Uytterhoeven
> > > > <geert+renesas@glider.be> wrote:
> > > > > Use the new devm_of_phy_optional_get() helper instead of open-coding the
> > > > > same operation.
> > > > >
> > > > > This lets us drop several checks for IS_ERR(), as phy_power_{on,off}()
> > > > > handle NULL parameters fine.
> > > > >
> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > ---
> > > > >  drivers/usb/host/ohci-exynos.c | 24 +++++++-----------------
> > > > >  1 file changed, 7 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > > > > index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
> > > > > --- a/drivers/usb/host/ohci-exynos.c
> > > > > +++ b/drivers/usb/host/ohci-exynos.c
> > > > > @@ -69,19 +69,12 @@ static int exynos_ohci_get_phy(struct device *dev,
> > > > >                         return -EINVAL;
> > > > >                 }
> > > > >
> > > > > -               phy = devm_of_phy_get(dev, child, NULL);
> > > > > +               phy = devm_of_phy_optional_get(dev, child, NULL);
> > > > >                 exynos_ohci->phy[phy_number] = phy;
> > > > >                 if (IS_ERR(phy)) {
> > > > > -                       ret = PTR_ERR(phy);
> > > > > -                       if (ret == -EPROBE_DEFER) {
> > > > > -                               of_node_put(child);
> > > > > -                               return ret;
> > > > > -                       } else if (ret != -ENOSYS && ret != -ENODEV) {
> > > > > -                               dev_err(dev,
> > > > > -                                       "Error retrieving usb2 phy: %d\n", ret);
> > > > > -                               of_node_put(child);
> > > > > -                               return ret;
> > > > > -                       }
> > > > > +                       of_node_put(child);
> > > > > +                       return dev_err_probe(dev, PTR_ERR(phy),
> > > > > +                                            "Error retrieving usb2 phy\n");
> > > >
> > > > Optional is really the only reason for the caller to decide whether to
> > > > print an error message or not. If we have both flavors of 'get', then
> > > > really the 'get' functions should print an error message.
> > >
> > > In case of a real error, both should print an error message, right?
> > >
> > > Anyway, I understand that's a three step operation:
> > >   1. Introduce and convert to the _optional variant,
> > >   2. Add error printing to callees.
> > >   3. Remove error printing from callers.
> >
> > I think you only need 2 out of 3 steps depending on the situation. In
> > this case, you can add error printing in the _optional variant when
> > you introduce it and then convert callers to it.
> >
> > Where we already have an optional variant, then you need steps 2 and 3.
> 
> Right, so the error printing can be done now, while introducing
> devm_of_phy_optional_get().
> 
> Vinod: Do you agree?

Ack.. IMO makes it better that way

> If yes, I can respin with that change.

ok

> If not, I'll have to respin anyway, as the bug in
> am65_cpsw_init_serdes_phy() has been fixed in the meantime.
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
diff mbox series

Patch

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 8d7977fd5d3bd502..8dd9c3b2411c383f 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -69,19 +69,12 @@  static int exynos_ohci_get_phy(struct device *dev,
 			return -EINVAL;
 		}
 
-		phy = devm_of_phy_get(dev, child, NULL);
+		phy = devm_of_phy_optional_get(dev, child, NULL);
 		exynos_ohci->phy[phy_number] = phy;
 		if (IS_ERR(phy)) {
-			ret = PTR_ERR(phy);
-			if (ret == -EPROBE_DEFER) {
-				of_node_put(child);
-				return ret;
-			} else if (ret != -ENOSYS && ret != -ENODEV) {
-				dev_err(dev,
-					"Error retrieving usb2 phy: %d\n", ret);
-				of_node_put(child);
-				return ret;
-			}
+			of_node_put(child);
+			return dev_err_probe(dev, PTR_ERR(phy),
+					     "Error retrieving usb2 phy\n");
 		}
 	}
 
@@ -97,12 +90,10 @@  static int exynos_ohci_phy_enable(struct device *dev)
 	int ret = 0;
 
 	for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy[i]))
-			ret = phy_power_on(exynos_ohci->phy[i]);
+		ret = phy_power_on(exynos_ohci->phy[i]);
 	if (ret)
 		for (i--; i >= 0; i--)
-			if (!IS_ERR(exynos_ohci->phy[i]))
-				phy_power_off(exynos_ohci->phy[i]);
+			phy_power_off(exynos_ohci->phy[i]);
 
 	return ret;
 }
@@ -114,8 +105,7 @@  static void exynos_ohci_phy_disable(struct device *dev)
 	int i;
 
 	for (i = 0; i < PHY_NUMBER; i++)
-		if (!IS_ERR(exynos_ohci->phy[i]))
-			phy_power_off(exynos_ohci->phy[i]);
+		phy_power_off(exynos_ohci->phy[i]);
 }
 
 static int exynos_ohci_probe(struct platform_device *pdev)