Fwd: [PATCH 4/4] [v3] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002

[Timur Tabi]

On 11/15/2017 12:47 AM, Varadarajan Narayanan wrote:
>> +	if (id->driver_data == QDF2XXX_V1) {
>> +		avail_gpios = num_gpios;
>> +
>> +		gpios = devm_kcalloc(&pdev->dev, avail_gpios, sizeof(gpios[0]),
>> +				     GFP_KERNEL);
> Wouldn't kmalloc suffice since the array gets initialized just below.

Sure.

>> +		if (!gpios)
>> +			return -ENOMEM;
>> +
>> +		for (i = 0; i < avail_gpios; i++)
>> +			gpios[i] = i;
>> +	} else {
>> +		/* The number of GPIOs in the approved list */
>> +		ret = device_property_read_u16_array(&pdev->dev, "gpios",
>> +						     NULL, 0);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "missing 'num-gpios' property\n");
>> +			return ret;
>> +		}
>> +		if (!ret || ret > MAX_GPIOS) {
> Instead of ret > MAX_GPIOS, should we check for ret > num_gpios?

Yes.

> 
>> +			dev_err(&pdev->dev, "invalid 'num-gpios' property\n");
>> +			return -ENODEV;
>> +		}
> In both the above error messages 'num-gpios' should be 'gpios'

Thanks, I not sure how that got messed up.

> 
>> +		avail_gpios = ret;
>> +
>> +		gpios = devm_kcalloc(&pdev->dev, avail_gpios, sizeof(gpios[0]),
>> +				     GFP_KERNEL);
> devm_kmalloc?

Ok.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.