* [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: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
* 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
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).