[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/mvebu/pinctrl-dove.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-dove.c b/drivers/pinctrl/mvebu/pinctrl-dove.c
index ffe74b2..a8b9b42 100644
--- a/drivers/pinctrl/mvebu/pinctrl-dove.c
+++ b/drivers/pinctrl/mvebu/pinctrl-dove.c
@@ -578,8 +578,12 @@ static struct of_device_id dove_pinctrl_of_match[] __devinitdata = {
 
 static int __devinit dove_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match =
-		of_match_device(dove_pinctrl_of_match, &pdev->dev);
+	const struct of_device_id *match;
+
+	match = of_match_device(dove_pinctrl_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
 	pdev->dev.platform_data = match->data;
 
 	/*
-- 
1.7.9.5

[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/mvebu/pinctrl-dove.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-dove.c b/drivers/pinctrl/mvebu/pinctrl-dove.c
index ffe74b2..a8b9b42 100644
--- a/drivers/pinctrl/mvebu/pinctrl-dove.c
+++ b/drivers/pinctrl/mvebu/pinctrl-dove.c
@@ -578,8 +578,12 @@ static struct of_device_id dove_pinctrl_of_match[] __devinitdata = {
 
 static int __devinit dove_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match =
-		of_match_device(dove_pinctrl_of_match, &pdev->dev);
+	const struct of_device_id *match;
+
+	match = of_match_device(dove_pinctrl_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
 	pdev->dev.platform_data = match->data;
 
 	/*
-- 
1.7.9.5

[PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/mvebu/pinctrl-kirkwood.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
index 9a74ef6..fe885ca 100644
--- a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
+++ b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
@@ -444,8 +444,11 @@ static struct of_device_id kirkwood_pinctrl_of_match[] __devinitdata = {
 
 static int __devinit kirkwood_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match =
-		of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+	const struct of_device_id *match;
+
+	match = of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
 	pdev->dev.platform_data = match->data;
 	return mvebu_pinctrl_probe(pdev);
 }
-- 
1.7.9.5

[PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/mvebu/pinctrl-kirkwood.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
index 9a74ef6..fe885ca 100644
--- a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
+++ b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
@@ -444,8 +444,11 @@ static struct of_device_id kirkwood_pinctrl_of_match[] __devinitdata = {
 
 static int __devinit kirkwood_pinctrl_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match =
-		of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+	const struct of_device_id *match;
+
+	match = of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
 	pdev->dev.platform_data = match->data;
 	return mvebu_pinctrl_probe(pdev);
 }
-- 
1.7.9.5

[PATCH 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
 	uint32_t *tmp;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
+	const struct of_device_id *match;
 
 	if (!np)
 		return -ENODEV;
 
+	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	if (!match)
+		return -ENODEV;
+
 	info->dev = &pdev->dev;
-	info->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
 	at91_pinctrl_child_count(info, np);
 
 	if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
 static int __devinit at91_gpio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct resource *res;
 	struct at91_gpio_chip *at91_chip = NULL;
 	struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+	match = of_match_device(at91_gpio_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-- 
1.7.9.5

[PATCH 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
 	uint32_t *tmp;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
+	const struct of_device_id *match;
 
 	if (!np)
 		return -ENODEV;
 
+	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	if (!match)
+		return -ENODEV;
+
 	info->dev = &pdev->dev;
-	info->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
 	at91_pinctrl_child_count(info, np);
 
 	if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
 static int __devinit at91_gpio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct resource *res;
 	struct at91_gpio_chip *at91_chip = NULL;
 	struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+	match = of_match_device(at91_gpio_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-- 
1.7.9.5

[PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 238060e..40bd1b3 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
 
 	if (platid)
 		version = platid->driver_data;
-	else if (np)
-		version = (unsigned int)
-			of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
+	else if (np) {
+		const struct of_device_id *match;
+
+		match = of_match_device(nmk_pinctrl_match, &pdev->dev);
+		if (!match)
+			return -ENODEV;
+		version = (unsigned int) match->data;
+	}
 
 	/* Poke in other ASIC variants here */
 	if (version == PINCTRL_NMK_STN8815)
-- 
1.7.9.5

[PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 238060e..40bd1b3 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
 
 	if (platid)
 		version = platid->driver_data;
-	else if (np)
-		version = (unsigned int)
-			of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
+	else if (np) {
+		const struct of_device_id *match;
+
+		match = of_match_device(nmk_pinctrl_match, &pdev->dev);
+		if (!match)
+			return -ENODEV;
+		version = (unsigned int) match->data;
+	}
 
 	/* Poke in other ASIC variants here */
 	if (version == PINCTRL_NMK_STN8815)
-- 
1.7.9.5

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
This resend CC Jean-Christophe.

 drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
 	uint32_t *tmp;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
+	const struct of_device_id *match;
 
 	if (!np)
 		return -ENODEV;
 
+	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	if (!match)
+		return -ENODEV;
+
 	info->dev = &pdev->dev;
-	info->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
 	at91_pinctrl_child_count(info, np);
 
 	if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
 static int __devinit at91_gpio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct resource *res;
 	struct at91_gpio_chip *at91_chip = NULL;
 	struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+	match = of_match_device(at91_gpio_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-- 
1.7.9.5

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

of_match_device() may return NULL.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
This resend CC Jean-Christophe.

 drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
 	uint32_t *tmp;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
+	const struct of_device_id *match;
 
 	if (!np)
 		return -ENODEV;
 
+	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	if (!match)
+		return -ENODEV;
+
 	info->dev = &pdev->dev;
-	info->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
 	at91_pinctrl_child_count(info, np);
 
 	if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
 static int __devinit at91_gpio_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct resource *res;
 	struct at91_gpio_chip *at91_chip = NULL;
 	struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
-		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+	match = of_match_device(at91_gpio_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-- 
1.7.9.5

Re: [PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 12:51:36 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Could you detail under what conditions of_match_device() may return
NULL in the specific case of this driver? This of_match_device() call
is using the same dove_pinctrl_of_match array that is used to ->probe()
this driver. So I don't see how you can get into ->probe() without
having a matching entry.

Am I missing something?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 12:51:36 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Could you detail under what conditions of_match_device() may return
NULL in the specific case of this driver? This of_match_device() call
is using the same dove_pinctrl_of_match array that is used to ->probe()
this driver. So I don't see how you can get into ->probe() without
having a matching entry.

Am I missing something?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Nicolas Ferre]

On 11/15/2012 05:58 AM, Axel Lin :
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Seems sensible,

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
> This resend CC Jean-Christophe.
> 
>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>  	uint32_t *tmp;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device_node *child;
> +	const struct of_device_id *match;
>  
>  	if (!np)
>  		return -ENODEV;
>  
> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	if (!match)
> +		return -ENODEV;
> +
>  	info->dev = &pdev->dev;
> -	info->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
>  	at91_pinctrl_child_count(info, np);
>  
>  	if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
>  	struct resource *res;
>  	struct at91_gpio_chip *at91_chip = NULL;
>  	struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> +	if (!match)
> +		return -ENODEV;
> +
> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>  	at91_chip->pioc_virq = irq;
>  	at91_chip->pioc_idx = alias_idx;
>  
> 


-- 
Nicolas Ferre

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Nicolas Ferre]

On 11/15/2012 05:58 AM, Axel Lin :
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Seems sensible,

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
> This resend CC Jean-Christophe.
> 
>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>  	uint32_t *tmp;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device_node *child;
> +	const struct of_device_id *match;
>  
>  	if (!np)
>  		return -ENODEV;
>  
> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	if (!match)
> +		return -ENODEV;
> +
>  	info->dev = &pdev->dev;
> -	info->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
>  	at91_pinctrl_child_count(info, np);
>  
>  	if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
>  	struct resource *res;
>  	struct at91_gpio_chip *at91_chip = NULL;
>  	struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> +	if (!match)
> +		return -ENODEV;
> +
> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>  	at91_chip->pioc_virq = irq;
>  	at91_chip->pioc_idx = alias_idx;
>  
> 


-- 
Nicolas Ferre

Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:58 Thu 15 Nov     , Axel Lin wrote:
> of_match_device() may return NULL.
this is not possible on at91

and I do a oups here as if we have a NULL pointer which means the driver is
wrong

Best Regards,
J.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> This resend CC Jean-Christophe.
> 
>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>  	uint32_t *tmp;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device_node *child;
> +	const struct of_device_id *match;
>  
>  	if (!np)
>  		return -ENODEV;
>  
> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	if (!match)
> +		return -ENODEV;
> +
>  	info->dev = &pdev->dev;
> -	info->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
>  	at91_pinctrl_child_count(info, np);
>  
>  	if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
>  	struct resource *res;
>  	struct at91_gpio_chip *at91_chip = NULL;
>  	struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> +	if (!match)
> +		return -ENODEV;
> +
> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>  	at91_chip->pioc_virq = irq;
>  	at91_chip->pioc_idx = alias_idx;
>  
> -- 
> 1.7.9.5
> 
> 
> 

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:58 Thu 15 Nov     , Axel Lin wrote:
> of_match_device() may return NULL.
this is not possible on at91

and I do a oups here as if we have a NULL pointer which means the driver is
wrong

Best Regards,
J.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> This resend CC Jean-Christophe.
> 
>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>  	uint32_t *tmp;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device_node *child;
> +	const struct of_device_id *match;
>  
>  	if (!np)
>  		return -ENODEV;
>  
> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	if (!match)
> +		return -ENODEV;
> +
>  	info->dev = &pdev->dev;
> -	info->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
>  	at91_pinctrl_child_count(info, np);
>  
>  	if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
>  	struct resource *res;
>  	struct at91_gpio_chip *at91_chip = NULL;
>  	struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> +	if (!match)
> +		return -ENODEV;
> +
> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>  	at91_chip->pioc_virq = irq;
>  	at91_chip->pioc_idx = alias_idx;
>  
> -- 
> 1.7.9.5
> 
> 
> 

[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Axel Lin]

2012/11/15 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

> Dear Axel Lin,
>
> On Thu, 15 Nov 2012 12:51:36 +0800, Axel Lin wrote:
> > of_match_device() may return NULL.
> >
> > Signed-off-by: Axel Lin <axel.lin@ingics.com>
>
> Could you detail under what conditions of_match_device() may return
> NULL in the specific case of this driver? This of_match_device() call
> is using the same dove_pinctrl_of_match array that is used to ->probe()
> this driver. So I don't see how you can get into ->probe() without
> having a matching entry.
>
> Am I missing something?
>

 hi Thomas,
You are right.

I was thinking when the case CONFIG_OF is not selected,
of_match_ptr(dove_pinctrl_of_match) is NULL and of_match_device returns
NULL.
But this driver only works when CONFIG_OF is selected.

BTW, I found there is no way to compile the dove and kirkwood pinctrl
drivers.
I need add below patch to compile these two drivers, how do you think about
below patch?
( In dove, the USE_OF is optional, it is selected only when MACH_DOVE_DT is
enabled )

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 14f8160..dda6785 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -531,6 +531,8 @@ config ARCH_DOVE
        select CPU_V7
        select GENERIC_CLOCKEVENTS
        select MIGHT_HAVE_PCI
+       select PINCTRL
+       select PINCTRL_DOVE if USE_OF
        select PLAT_ORION_LEGACY
        select USB_ARCH_HAS_EHCI
        help
@@ -542,6 +544,8 @@ config ARCH_KIRKWOOD
        select CPU_FEROCEON
        select GENERIC_CLOCKEVENTS
        select PCI
+       select PINCTRL
+       select PINCTRL_KIRKWOOD
        select PLAT_ORION_LEGACY
        help
          Support for the following Marvell Kirkwood series SoCs:

Regards,
Axel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121115/be2e1627/attachment-0001.html>

Re: [PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 21:44:07 +0800, Axel Lin wrote:

> BTW, I found there is no way to compile the dove and kirkwood pinctrl
> drivers.
> I need add below patch to compile these two drivers, how do you think about
> below patch?
> ( In dove, the USE_OF is optional, it is selected only when MACH_DOVE_DT is
> enabled )
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 14f8160..dda6785 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -531,6 +531,8 @@ config ARCH_DOVE
>         select CPU_V7
>         select GENERIC_CLOCKEVENTS
>         select MIGHT_HAVE_PCI
> +       select PINCTRL
> +       select PINCTRL_DOVE if USE_OF
>         select PLAT_ORION_LEGACY
>         select USB_ARCH_HAS_EHCI
>         help
> @@ -542,6 +544,8 @@ config ARCH_KIRKWOOD
>         select CPU_FEROCEON
>         select GENERIC_CLOCKEVENTS
>         select PCI
> +       select PINCTRL
> +       select PINCTRL_KIRKWOOD
>         select PLAT_ORION_LEGACY
>         help
>           Support for the following Marvell Kirkwood series SoCs:

See "[PATCH 1/5] ARM: Kirkwood: Allow use of pinctrl" and "[PATCH 4/5]
ARM: Dove: Make use of pinctrl driver" which have been posted by Andrew
Lunn on October, 24th.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 21:44:07 +0800, Axel Lin wrote:

> BTW, I found there is no way to compile the dove and kirkwood pinctrl
> drivers.
> I need add below patch to compile these two drivers, how do you think about
> below patch?
> ( In dove, the USE_OF is optional, it is selected only when MACH_DOVE_DT is
> enabled )
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 14f8160..dda6785 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -531,6 +531,8 @@ config ARCH_DOVE
>         select CPU_V7
>         select GENERIC_CLOCKEVENTS
>         select MIGHT_HAVE_PCI
> +       select PINCTRL
> +       select PINCTRL_DOVE if USE_OF
>         select PLAT_ORION_LEGACY
>         select USB_ARCH_HAS_EHCI
>         help
> @@ -542,6 +544,8 @@ config ARCH_KIRKWOOD
>         select CPU_FEROCEON
>         select GENERIC_CLOCKEVENTS
>         select PCI
> +       select PINCTRL
> +       select PINCTRL_KIRKWOOD
>         select PLAT_ORION_LEGACY
>         help
>           Support for the following Marvell Kirkwood series SoCs:

See "[PATCH 1/5] ARM: Kirkwood: Allow use of pinctrl" and "[PATCH 4/5]
ARM: Dove: Make use of pinctrl driver" which have been posted by Andrew
Lunn on October, 24th.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Linus Walleij]

On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:

> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index 238060e..40bd1b3 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
>
>         if (platid)
>                 version = platid->driver_data;
> -       else if (np)
> -               version = (unsigned int)
> -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> +       else if (np) {
> +               const struct of_device_id *match;
> +
> +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> +               if (!match)
> +                       return -ENODEV;
> +               version = (unsigned int) match->data;
> +       }

AFAICT this can actually happen so patch applied, unless Lee
speaks against it.

Patches 1-3/4 are dropped though, as the maintainers didn't seem
to like them.

Yours,
Linus Walleij

[PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Linus Walleij]

On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:

> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index 238060e..40bd1b3 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
>
>         if (platid)
>                 version = platid->driver_data;
> -       else if (np)
> -               version = (unsigned int)
> -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> +       else if (np) {
> +               const struct of_device_id *match;
> +
> +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> +               if (!match)
> +                       return -ENODEV;
> +               version = (unsigned int) match->data;
> +       }

AFAICT this can actually happen so patch applied, unless Lee
speaks against it.

Patches 1-3/4 are dropped though, as the maintainers didn't seem
to like them.

Yours,
Linus Walleij

Re: [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Lee Jones]

On Thu, 15 Nov 2012, Linus Walleij wrote:

> On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:
> 
> > of_match_device() may return NULL.
> >
> > Signed-off-by: Axel Lin <axel.lin@ingics.com>
> > ---
> >  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> > index 238060e..40bd1b3 100644
> > --- a/drivers/pinctrl/pinctrl-nomadik.c
> > +++ b/drivers/pinctrl/pinctrl-nomadik.c
> > @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
> >
> >         if (platid)
> >                 version = platid->driver_data;
> > -       else if (np)
> > -               version = (unsigned int)
> > -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> > +       else if (np) {
> > +               const struct of_device_id *match;
> > +
> > +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> > +               if (!match)
> > +                       return -ENODEV;
> > +               version = (unsigned int) match->data;
> > +       }
> 
> AFAICT this can actually happen so patch applied, unless Lee
> speaks against it.

At the very least it make the code easy to read.

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

[Lee Jones]

On Thu, 15 Nov 2012, Linus Walleij wrote:

> On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:
> 
> > of_match_device() may return NULL.
> >
> > Signed-off-by: Axel Lin <axel.lin@ingics.com>
> > ---
> >  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> > index 238060e..40bd1b3 100644
> > --- a/drivers/pinctrl/pinctrl-nomadik.c
> > +++ b/drivers/pinctrl/pinctrl-nomadik.c
> > @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
> >
> >         if (platid)
> >                 version = platid->driver_data;
> > -       else if (np)
> > -               version = (unsigned int)
> > -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> > +       else if (np) {
> > +               const struct of_device_id *match;
> > +
> > +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> > +               if (!match)
> > +                       return -ENODEV;
> > +               version = (unsigned int) match->data;
> > +       }
> 
> AFAICT this can actually happen so patch applied, unless Lee
> speaks against it.

At the very least it make the code easy to read.

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 12:52:39 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Why? We already discussed together that this situation could not
happen: if you are in the probe() function, then there *must* be a
match, because the probe() function precisely gets called when there is
a match.

So unless you give more details about what the problem is, I don't
think this patch and the similar one you sent for Dove are useful.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

[Thomas Petazzoni]

Dear Axel Lin,

On Thu, 15 Nov 2012 12:52:39 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Why? We already discussed together that this situation could not
happen: if you are in the probe() function, then there *must* be a
match, because the probe() function precisely gets called when there is
a match.

So unless you give more details about what the problem is, I don't
think this patch and the similar one you sent for Dove are useful.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Nicolas Ferre]

On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> On 12:58 Thu 15 Nov     , Axel Lin wrote:
>> of_match_device() may return NULL.
> this is not possible on at91
> 
> and I do a oups here as if we have a NULL pointer which means the driver is
> wrong

Well, okay, but it does not prevent from adding a supplementary check to
mimic every other pinctrl driver and use a common return path to the
of_match_device() function.

So I am not completely against this patch (that I why I added my Acked-by).


>> Signed-off-by: Axel Lin <axel.lin@ingics.com>
>> ---
>> This resend CC Jean-Christophe.
>>
>>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index 8490a55..32006c8 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>>  	uint32_t *tmp;
>>  	struct device_node *np = pdev->dev.of_node;
>>  	struct device_node *child;
>> +	const struct of_device_id *match;
>>  
>>  	if (!np)
>>  		return -ENODEV;
>>  
>> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> +	if (!match)
>> +		return -ENODEV;
>> +
>>  	info->dev = &pdev->dev;
>> -	info->ops = (struct at91_pinctrl_mux_ops*)
>> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
>> +
>>  	at91_pinctrl_child_count(info, np);
>>  
>>  	if (info->nbanks < 1) {
>> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>>  {
>>  	struct device_node *np = pdev->dev.of_node;
>> +	const struct of_device_id *match;
>>  	struct resource *res;
>>  	struct at91_gpio_chip *at91_chip = NULL;
>>  	struct gpio_chip *chip;
>> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>  
>> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
>> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
>> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
>> +	if (!match)
>> +		return -ENODEV;
>> +
>> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>>  	at91_chip->pioc_virq = irq;
>>  	at91_chip->pioc_idx = alias_idx;
>>  
>> -- 
>> 1.7.9.5
>>
>>
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 


-- 
Nicolas Ferre

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Nicolas Ferre]

On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> On 12:58 Thu 15 Nov     , Axel Lin wrote:
>> of_match_device() may return NULL.
> this is not possible on at91
> 
> and I do a oups here as if we have a NULL pointer which means the driver is
> wrong

Well, okay, but it does not prevent from adding a supplementary check to
mimic every other pinctrl driver and use a common return path to the
of_match_device() function.

So I am not completely against this patch (that I why I added my Acked-by).


>> Signed-off-by: Axel Lin <axel.lin@ingics.com>
>> ---
>> This resend CC Jean-Christophe.
>>
>>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index 8490a55..32006c8 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>>  	uint32_t *tmp;
>>  	struct device_node *np = pdev->dev.of_node;
>>  	struct device_node *child;
>> +	const struct of_device_id *match;
>>  
>>  	if (!np)
>>  		return -ENODEV;
>>  
>> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> +	if (!match)
>> +		return -ENODEV;
>> +
>>  	info->dev = &pdev->dev;
>> -	info->ops = (struct at91_pinctrl_mux_ops*)
>> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
>> +
>>  	at91_pinctrl_child_count(info, np);
>>  
>>  	if (info->nbanks < 1) {
>> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
>>  {
>>  	struct device_node *np = pdev->dev.of_node;
>> +	const struct of_device_id *match;
>>  	struct resource *res;
>>  	struct at91_gpio_chip *at91_chip = NULL;
>>  	struct gpio_chip *chip;
>> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>  
>> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
>> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
>> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
>> +	if (!match)
>> +		return -ENODEV;
>> +
>> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>>  	at91_chip->pioc_virq = irq;
>>  	at91_chip->pioc_idx = alias_idx;
>>  
>> -- 
>> 1.7.9.5
>>
>>
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 


-- 
Nicolas Ferre

Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:36 Fri 16 Nov     , Nicolas Ferre wrote:
> On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> > On 12:58 Thu 15 Nov     , Axel Lin wrote:
> >> of_match_device() may return NULL.
> > this is not possible on at91
> > 
> > and I do a oups here as if we have a NULL pointer which means the driver is
> > wrong
> 
> Well, okay, but it does not prevent from adding a supplementary check to
> mimic every other pinctrl driver and use a common return path to the
> of_match_device() function.
> 
> So I am not completely against this patch (that I why I added my Acked-by).
here it's a silent warning a oops it clear we have a bug

Best Regards,
J.
> 
> 
> >> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> >> ---
> >> This resend CC Jean-Christophe.
> >>
> >>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
> >>  1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >> index 8490a55..32006c8 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> >>  	uint32_t *tmp;
> >>  	struct device_node *np = pdev->dev.of_node;
> >>  	struct device_node *child;
> >> +	const struct of_device_id *match;
> >>  
> >>  	if (!np)
> >>  		return -ENODEV;
> >>  
> >> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> +	if (!match)
> >> +		return -ENODEV;
> >> +
> >>  	info->dev = &pdev->dev;
> >> -	info->ops = (struct at91_pinctrl_mux_ops*)
> >> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >> +
> >>  	at91_pinctrl_child_count(info, np);
> >>  
> >>  	if (info->nbanks < 1) {
> >> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> >>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device_node *np = pdev->dev.of_node;
> >> +	const struct of_device_id *match;
> >>  	struct resource *res;
> >>  	struct at91_gpio_chip *at91_chip = NULL;
> >>  	struct gpio_chip *chip;
> >> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >>  		goto err;
> >>  	}
> >>  
> >> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> >> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> >> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> >> +	if (!match)
> >> +		return -ENODEV;
> >> +
> >> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >>  	at91_chip->pioc_virq = irq;
> >>  	at91_chip->pioc_idx = alias_idx;
> >>  
> >> -- 
> >> 1.7.9.5
> >>
> >>
> >>
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> > 
> 
> 
> -- 
> Nicolas Ferre

[PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:36 Fri 16 Nov     , Nicolas Ferre wrote:
> On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> > On 12:58 Thu 15 Nov     , Axel Lin wrote:
> >> of_match_device() may return NULL.
> > this is not possible on at91
> > 
> > and I do a oups here as if we have a NULL pointer which means the driver is
> > wrong
> 
> Well, okay, but it does not prevent from adding a supplementary check to
> mimic every other pinctrl driver and use a common return path to the
> of_match_device() function.
> 
> So I am not completely against this patch (that I why I added my Acked-by).
here it's a silent warning a oops it clear we have a bug

Best Regards,
J.
> 
> 
> >> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> >> ---
> >> This resend CC Jean-Christophe.
> >>
> >>  drivers/pinctrl/pinctrl-at91.c |   17 +++++++++++++----
> >>  1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >> index 8490a55..32006c8 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> >>  	uint32_t *tmp;
> >>  	struct device_node *np = pdev->dev.of_node;
> >>  	struct device_node *child;
> >> +	const struct of_device_id *match;
> >>  
> >>  	if (!np)
> >>  		return -ENODEV;
> >>  
> >> +	match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> +	if (!match)
> >> +		return -ENODEV;
> >> +
> >>  	info->dev = &pdev->dev;
> >> -	info->ops = (struct at91_pinctrl_mux_ops*)
> >> -		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> +	info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >> +
> >>  	at91_pinctrl_child_count(info, np);
> >>  
> >>  	if (info->nbanks < 1) {
> >> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> >>  static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device_node *np = pdev->dev.of_node;
> >> +	const struct of_device_id *match;
> >>  	struct resource *res;
> >>  	struct at91_gpio_chip *at91_chip = NULL;
> >>  	struct gpio_chip *chip;
> >> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >>  		goto err;
> >>  	}
> >>  
> >> -	at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> >> -		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> >> +	match = of_match_device(at91_gpio_of_match, &pdev->dev);
> >> +	if (!match)
> >> +		return -ENODEV;
> >> +
> >> +	at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >>  	at91_chip->pioc_virq = irq;
> >>  	at91_chip->pioc_idx = alias_idx;
> >>  
> >> -- 
> >> 1.7.9.5
> >>
> >>
> >>
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> > 
> 
> 
> -- 
> Nicolas Ferre