diff mbox

pinctrl: qcom: qdf2xxx: improve error checking and reporting

Message ID 1447171030-15056-1-git-send-email-timur@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Timur Tabi Nov. 10, 2015, 3:57 p.m. UTC
The driver doesn't report an error message if the ACPI tables are missing
the num-gpios property (which indicates how many GPIOs there are on this
SOC), and it didn't check to ensure that the mallocs didn't fail.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/pinctrl/qcom/pinctrl-qdf2xxx.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Linus Walleij Nov. 17, 2015, 1:33 p.m. UTC | #1
On Tue, Nov 10, 2015 at 4:57 PM, Timur Tabi <timur@codeaurora.org> wrote:

> The driver doesn't report an error message if the ACPI tables are missing
> the num-gpios property (which indicates how many GPIOs there are on this
> SOC), and it didn't check to ensure that the mallocs didn't fail.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>

Waiting for Björn's ACK on this.

And I suspect Arnd would be interested to know if you are doing
ACPI on something that is certainly not a server-class hardware.
Because you shouldn't.

Yours,
Linus Walleij
Timur Tabi Nov. 17, 2015, 1:36 p.m. UTC | #2
Linus Walleij wrote:
> And I suspect Arnd would be interested to know if you are doing
> ACPI on something that is certainly not a server-class hardware.
> Because you shouldn't.

This driver is for the TLMM pin control device on Qualcomm's ARM64 
Server chip.
Arnd Bergmann Nov. 17, 2015, 2:15 p.m. UTC | #3
On Tuesday 17 November 2015 14:33:49 Linus Walleij wrote:
> On Tue, Nov 10, 2015 at 4:57 PM, Timur Tabi <timur@codeaurora.org> wrote:
> 
> > The driver doesn't report an error message if the ACPI tables are missing
> > the num-gpios property (which indicates how many GPIOs there are on this
> > SOC), and it didn't check to ensure that the mallocs didn't fail.
> >
> > Signed-off-by: Timur Tabi <timur@codeaurora.org>
> 
> Waiting for Björn's ACK on this.
> 
> And I suspect Arnd would be interested to know if you are doing
> ACPI on something that is certainly not a server-class hardware.
> Because you shouldn't.
> 

It's more fun just watching this really. No comment.

	Arnd
Linus Walleij Nov. 17, 2015, 2:44 p.m. UTC | #4
On Tue, Nov 17, 2015 at 2:36 PM, Timur Tabi <timur@codeaurora.org> wrote:
> Linus Walleij wrote:
>>
>> And I suspect Arnd would be interested to know if you are doing
>> ACPI on something that is certainly not a server-class hardware.
>> Because you shouldn't.
>
> This driver is for the TLMM pin control device on Qualcomm's ARM64 Server
> chip.

Ohhh sweet. OK then.

Yours,
Linus Walleij
Bjorn Andersson Nov. 23, 2015, 6:02 p.m. UTC | #5
On Tue 10 Nov 07:57 PST 2015, Timur Tabi wrote:

> The driver doesn't report an error message if the ACPI tables are missing
> the num-gpios property (which indicates how many GPIOs there are on this
> SOC), and it didn't check to ensure that the mallocs didn't fail.
> 
> Signed-off-by: Timur Tabi <timur@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>

Regards,
Bjorn
Linus Walleij Dec. 1, 2015, 8:59 a.m. UTC | #6
On Tue, Nov 10, 2015 at 4:57 PM, Timur Tabi <timur@codeaurora.org> wrote:

> The driver doesn't report an error message if the ACPI tables are missing
> the num-gpios property (which indicates how many GPIOs there are on this
> SOC), and it didn't check to ensure that the mallocs didn't fail.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>

Patch applied with Björn's review tag.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c b/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
index e9ff3bc..f448534 100644
--- a/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
+++ b/drivers/pinctrl/qcom/pinctrl-qdf2xxx.c
@@ -32,6 +32,9 @@ 
 
 static struct msm_pinctrl_soc_data qdf2xxx_pinctrl;
 
+/* A reasonable limit to the number of GPIOS */
+#define MAX_GPIOS	256
+
 static int qdf2xxx_pinctrl_probe(struct platform_device *pdev)
 {
 	struct pinctrl_pin_desc *pins;
@@ -42,11 +45,13 @@  static int qdf2xxx_pinctrl_probe(struct platform_device *pdev)
 
 	/* Query the number of GPIOs from ACPI */
 	ret = device_property_read_u32(&pdev->dev, "num-gpios", &num_gpios);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "missing num-gpios property\n");
 		return ret;
+	}
 
-	if (!num_gpios) {
-		dev_warn(&pdev->dev, "missing num-gpios property\n");
+	if (!num_gpios || num_gpios > MAX_GPIOS) {
+		dev_warn(&pdev->dev, "invalid num-gpios property\n");
 		return -ENODEV;
 	}
 
@@ -55,6 +60,9 @@  static int qdf2xxx_pinctrl_probe(struct platform_device *pdev)
 	groups = devm_kcalloc(&pdev->dev, num_gpios,
 		sizeof(struct msm_pingroup), GFP_KERNEL);
 
+	if (!pins || !groups)
+		return -ENOMEM;
+
 	for (i = 0; i < num_gpios; i++) {
 		pins[i].number = i;