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 <rog...@ti.com>
>> ---
>> 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
