From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C37A0C47404 for ; Mon, 7 Oct 2019 10:22:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8956E206BB for ; Mon, 7 Oct 2019 10:22:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="feJvXfvo" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727334AbfJGKWb (ORCPT ); Mon, 7 Oct 2019 06:22:31 -0400 Received: from userp2130.oracle.com ([156.151.31.86]:43652 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726010AbfJGKWa (ORCPT ); Mon, 7 Oct 2019 06:22:30 -0400 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x97AJJ9K096797; Mon, 7 Oct 2019 10:22:24 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : mime-version : content-type; s=corp-2019-08-05; bh=zRhaoJf1IYCzNl91rFA2hoUUNaBQLG5Ixrc7LjJhwME=; b=feJvXfvosbX8esA8u/DZ3xgJn5smmQvtPdGF6J9Wq6CUhujq3gbdZrFn6Z4uW7hY+0s3 JT2FxKnQykaTm9M8wFLjyNKGVpj0xwkvmvcdTHAsSInhUlPbEWRO9bslaGBOmT6trEXC b9GDnx+8sWwex5TaIvnWhAbOihKbhSWG/1vK3Bp5Y2jg2c3C5vyIF5HfqpdmxU9IX3QL HkRCsWoWPTCq7IYA3/KGDeq0Mra4ejis7D77zo6OFT7MAkVfFS3SYgWwgT9Df8Ol1Jgh UoQQ+zxAOAkTLLJ1EZL+OPRd4EL0uytdhhN5Jf/xhQCtFYYXhfNMW7MXxud6Ar229uxM 8Q== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by userp2130.oracle.com with ESMTP id 2vejku5v4y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 07 Oct 2019 10:22:24 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x97AJ3IM004175; Mon, 7 Oct 2019 10:22:24 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3020.oracle.com with ESMTP id 2vg203dydm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 07 Oct 2019 10:22:24 +0000 Received: from abhmp0015.oracle.com (abhmp0015.oracle.com [141.146.116.21]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x97AMLmW004990; Mon, 7 Oct 2019 10:22:22 GMT Received: from kadam (/41.57.98.10) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 07 Oct 2019 03:22:21 -0700 Date: Mon, 7 Oct 2019 13:22:14 +0300 From: Dan Carpenter , Heikki Krogerus To: kbuild@01.org, Biju Das Cc: kbuild-all@01.org, linux-usb@vger.kernel.org, Greg Kroah-Hartman Subject: [usb:usb-next 32/38] drivers/usb/typec/hd3ss3220.c:182 hd3ss3220_probe() warn: passing zero to 'PTR_ERR' Message-ID: <20191007102214.GE21515@kadam> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.9.4 (2018-02-28) X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9402 signatures=668684 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000 definitions=main-1910070103 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9402 signatures=668684 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000 definitions=main-1910070103 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-next head: dd3fd317e2beb899cbffcf364de049b9f9a02db5 commit: 1c48c759ef4bb9031b3347277f04484e07e27d97 [32/38] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller If you fix the issue, kindly add following tag Reported-by: kbuild test robot Reported-by: Dan Carpenter smatch warnings: drivers/usb/typec/hd3ss3220.c:182 hd3ss3220_probe() warn: passing zero to 'PTR_ERR' # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=1c48c759ef4bb9031b3347277f04484e07e27d97 git remote add usb https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git git remote update usb git checkout 1c48c759ef4bb9031b3347277f04484e07e27d97 vim +/PTR_ERR +182 drivers/usb/typec/hd3ss3220.c 1c48c759ef4bb9 Biju Das 2019-09-04 152 1c48c759ef4bb9 Biju Das 2019-09-04 153 static int hd3ss3220_probe(struct i2c_client *client, 1c48c759ef4bb9 Biju Das 2019-09-04 154 const struct i2c_device_id *id) 1c48c759ef4bb9 Biju Das 2019-09-04 155 { 1c48c759ef4bb9 Biju Das 2019-09-04 156 struct hd3ss3220 *hd3ss3220; 1c48c759ef4bb9 Biju Das 2019-09-04 157 struct fwnode_handle *connector; 1c48c759ef4bb9 Biju Das 2019-09-04 158 int ret; 1c48c759ef4bb9 Biju Das 2019-09-04 159 unsigned int data; 1c48c759ef4bb9 Biju Das 2019-09-04 160 1c48c759ef4bb9 Biju Das 2019-09-04 161 hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220), 1c48c759ef4bb9 Biju Das 2019-09-04 162 GFP_KERNEL); 1c48c759ef4bb9 Biju Das 2019-09-04 163 if (!hd3ss3220) 1c48c759ef4bb9 Biju Das 2019-09-04 164 return -ENOMEM; 1c48c759ef4bb9 Biju Das 2019-09-04 165 1c48c759ef4bb9 Biju Das 2019-09-04 166 i2c_set_clientdata(client, hd3ss3220); 1c48c759ef4bb9 Biju Das 2019-09-04 167 1c48c759ef4bb9 Biju Das 2019-09-04 168 hd3ss3220->dev = &client->dev; 1c48c759ef4bb9 Biju Das 2019-09-04 169 hd3ss3220->regmap = devm_regmap_init_i2c(client, &config); 1c48c759ef4bb9 Biju Das 2019-09-04 170 if (IS_ERR(hd3ss3220->regmap)) 1c48c759ef4bb9 Biju Das 2019-09-04 171 return PTR_ERR(hd3ss3220->regmap); 1c48c759ef4bb9 Biju Das 2019-09-04 172 1c48c759ef4bb9 Biju Das 2019-09-04 173 hd3ss3220_set_source_pref(hd3ss3220, 1c48c759ef4bb9 Biju Das 2019-09-04 174 HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT); 1c48c759ef4bb9 Biju Das 2019-09-04 175 connector = device_get_named_child_node(hd3ss3220->dev, "connector"); 1c48c759ef4bb9 Biju Das 2019-09-04 176 if (IS_ERR(connector)) 1c48c759ef4bb9 Biju Das 2019-09-04 177 return PTR_ERR(connector); 1c48c759ef4bb9 Biju Das 2019-09-04 178 1c48c759ef4bb9 Biju Das 2019-09-04 179 hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector); 1c48c759ef4bb9 Biju Das 2019-09-04 180 fwnode_handle_put(connector); 1c48c759ef4bb9 Biju Das 2019-09-04 181 if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) 1c48c759ef4bb9 Biju Das 2019-09-04 @182 return PTR_ERR(hd3ss3220->role_sw); When fwnode_usb_role_switch_get() returns NULL then we return success here. It seems like a bug. When function returns a mix of NULL and error pointers then NULL is a special case of success. For example, a module tries to request a feature but that feature is deliberately turned off. It's not an error so we can't return an error pointer, but at same time we can't return a valid pointer because the feature is disabled. For fwnode_usb_role_switch_get() it is a bit unclear to me what NULL means in that context, and there are no comments to explain it... The fwnode_connection_find_match() is the same way where it mixes NULL and error pointers, doesn't have a comment, and it really seems like NULL is an error, not a special case of success like it's supposed to be. I have added Heikki Krogerus to the CC list. 1c48c759ef4bb9 Biju Das 2019-09-04 183 1c48c759ef4bb9 Biju Das 2019-09-04 184 hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE; 1c48c759ef4bb9 Biju Das 2019-09-04 185 hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set; 1c48c759ef4bb9 Biju Das 2019-09-04 186 hd3ss3220->typec_cap.type = TYPEC_PORT_DRP; 1c48c759ef4bb9 Biju Das 2019-09-04 187 hd3ss3220->typec_cap.data = TYPEC_PORT_DRD; 1c48c759ef4bb9 Biju Das 2019-09-04 188 1c48c759ef4bb9 Biju Das 2019-09-04 189 hd3ss3220->port = typec_register_port(&client->dev, 1c48c759ef4bb9 Biju Das 2019-09-04 190 &hd3ss3220->typec_cap); 1c48c759ef4bb9 Biju Das 2019-09-04 191 if (IS_ERR(hd3ss3220->port)) 1c48c759ef4bb9 Biju Das 2019-09-04 192 return PTR_ERR(hd3ss3220->port); This error path should call usb_role_switch_put(hd3ss3220->role_sw) and probably a fwnode_handle_put() as well? I noticed this because I scrolled to the bottom of the fucntion and noticed that there was only one error label. When you see a single error label like that, it's normally a red flag that the error handling is buggy. Error labels should have descriptive names which say what they do. I suggest something like: err_unreg_port: typec_unregister_port(hd3ss3220->port); err_put_role: usb_role_switch_put(hd3ss3220->role_sw); err_put_handle: fwnode_handle_put(foo bar); return ret; The rule behind this style of error handling is that you just have to keep track of the most recently allocated resource and at the bottom you free them in the reverse order from how you allocated them. Here we had allocated ->role_sw but the typec_register_port() so we do goto free_role_sw; Now people can guess what the goto does because the name is descriptive and since it matches the most recently allocated resource that means it's okay. 1c48c759ef4bb9 Biju Das 2019-09-04 193 1c48c759ef4bb9 Biju Das 2019-09-04 194 hd3ss3220_set_role(hd3ss3220); 1c48c759ef4bb9 Biju Das 2019-09-04 195 ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data); 1c48c759ef4bb9 Biju Das 2019-09-04 196 if (ret < 0) 1c48c759ef4bb9 Biju Das 2019-09-04 197 goto error; 1c48c759ef4bb9 Biju Das 2019-09-04 198 1c48c759ef4bb9 Biju Das 2019-09-04 199 if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) { 1c48c759ef4bb9 Biju Das 2019-09-04 200 ret = regmap_write(hd3ss3220->regmap, 1c48c759ef4bb9 Biju Das 2019-09-04 201 HD3SS3220_REG_CN_STAT_CTRL, 1c48c759ef4bb9 Biju Das 2019-09-04 202 data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS); 1c48c759ef4bb9 Biju Das 2019-09-04 203 if (ret < 0) 1c48c759ef4bb9 Biju Das 2019-09-04 204 goto error; 1c48c759ef4bb9 Biju Das 2019-09-04 205 } 1c48c759ef4bb9 Biju Das 2019-09-04 206 1c48c759ef4bb9 Biju Das 2019-09-04 207 if (client->irq > 0) { 1c48c759ef4bb9 Biju Das 2019-09-04 208 ret = devm_request_threaded_irq(&client->dev, client->irq, NULL, 1c48c759ef4bb9 Biju Das 2019-09-04 209 hd3ss3220_irq_handler, 1c48c759ef4bb9 Biju Das 2019-09-04 210 IRQF_TRIGGER_FALLING | IRQF_ONESHOT, 1c48c759ef4bb9 Biju Das 2019-09-04 211 "hd3ss3220", &client->dev); 1c48c759ef4bb9 Biju Das 2019-09-04 212 if (ret) 1c48c759ef4bb9 Biju Das 2019-09-04 213 goto error; 1c48c759ef4bb9 Biju Das 2019-09-04 214 } 1c48c759ef4bb9 Biju Das 2019-09-04 215 1c48c759ef4bb9 Biju Das 2019-09-04 216 ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV); 1c48c759ef4bb9 Biju Das 2019-09-04 217 if (ret < 0) 1c48c759ef4bb9 Biju Das 2019-09-04 218 goto error; 1c48c759ef4bb9 Biju Das 2019-09-04 219 1c48c759ef4bb9 Biju Das 2019-09-04 220 dev_info(&client->dev, "probed revision=0x%x\n", ret); 1c48c759ef4bb9 Biju Das 2019-09-04 221 1c48c759ef4bb9 Biju Das 2019-09-04 222 return 0; 1c48c759ef4bb9 Biju Das 2019-09-04 223 error: 1c48c759ef4bb9 Biju Das 2019-09-04 224 typec_unregister_port(hd3ss3220->port); 1c48c759ef4bb9 Biju Das 2019-09-04 225 usb_role_switch_put(hd3ss3220->role_sw); 1c48c759ef4bb9 Biju Das 2019-09-04 226 1c48c759ef4bb9 Biju Das 2019-09-04 227 return ret; 1c48c759ef4bb9 Biju Das 2019-09-04 228 } --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation