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@xxxxxxxxxxxxx> >>>> --- >>>> 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
