[PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[Kushagra Verma]

This patch removes the use of -ENOSYS as it is used when users try to call a
syscall that doesn't exist. So, we don't need to check if 'ret == -ENOSYS'.

Signed-off-by: Kushagra Verma <kushagra765@outlook.com>
---
 drivers/usb/dwc3/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c78205c5e19f..3c1a877d5183 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1283,7 +1283,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
 	if (IS_ERR(dwc->usb2_generic_phy)) {
 		ret = PTR_ERR(dwc->usb2_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
+		if (ret == -ENODEV)
 			dwc->usb2_generic_phy = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
@@ -1292,7 +1292,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
 	if (IS_ERR(dwc->usb3_generic_phy)) {
 		ret = PTR_ERR(dwc->usb3_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
+		if (ret == -ENODEV)
 			dwc->usb3_generic_phy = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
-- 
2.36.1

Re: [PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[Greg KH]

On Tue, May 17, 2022 at 08:22:00PM +0530, Kushagra Verma wrote:
> This patch removes the use of -ENOSYS as it is used when users try to call a
> syscall that doesn't exist. So, we don't need to check if 'ret == -ENOSYS'.

ENOSYS is also used internally in the kernel for other things.

> 
> Signed-off-by: Kushagra Verma <kushagra765@outlook.com>
> ---
>  drivers/usb/dwc3/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c78205c5e19f..3c1a877d5183 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1283,7 +1283,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>  	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>  	if (IS_ERR(dwc->usb2_generic_phy)) {
>  		ret = PTR_ERR(dwc->usb2_generic_phy);
> -		if (ret == -ENOSYS || ret == -ENODEV)
> +		if (ret == -ENODEV)

Did you validate that no callers can ever set this to ENOSYS?

Why was this added in the first place?  What commit added it?

thanks,

greg k-h

Re: [PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[Kushagra Verma]

On Thu, 2022-05-19 at 17:59 +0200, Greg KH wrote:
> On Tue, May 17, 2022 at 08:22:00PM +0530, Kushagra Verma wrote:
> > This patch removes the use of -ENOSYS as it is used when users try
> > to call a
> > syscall that doesn't exist. So, we don't need to check if 'ret == -
> > ENOSYS'.
> 
> ENOSYS is also used internally in the kernel for other things.
> 
> > 
> > Signed-off-by: Kushagra Verma <kushagra765@outlook.com>
> > ---
> >  drivers/usb/dwc3/core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index c78205c5e19f..3c1a877d5183 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1283,7 +1283,7 @@ static int dwc3_core_get_phy(struct dwc3
> > *dwc)
> >         dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> >         if (IS_ERR(dwc->usb2_generic_phy)) {
> >                 ret = PTR_ERR(dwc->usb2_generic_phy);
> > -               if (ret == -ENOSYS || ret == -ENODEV)
> > +               if (ret == -ENODEV)
> 
> Did you validate that no callers can ever set this to ENOSYS?
Yes, I think no callers should ever set ret to ENOSYS unless it comes under the other things ENOSYS is used for.
> 
> Why was this added in the first place?  What commit added it?
This was added by commit 57303488cd37d ("usb: dwc3: adapt dwc3 core to use Generic PHY Framework").
> 
> thanks,
> 
> greg k-h

Thanks,

Kushagra

Re: [PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[Greg KH]

On Fri, May 20, 2022 at 12:16:29PM +0530, Kushagra Verma wrote:
> On Thu, 2022-05-19 at 17:59 +0200, Greg KH wrote:
> > On Tue, May 17, 2022 at 08:22:00PM +0530, Kushagra Verma wrote:
> > > This patch removes the use of -ENOSYS as it is used when users try
> > > to call a
> > > syscall that doesn't exist. So, we don't need to check if 'ret == -
> > > ENOSYS'.
> > 
> > ENOSYS is also used internally in the kernel for other things.
> > 
> > > 
> > > Signed-off-by: Kushagra Verma <kushagra765@outlook.com>
> > > ---
> > >  drivers/usb/dwc3/core.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index c78205c5e19f..3c1a877d5183 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -1283,7 +1283,7 @@ static int dwc3_core_get_phy(struct dwc3
> > > *dwc)
> > >         dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> > >         if (IS_ERR(dwc->usb2_generic_phy)) {
> > >                 ret = PTR_ERR(dwc->usb2_generic_phy);
> > > -               if (ret == -ENOSYS || ret == -ENODEV)
> > > +               if (ret == -ENODEV)
> > 
> > Did you validate that no callers can ever set this to ENOSYS?
> Yes, I think no callers should ever set ret to ENOSYS unless it comes under the other things ENOSYS is used for.

Please audit the code to verify this and document it in the changelog
text when you resend this.

> > Why was this added in the first place?  What commit added it?
> This was added by commit 57303488cd37d ("usb: dwc3: adapt dwc3 core to use Generic PHY Framework").

Great, please also include that information.

thanks,

greg k-h

Re: [PATCH] USB / dwc3: remove the use of -ENOSYS from core.c

[Kushagra Verma]

On Fri, 2022-05-20 at 10:24 +0200, Greg KH wrote:
> On Fri, May 20, 2022 at 12:16:29PM +0530, Kushagra Verma wrote:
> > On Thu, 2022-05-19 at 17:59 +0200, Greg KH wrote:
> > > On Tue, May 17, 2022 at 08:22:00PM +0530, Kushagra Verma wrote:
> > > > This patch removes the use of -ENOSYS as it is used when users try
> > > > to call a
> > > > syscall that doesn't exist. So, we don't need to check if 'ret == -
> > > > ENOSYS'.
> > > 
> > > ENOSYS is also used internally in the kernel for other things.
> > > 
> > > > 
> > > > Signed-off-by: Kushagra Verma <kushagra765@outlook.com>
> > > > ---
> > > >  drivers/usb/dwc3/core.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index c78205c5e19f..3c1a877d5183 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -1283,7 +1283,7 @@ static int dwc3_core_get_phy(struct dwc3
> > > > *dwc)
> > > >         dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> > > >         if (IS_ERR(dwc->usb2_generic_phy)) {
> > > >                 ret = PTR_ERR(dwc->usb2_generic_phy);
> > > > -               if (ret == -ENOSYS || ret == -ENODEV)
> > > > +               if (ret == -ENODEV)
> > > 
> > > Did you validate that no callers can ever set this to ENOSYS?
> > Yes, I think no callers should ever set ret to ENOSYS unless it comes under the other things ENOSYS is used for.
> 
> Please audit the code to verify this and document it in the changelog
> text when you resend this.
I have audited the code and none of the callers return -ENOSYS. The devm_phy_get function calls phy_get to
lookup and obtain a reference to a phy. This function returns the phy driver or -ENODEV if there is no such
phy (according to the comments above it).
> 
> > > Why was this added in the first place?  What commit added it?
> > This was added by commit 57303488cd37d ("usb: dwc3: adapt dwc3 core to use Generic PHY Framework").
> 
> Great, please also include that information.
Ok.
> 
> thanks,
> 
> greg k-h

Thanks,

Kushagra