From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752720AbcLHI1C (ORCPT ); Thu, 8 Dec 2016 03:27:02 -0500 Received: from fllnx210.ext.ti.com ([198.47.19.17]:16612 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752079AbcLHI1A (ORCPT ); Thu, 8 Dec 2016 03:27:00 -0500 Subject: Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration To: Chanwoo Choi , References: <1481112734-19536-1-git-send-email-rogerq@ti.com> <584804A1.3070208@samsung.com> CC: From: Roger Quadros Message-ID: Date: Thu, 8 Dec 2016 10:26:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <584804A1.3070208@samsung.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chanwoo, On 07/12/16 14:46, Chanwoo Choi wrote: > 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 I'm OK with this. > > 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" This is also fine. I'll send a v2. > >> + return -EINVAL; >> + } >> + >> palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL); >> if (!palmas_usb) >> return -ENOMEM; >> > > -- cheers, -roger