From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751372AbdJWIpw (ORCPT ); Mon, 23 Oct 2017 04:45:52 -0400 Received: from mout.web.de ([212.227.15.4]:63842 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751152AbdJWIpu (ORCPT ); Mon, 23 Oct 2017 04:45:50 -0400 Subject: Re: [PATCH] gpu/drm/bridge/sii9234: Use common error handling code in sii9234_writebm() To: Dan Carpenter , dri-devel@lists.freedesktop.org Cc: Andrzej Hajda , Archit Taneja , David Airlie , Laurent Pinchart , Maciej Purski , Rob Herring , LKML , kernel-janitors@vger.kernel.org References: <8bb4a1d2-8876-731f-2938-8be075f4252c@users.sourceforge.net> <20171023071859.m55dgxzooje4i35e@mwanda> From: SF Markus Elfring Message-ID: <54f4eff0-e7bb-214b-980e-208587850037@users.sourceforge.net> Date: Mon, 23 Oct 2017 10:45:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171023071859.m55dgxzooje4i35e@mwanda> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:QZ/NGX+Krj8SCqavihxDNBNwlUBVyY4FNOReuDipoXDSI6CcWHG LwkVmzINtwuuZlgyWcCUcq0JHcVv0N6ZSzCJ9mN3kMn6lhCWN7Dz2y9E7WEDcBeQp+GKEzi Ae16BPejZcgc6XkBxGin9QcFH7etIcWob2A1M1vHoERj9n/sVlwLXgkRBFRWQWA7hHwrN7G Q+mZjVkcKkN5bF2OOZ77w== X-UI-Out-Filterresults: notjunk:1;V01:K0:oV+1tw8hLUg=:/bBtXzAiemS/DPZeGlLmKs xQrLH75KZKJRrFa64ri31K9flwIZ6zUvDb5UZTzxhG53DJAITe/i52Sx/k9MFJ4Yb2yoO2RTK Wd3DQSa8QD3997I9w+Hjl0RC5er+xTQca8Vj0SD7Kn2sn1eZDdvWw90rXya3kW0A0fsYDMcDb aqv3LofIlyitPbuOWXbAQN6zjFvMFKVJQeHVWjqK/8nsehGjR3FiabP5dSOVZr9AVpUS5ywWu eYj6ob+WqFL/4aVirWA3EjsHmINAssFjRbSdpKzh6sQpIT3yUUKcbUQzpHNmlVIwzeV/+6KQl V1PsSxKZV6VfVUxzM+RG9JrGkuQA9bNxki/yav3ab1+oK8qNTzlcthncIU1EQGxrXfWq0DaNw cwiHpmT4KTvgpUQ7fpcZO3E+P+ert0kO6p3HtN9qwcURrK7dmJjQ3ggik/k2Kk9fhljwQtCpM zkhuFHAr9ldK31atZTQ6CxVh2DmBTu8Kvek8GU2iKCNskP3J80ojnYv2TiEL4Bqqa1OqRnT7d rTS7XYDke4TzTTN9Hr7yfqSD+TSDCiCpOOojr03xTY6da4adW9v6dEDILZ3a5uwcM64Xre3+k g68TL4T4uGM67jSzIBiC7tRjH9Enus2aYGirlHPGT6aAm9ldU6Ghdz/U06+RO/A0B9KR7XJUc iDHA2HErECiblD6TTo1UZs6n1zY1vj0nUMFc6ubWcGcfi89BZjHV0w+gfktPQGr+83pc5VcsJ 6AVTQuagv2XmLxw0c3xdrbQR0O3o2RSYUQ8V34i0Yp9sv+x7KZeLgBmgha2BBdzyp6NgHcPtM 6JYRwiSIFPY6dtqir0S3COyk3kDgt/i3QxlNuShIPoW6Ge0Yoc= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> ret = i2c_smbus_write_byte_data(client, offset, value); >> - if (ret < 0) { >> - dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n", >> - sii9234_client_name[id], offset, value); >> - ctx->i2c_error = ret; >> - } >> + if (!ret) >> + return 0; > > Ugh. No. Don't do success handling on the last if statement. I find my approach useful in this case. > Also while I personally prefer testing for non-zero, I got used to this checking style to some degree. > the ALSA people got annoyed at you for changing tests for < 0 It seems that involved software developers have got special preferences there. > but you're doing it again. I dared to propose such an adjustment once more. Would you like discuss corresponding reasons any further? > And it introduces a bug, Unfortunately, a hiccup in my software development attention … > although I see now that you fixed it in v2. Thanks that you noticed also this small update. https://patchwork.kernel.org/patch/10021767/ https://lkml.kernel.org/r/<2ecf0bb7-7129-40e4-cefc-0bc2d0f7ee8b@users.sourceforge.net> > I can't get excited about these sort of risky low value patches. I try again to point special software improvement opportunities out. >> +report_failure: >> + dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n", >> + sii9234_client_name[id], offset, value); >> + ctx->i2c_error = ret; >> return ret; >> } How do you think about to move this source code to the end of this function implementation? Regards, Markus