Re: [PATCH] tty: serial: samsung_tty: remove set but not used variables

[Greg KH <gregkh@linuxfoundation.org>]

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?  :)