From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752423AbcLGMq3 (ORCPT ); Wed, 7 Dec 2016 07:46:29 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:60245 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751064AbcLGMq2 (ORCPT ); Wed, 7 Dec 2016 07:46:28 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfee61b-f796f6d000004092-16-584804a13092 Content-transfer-encoding: 8BIT Message-id: <584804A1.3070208@samsung.com> Date: Wed, 07 Dec 2016 21:46:25 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Roger Quadros , myungjoo.ham@samsung.com Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration References: <1481112734-19536-1-git-send-email-rogerq@ti.com> In-reply-to: <1481112734-19536-1-git-send-email-rogerq@ti.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHIsWRmVeSWpSXmKPExsVy+t9jAd1FLB4RBrfmsVhc3jWHzeJ24wo2 i55HWg7MHn1bVjF6HL+xncnj8ya5AOYoN5uM1MSU1CKF1Lzk/JTMvHRbpdAQN10LJYW8xNxU W6UIXd+QICWFssScUiDPyAANODgHuAcr6dsluGUc3PCcqeAGd8WtPYdYGhg3cnYxcnBICJhI XDys2cXICWSKSVy4t56ti5GLQ0hgKaPEtufHWEASvAKCEj8m32MBqWcWkJc4cikbJMwsoC4x ad4iZoj6B4wSN46tZoOo15I4fe0BE4jNIqAq8XDBbzCbDSi+/8UNsBp+AUWJqz8eM4LMFBWI kOg+UQliighYS2z47gMxXkHi171NrCC2sICvxO/j/YwQqyYzSry5+gpsDKeAncSR6fuZJzAK zkJy6SyES2chuXQBI/MqRonUguSC4qT0XKO81HK94sTc4tK8dL3k/NxNjOC4eSa9g/HwLvdD jAIcjEo8vBEc7hFCrIllxZW5hxglOJiVRHhf/wcK8aYkVlalFuXHF5XmpBYfYjQFenUis5Ro cj4wpvNK4g1NzE3MjQ0szC0tTYyUxHkbZz8LFxJITyxJzU5NLUgtgulj4uCUamBsip94w2Mz 0/6br3w0JZZNfferRk770fYJ22a4ML3cq6VWG7ls2/euDRcm7DPvnsOTHuv73DnnFuvbrYHc S0oWvrtx09lt64V/7jM/zXObXtcXLaPn/6g0vSuFx9hQKczXdOPU8jTTGzPto2Yd2/rmfa6h 0Zn0n8udTdqn+DT+Ydx0xIvX2H2dEktxRqKhFnNRcSIA4EphJbECAAA= X-MTR: 20000000000000000@CPGS Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Roger, Looks good to me. But I have some comment. How about changing the subject as following? - old : Fail gracefully if invalid configuration - new : Check the parent instance to prevent the NULL pointer error On 2016년 12월 07일 21:12, Roger Quadros wrote: > extcon-palmas must be child of palmas and expects parent's > drvdata to be valid. Check for non NULL parent drvdata and > fail if it is NULL. Not doing so will result in a NULL > pointer dereference later in the probe() parent drvdata > is NULL (e.g. misplaced extcon-palmas node in device tree). > > Signed-off-by: Roger Quadros > --- > drivers/extcon/extcon-palmas.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c > index 634ba70..ec987ab 100644 > --- a/drivers/extcon/extcon-palmas.c > +++ b/drivers/extcon/extcon-palmas.c > @@ -190,6 +190,11 @@ static int palmas_usb_probe(struct platform_device *pdev) > struct palmas_usb *palmas_usb; > int status; > > + if (!palmas) { > + dev_err(&pdev->dev, "device has invalid parent\n"); How about changing the error message as following? because the extcon-palmas used the 'failed to ..' style for error message. - "failed to get valid parent" > + return -EINVAL; > + } > + > palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL); > if (!palmas_usb) > return -ENOMEM; > Best Regards, Chanwoo Choi