* [PATCH] tty: serial: samsung_tty: remove set but not used variables @ 2021-04-23 9:39 Tian Tao 2021-04-23 9:47 ` Greg KH 2021-04-23 9:55 ` Jiri Slaby 0 siblings, 2 replies; 8+ messages in thread From: Tian Tao @ 2021-04-23 9:39 UTC (permalink / raw) To: gregkh, jirislaby; +Cc: linux-samsung-soc, linux-serial, Tian Tao The value of 'ret' is not used, so just delete it. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/tty/serial/samsung_tty.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index d9e4b67..d269d75 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) default: dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", prop); - ret = -EINVAL; break; } } -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-23 9:39 [PATCH] tty: serial: samsung_tty: remove set but not used variables Tian Tao @ 2021-04-23 9:47 ` Greg KH 2021-04-23 9:54 ` tiantao (H) 2021-04-23 9:55 ` Jiri Slaby 1 sibling, 1 reply; 8+ messages in thread From: Greg KH @ 2021-04-23 9:47 UTC (permalink / raw) To: Tian Tao; +Cc: jirislaby, linux-samsung-soc, linux-serial On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: > The value of 'ret' is not used, so just delete it. > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/tty/serial/samsung_tty.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index d9e4b67..d269d75 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > default: > dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", > prop); > - ret = -EINVAL; That looks odd, shouldn't you do something with this instead of ignoring it??? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-23 9:47 ` Greg KH @ 2021-04-23 9:54 ` tiantao (H) 2021-04-23 10:14 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: tiantao (H) @ 2021-04-23 9:54 UTC (permalink / raw) To: Greg KH, Tian Tao; +Cc: jirislaby, linux-samsung-soc, linux-serial 在 2021/4/23 17:47, Greg KH 写道: > On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: >> The value of 'ret' is not used, so just delete it. >> >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >> --- >> drivers/tty/serial/samsung_tty.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c >> index d9e4b67..d269d75 100644 >> --- a/drivers/tty/serial/samsung_tty.c >> +++ b/drivers/tty/serial/samsung_tty.c >> @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) >> default: >> dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", >> prop); >> - ret = -EINVAL; > That looks odd, shouldn't you do something with this instead of ignoring > it??? How about this ？ diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index d9e4b67..9fbc611 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2220,8 +2220,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) default: dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", prop); - ret = -EINVAL; - break; + return -EINVAL; > . > ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-23 9:54 ` tiantao (H) @ 2021-04-23 10:14 ` Greg KH 2021-04-26 6:45 ` Krzysztof Kozlowski 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2021-04-23 10:14 UTC (permalink / raw) To: tiantao (H); +Cc: Tian Tao, jirislaby, linux-samsung-soc, linux-serial On Fri, Apr 23, 2021 at 05:54:16PM +0800, tiantao (H) wrote: > > 在 2021/4/23 17:47, Greg KH 写道: > > On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: > > > The value of 'ret' is not used, so just delete it. > > > > > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > > > --- > > > drivers/tty/serial/samsung_tty.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > > > index d9e4b67..d269d75 100644 > > > --- a/drivers/tty/serial/samsung_tty.c > > > +++ b/drivers/tty/serial/samsung_tty.c > > > @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > > > default: > > > dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", > > > prop); > > > - ret = -EINVAL; > > That looks odd, shouldn't you do something with this instead of ignoring > > it??? > > How about this ？ > > diff --git a/drivers/tty/serial/samsung_tty.c > b/drivers/tty/serial/samsung_tty.c > index d9e4b67..9fbc611 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2220,8 +2220,7 @@ static int s3c24xx_serial_probe(struct platform_device > *pdev) > default: > dev_warn(&pdev->dev, "unsupported > reg-io-width (%d)\n", > prop); > - ret = -EINVAL; > - break; > + return -EINVAL; > You tell me, does the patch work for you? Is this really a "hard error" and did you now just break devices that used to work properly? Are you correctly unwinding any previously allocated state when you return here? Please do some research on this, and ideally, lots of testing, before submitting it as a real solution. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-23 10:14 ` Greg KH @ 2021-04-26 6:45 ` Krzysztof Kozlowski 2021-04-26 6:56 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2021-04-26 6:45 UTC (permalink / raw) To: Greg KH, tiantao (H); +Cc: Tian Tao, jirislaby, linux-samsung-soc, linux-serial On 23/04/2021 12:14, Greg KH wrote: > On Fri, Apr 23, 2021 at 05:54:16PM +0800, tiantao (H) wrote: >> >> 在 2021/4/23 17:47, Greg KH 写道: >>> On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: >>>> The value of 'ret' is not used, so just delete it. Tian Tao, please use scripts/get_maintainers.pl to get the list of people needed for Cc. >>>> >>>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >>>> --- >>>> drivers/tty/serial/samsung_tty.c | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c >>>> index d9e4b67..d269d75 100644 >>>> --- a/drivers/tty/serial/samsung_tty.c >>>> +++ b/drivers/tty/serial/samsung_tty.c >>>> @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) >>>> default: >>>> dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", >>>> prop); >>>> - ret = -EINVAL; >>> That looks odd, shouldn't you do something with this instead of ignoring >>> it??? >> >> How about this ？ >> >> diff --git a/drivers/tty/serial/samsung_tty.c >> b/drivers/tty/serial/samsung_tty.c >> index d9e4b67..9fbc611 100644 >> --- a/drivers/tty/serial/samsung_tty.c >> +++ b/drivers/tty/serial/samsung_tty.c >> @@ -2220,8 +2220,7 @@ static int s3c24xx_serial_probe(struct platform_device >> *pdev) >> default: >> dev_warn(&pdev->dev, "unsupported >> reg-io-width (%d)\n", >> prop); >> - ret = -EINVAL; >> - break; >> + return -EINVAL; >> > > You tell me, does the patch work for you? > > Is this really a "hard error" and did you now just break devices that > used to work properly? Are you correctly unwinding any previously > allocated state when you return here? > > Please do some research on this, and ideally, lots of testing, before > submitting it as a real solution. It's a patch coming from automated tool (e.g. Coverity), so I doubt there is any testing here. However the "return -EINVAL" looks correct here: 1. No particular unwinding is needed here, 2. It's an optional property (not used by existing DTS, only non-upstreamed by Samsung) thus treating it as hard-error is fine. Probably better to exit than convert it to some default value. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-26 6:45 ` Krzysztof Kozlowski @ 2021-04-26 6:56 ` Greg KH 2021-04-26 6:57 ` Krzysztof Kozlowski 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2021-04-26 6:56 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: tiantao (H), Tian Tao, jirislaby, linux-samsung-soc, linux-serial On Mon, Apr 26, 2021 at 08:45:44AM +0200, Krzysztof Kozlowski wrote: > On 23/04/2021 12:14, Greg KH wrote: > > On Fri, Apr 23, 2021 at 05:54:16PM +0800, tiantao (H) wrote: > >> > >> 在 2021/4/23 17:47, Greg KH 写道: > >>> On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: > >>>> The value of 'ret' is not used, so just delete it. > > Tian Tao, please use scripts/get_maintainers.pl to get the list of > people needed for Cc. > > >>>> > >>>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > >>>> --- > >>>> drivers/tty/serial/samsung_tty.c | 1 - > >>>> 1 file changed, 1 deletion(-) > >>>> > >>>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > >>>> index d9e4b67..d269d75 100644 > >>>> --- a/drivers/tty/serial/samsung_tty.c > >>>> +++ b/drivers/tty/serial/samsung_tty.c > >>>> @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > >>>> default: > >>>> dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", > >>>> prop); > >>>> - ret = -EINVAL; > >>> That looks odd, shouldn't you do something with this instead of ignoring > >>> it??? > >> > >> How about this ？ > >> > >> diff --git a/drivers/tty/serial/samsung_tty.c > >> b/drivers/tty/serial/samsung_tty.c > >> index d9e4b67..9fbc611 100644 > >> --- a/drivers/tty/serial/samsung_tty.c > >> +++ b/drivers/tty/serial/samsung_tty.c > >> @@ -2220,8 +2220,7 @@ static int s3c24xx_serial_probe(struct platform_device > >> *pdev) > >> default: > >> dev_warn(&pdev->dev, "unsupported > >> reg-io-width (%d)\n", > >> prop); > >> - ret = -EINVAL; > >> - break; > >> + return -EINVAL; > >> > > > > You tell me, does the patch work for you? > > > > Is this really a "hard error" and did you now just break devices that > > used to work properly? Are you correctly unwinding any previously > > allocated state when you return here? > > > > Please do some research on this, and ideally, lots of testing, before > > submitting it as a real solution. > > It's a patch coming from automated tool (e.g. Coverity), so I doubt > there is any testing here. However the "return -EINVAL" looks correct here: > 1. No particular unwinding is needed here, > 2. It's an optional property (not used by existing DTS, only > non-upstreamed by Samsung) thus treating it as hard-error is fine. > Probably better to exit than convert it to some default value. So is that a "Reviwed-by:" or not? :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-26 6:56 ` Greg KH @ 2021-04-26 6:57 ` Krzysztof Kozlowski 0 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2021-04-26 6:57 UTC (permalink / raw) To: Greg KH; +Cc: tiantao (H), Tian Tao, jirislaby, linux-samsung-soc, linux-serial On 26/04/2021 08:56, Greg KH wrote: > On Mon, Apr 26, 2021 at 08:45:44AM +0200, Krzysztof Kozlowski wrote: >> On 23/04/2021 12:14, Greg KH wrote: >>> On Fri, Apr 23, 2021 at 05:54:16PM +0800, tiantao (H) wrote: >>>> >>>> 在 2021/4/23 17:47, Greg KH 写道: >>>>> On Fri, Apr 23, 2021 at 05:39:00PM +0800, Tian Tao wrote: >>>>>> The value of 'ret' is not used, so just delete it. >> >> Tian Tao, please use scripts/get_maintainers.pl to get the list of >> people needed for Cc. >> >>>>>> >>>>>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >>>>>> --- >>>>>> drivers/tty/serial/samsung_tty.c | 1 - >>>>>> 1 file changed, 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c >>>>>> index d9e4b67..d269d75 100644 >>>>>> --- a/drivers/tty/serial/samsung_tty.c >>>>>> +++ b/drivers/tty/serial/samsung_tty.c >>>>>> @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) >>>>>> default: >>>>>> dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", >>>>>> prop); >>>>>> - ret = -EINVAL; >>>>> That looks odd, shouldn't you do something with this instead of ignoring >>>>> it??? >>>> >>>> How about this ？ >>>> >>>> diff --git a/drivers/tty/serial/samsung_tty.c >>>> b/drivers/tty/serial/samsung_tty.c >>>> index d9e4b67..9fbc611 100644 >>>> --- a/drivers/tty/serial/samsung_tty.c >>>> +++ b/drivers/tty/serial/samsung_tty.c >>>> @@ -2220,8 +2220,7 @@ static int s3c24xx_serial_probe(struct platform_device >>>> *pdev) >>>> default: >>>> dev_warn(&pdev->dev, "unsupported >>>> reg-io-width (%d)\n", >>>> prop); >>>> - ret = -EINVAL; >>>> - break; >>>> + return -EINVAL; >>>> >>> >>> You tell me, does the patch work for you? >>> >>> Is this really a "hard error" and did you now just break devices that >>> used to work properly? Are you correctly unwinding any previously >>> allocated state when you return here? >>> >>> Please do some research on this, and ideally, lots of testing, before >>> submitting it as a real solution. >> >> It's a patch coming from automated tool (e.g. Coverity), so I doubt >> there is any testing here. However the "return -EINVAL" looks correct here: >> 1. No particular unwinding is needed here, >> 2. It's an optional property (not used by existing DTS, only >> non-upstreamed by Samsung) thus treating it as hard-error is fine. >> Probably better to exit than convert it to some default value. > > So is that a "Reviwed-by:" or not? :) First, Tian Tao needs to send a v2 of that patch with a "return" instead of break. Then this will be a reviewed-by. :) Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables 2021-04-23 9:39 [PATCH] tty: serial: samsung_tty: remove set but not used variables Tian Tao 2021-04-23 9:47 ` Greg KH @ 2021-04-23 9:55 ` Jiri Slaby 1 sibling, 0 replies; 8+ messages in thread From: Jiri Slaby @ 2021-04-23 9:55 UTC (permalink / raw) To: Tian Tao, gregkh; +Cc: linux-samsung-soc, linux-serial, Hyunki Koo On 23. 04. 21, 11:39, Tian Tao wrote: > The value of 'ret' is not used, so just delete it. > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/tty/serial/samsung_tty.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index d9e4b67..d269d75 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2220,7 +2220,6 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) > default: > dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", > prop); > - ret = -EINVAL; > break; Added by: commit 57253ccd5831e7e5720c433437775c3e6b7d0c72 Author: Hyunki Koo <hyunki00.koo@samsung.com> Date: Wed May 6 17:02:40 2020 +0900 serial: samsung: 32-bit access for TX/RX hold registers Hyunki, what was the intention with "ret" here? > } > } > -- js suse labs ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-26 6:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-23 9:39 [PATCH] tty: serial: samsung_tty: remove set but not used variables Tian Tao 2021-04-23 9:47 ` Greg KH 2021-04-23 9:54 ` tiantao (H) 2021-04-23 10:14 ` Greg KH 2021-04-26 6:45 ` Krzysztof Kozlowski 2021-04-26 6:56 ` Greg KH 2021-04-26 6:57 ` Krzysztof Kozlowski 2021-04-23 9:55 ` Jiri Slaby
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).