LinuxLists.cc - [PATCH 01/16] regulator: max77686: Use of_get_child_by

[permalink] [raw]

Subject: [PATCH 05/16] regulator: max8925: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/max8925-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/max8925-regulator.c b/drivers/regulator/max8925-regulator.c
index 759510789e71..5b939894bfc4 100644
--- a/drivers/regulator/max8925-regulator.c
+++ b/drivers/regulator/max8925-regulator.c
@@ -254,7 +254,7 @@ static int max8925_regulator_dt_init(struct platform_device *pdev,
nproot = of_node_get(pdev->dev.parent->of_node);
if (!nproot)
return -ENODEV;
- np = of_find_node_by_name(nproot, "regulators");
+ np = of_get_child_by_name(nproot, "regulators");
if (!np) {
dev_err(&pdev->dev, "failed to find regulators node\n");
return -ENODEV;
--
1.7.9.5

2014-02-14 11:55:56

[permalink] [raw]

Subject: [PATCH 06/16] regulator: max8997: Use of_get_child_by_name

2014-02-14 11:55:59

[permalink] [raw]

Subject: [PATCH 07/16] regulator: 88pm8607: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/88pm8607.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index f704d83c93c4..fa99bfccaa6b 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -322,7 +322,7 @@ static int pm8607_regulator_dt_init(struct platform_device *pdev,
nproot = of_node_get(pdev->dev.parent->of_node);
if (!nproot)
return -ENODEV;
- nproot = of_find_node_by_name(nproot, "regulators");
+ nproot = of_get_child_by_name(nproot, "regulators");
if (!nproot) {
dev_err(&pdev->dev, "failed to find regulators node\n");
return -ENODEV;
--
1.7.9.5

2014-02-14 11:56:11

[permalink] [raw]

Subject: [PATCH 11/16] regulator: da9055: Use of_get_child_by_name

2014-02-14 11:56:05

[permalink] [raw]

Subject: [PATCH 09/16] regulator: as3711: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/as3711-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/as3711-regulator.c b/drivers/regulator/as3711-regulator.c
index 67fd548dcdba..856c55f3a832 100644
--- a/drivers/regulator/as3711-regulator.c
+++ b/drivers/regulator/as3711-regulator.c
@@ -191,7 +191,7 @@ static int as3711_regulator_parse_dt(struct device *dev,
{
struct as3711_regulator_pdata *pdata = dev_get_platdata(dev);
struct device_node *regulators =
- of_find_node_by_name(dev->parent->of_node, "regulators");
+ of_get_child_by_name(dev->parent->of_node, "regulators");
struct of_regulator_match *match;
int ret, i;

--
1.7.9.5

2014-02-14 11:56:17

[permalink] [raw]

Subject: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/mc13xxx-regulator-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
index da4859282302..4498a3f0733d 100644
--- a/drivers/regulator/mc13xxx-regulator-core.c
+++ b/drivers/regulator/mc13xxx-regulator-core.c
@@ -168,7 +168,7 @@ int mc13xxx_get_num_regulators_dt(struct platform_device *pdev)
int num;

of_node_get(pdev->dev.parent->of_node);
- parent = of_find_node_by_name(pdev->dev.parent->of_node, "regulators");
+ parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
if (!parent)
return -ENODEV;

@@ -188,7 +188,7 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
int i, parsed = 0;

of_node_get(pdev->dev.parent->of_node);
- parent = of_find_node_by_name(pdev->dev.parent->of_node, "regulators");
+ parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
if (!parent)
return NULL;

--
1.7.9.5

2014-02-14 11:56:26

[permalink] [raw]

Subject: [PATCH 16/16] regulator: tps65217: Use of_get_child_by_name

2014-02-14 11:56:21

[permalink] [raw]

Subject: [PATCH 14/16] regulator: pfuze100: Use of_get_child_by_name

2014-02-14 11:56:54

[permalink] [raw]

Subject: [PATCH 15/16] regulator: tps6507x: Use of_get_child_by_name

2014-02-14 11:56:13

[permalink] [raw]

Subject: [PATCH 12/16] regulator: da9063: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/da9063-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index 91e99a2c8dc1..bfd7f2f795a6 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -666,7 +666,7 @@ static struct da9063_regulators_pdata *da9063_parse_regulators_dt(
struct device_node *node;
int i, n, num;

- node = of_find_node_by_name(pdev->dev.parent->of_node, "regulators");
+ node = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
if (!node) {
dev_err(&pdev->dev, "Regulators device node not found\n");
return ERR_PTR(-ENODEV);
--
1.7.9.5

2014-02-14 11:57:52

[permalink] [raw]

Subject: [PATCH 10/16] regulator: da9052: Use of_get_child_by_name

2014-02-14 11:58:20

[permalink] [raw]

Subject: [PATCH 08/16] regulator: act8865: Use of_get_child_by_name

of_find_node_by_name walks the allnodes list, and can thus walk
outside of the parent node. Use of_get_child_by_name instead.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/act8865-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
index 084cc0819a52..c9f98b061307 100644
--- a/drivers/regulator/act8865-regulator.c
+++ b/drivers/regulator/act8865-regulator.c
@@ -213,7 +213,7 @@ static int act8865_pdata_from_dt(struct device *dev,
struct device_node *np;
struct act8865_regulator_data *regulator;

- np = of_find_node_by_name(dev->of_node, "regulators");
+ np = of_get_child_by_name(dev->of_node, "regulators");
if (!np) {
dev_err(dev, "missing 'regulators' subnode in DT\n");
return -EINVAL;
--
1.7.9.5

2014-02-14 11:55:50

[permalink] [raw]

Subject: [PATCH 04/16] regulator: max8907: Use of_get_child_by_name

2014-02-14 11:55:48

[permalink] [raw]

Subject: [PATCH 03/16] regulator: max8660: Use of_get_child_by_name

2014-02-14 11:55:45

[permalink] [raw]

Subject: [PATCH 02/16] regulator: max77693: Use of_get_child_by_name

2014-02-15 10:07:30

[permalink] [raw]

Subject: Re: [PATCH 02/16] regulator: max77693: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:48PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:07:39

[permalink] [raw]

Subject: Re: [PATCH 12/16] regulator: da9063: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:58PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:07:53

[permalink] [raw]

Subject: Re: [PATCH 11/16] regulator: da9055: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:57PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:08:04

[permalink] [raw]

Subject: Re: [PATCH 03/16] regulator: max8660: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:49PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:08:07

[permalink] [raw]

Subject: Re: [PATCH 08/16] regulator: act8865: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:54PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:08:13

[permalink] [raw]

Subject: Re: [PATCH 07/16] regulator: 88pm8607: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:53PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:08:27

[permalink] [raw]

Subject: Re: [PATCH 04/16] regulator: max8907: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:50PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:08:34

[permalink] [raw]

Subject: Re: [PATCH 10/16] regulator: da9052: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:56PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:09:00

[permalink] [raw]

Subject: Re: [PATCH 09/16] regulator: as3711: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:55PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:09:20

[permalink] [raw]

Subject: Re: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:59PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:09:31

[permalink] [raw]

Subject: Re: [PATCH 15/16] regulator: tps6507x: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:20:01PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:09:36

[permalink] [raw]

Subject: Re: [PATCH 05/16] regulator: max8925: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:51PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:10:12

[permalink] [raw]

Subject: Re: [PATCH 06/16] regulator: max8997: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:52PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:10:56

[permalink] [raw]

Subject: Re: [PATCH 16/16] regulator: tps65217: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:20:02PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:11:40

[permalink] [raw]

Subject: Re: [PATCH 14/16] regulator: pfuze100: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:20:00PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks.

Attachments:

(No filename) (210.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-15 10:13:59

[permalink] [raw]

Subject: Re: [PATCH 01/16] regulator: max77686: Use of_get_child_by_name

On Fri, Feb 14, 2014 at 05:19:47PM +0530, Sachin Kamat wrote:
> of_find_node_by_name walks the allnodes list, and can thus walk
> outside of the parent node. Use of_get_child_by_name instead.

Applied, thanks - and thanks for doing this cleanup series.

Attachments:

(No filename) (253.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2014-02-24 13:51:10

[permalink] [raw]

Subject: Re: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

Hi

This patch breaks DT-Enabled kernel run on non-DT board:
[ 0.822977] Unable to handle kernel NULL pointer dereference at
virtual address 0000001c
(...)
[ 0.880320] [<c02f3e94>] (of_get_next_child) from [<c02f5420>]
(of_get_child_by_name+0x38/0x50)
[ 0.881449] [<c02f5420>] (of_get_child_by_name) from [<c01f6258>]
(mc13xxx_get_num_regulators_dt+0x18/0x64)
[ 0.882707] [<c01f6258>] (mc13xxx_get_num_regulators_dt) from
[<c01f5d38>] (mc13783_regulator_probe+0x34/0x17c)
[ 0.884011] [<c01f5d38>] (mc13783_regulator_probe) from [<c021fed0>]
(platform_drv_probe+0x20/0x54)
[ 0.885182] [<c021fed0>] (platform_drv_probe) from [<c021e674>]
(driver_probe_device+0x144/0x360)
(...)

Because mc13783-regulator do in its probe:
static int mc13783_regulator_probe(struct platform_device *pdev)
{
(...)
int i, num_regulators;

num_regulators = mc13xxx_get_num_regulators_dt(pdev);
if (num_regulators <= 0 && pdata)
num_regulators = pdata->num_regulators

and mc13xxx_get_num_regulators_dt() do, before your patch:

parent = of_find_node_by_name(pdev->dev.parent->of_node, "regulators");
if (!parent)
return -ENODEV;

of_find_node_by_name will return NULL if the node passed is NULL and the
DT is non-existant.

But, with your change we use this:
parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");

Which will OOPS as it does not expect to have a NULL node passed as
argument.

So please revert this patch.

Regards,

Philippe

2014-02-24 14:51:15

[permalink] [raw]

Subject: Re: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

On 24 February 2014 19:14, Philippe R?tornaz <[email protected]> wrote:
> Hi
>
> This patch breaks DT-Enabled kernel run on non-DT board:
> [ 0.822977] Unable to handle kernel NULL pointer dereference at virtual
> address 0000001c
> (...)
> [ 0.880320] [<c02f3e94>] (of_get_next_child) from [<c02f5420>]
> (of_get_child_by_name+0x38/0x50)
> [ 0.881449] [<c02f5420>] (of_get_child_by_name) from [<c01f6258>]
> (mc13xxx_get_num_regulators_dt+0x18/0x64)
> [ 0.882707] [<c01f6258>] (mc13xxx_get_num_regulators_dt) from
> [<c01f5d38>] (mc13783_regulator_probe+0x34/0x17c)
> [ 0.884011] [<c01f5d38>] (mc13783_regulator_probe) from [<c021fed0>]
> (platform_drv_probe+0x20/0x54)
> [ 0.885182] [<c021fed0>] (platform_drv_probe) from [<c021e674>]
> (driver_probe_device+0x144/0x360)
> (...)
>
> Because mc13783-regulator do in its probe:
> static int mc13783_regulator_probe(struct platform_device *pdev)
> {
> (...)
> int i, num_regulators;
>
> num_regulators = mc13xxx_get_num_regulators_dt(pdev);
> if (num_regulators <= 0 && pdata)
> num_regulators = pdata->num_regulators
>
>
> and mc13xxx_get_num_regulators_dt() do, before your patch:
>
> parent = of_find_node_by_name(pdev->dev.parent->of_node, "regulators");
> if (!parent)
> return -ENODEV;
>
> of_find_node_by_name will return NULL if the node passed is NULL and the
> DT is non-existant.
>
> But, with your change we use this:
> parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
>
> Which will OOPS as it does not expect to have a NULL node passed as
> argument.
>
> So please revert this patch.

Sorry for the issue caused due to this patch. I would prefer to try to
have this fixed before reverting.
Can you please try with below change and let me know if it works:

parent = of_get_child_by_name(pdev->dev->of_node, "regulators");

Also, how about adding NULL check to see it the node exists at all?

--
With warm regards,
Sachin

2014-02-24 15:07:21

[permalink] [raw]

Subject: Re: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

Le 24/02/2014 15:51, Sachin Kamat a ?crit :
> On 24 February 2014 19:14, Philippe R?tornaz
> <[email protected]> wrote:
>> Hi
>>
>> This patch breaks DT-Enabled kernel run on non-DT board: [
>> 0.822977] Unable to handle kernel NULL pointer dereference at
>> virtual address 0000001c (...) [ 0.880320] [<c02f3e94>]
>> (of_get_next_child) from [<c02f5420>]
>> (of_get_child_by_name+0x38/0x50) [ 0.881449] [<c02f5420>]
>> (of_get_child_by_name) from [<c01f6258>]
>> (mc13xxx_get_num_regulators_dt+0x18/0x64) [ 0.882707]
>> [<c01f6258>] (mc13xxx_get_num_regulators_dt) from [<c01f5d38>]
>> (mc13783_regulator_probe+0x34/0x17c) [ 0.884011] [<c01f5d38>]
>> (mc13783_regulator_probe) from [<c021fed0>]
>> (platform_drv_probe+0x20/0x54) [ 0.885182] [<c021fed0>]
>> (platform_drv_probe) from [<c021e674>]
>> (driver_probe_device+0x144/0x360) (...)
>>
>> Because mc13783-regulator do in its probe: static int
>> mc13783_regulator_probe(struct platform_device *pdev) { (...) int
>> i, num_regulators;
>>
>> num_regulators = mc13xxx_get_num_regulators_dt(pdev); if
>> (num_regulators <= 0 && pdata) num_regulators =
>> pdata->num_regulators
>>
>>
>> and mc13xxx_get_num_regulators_dt() do, before your patch:
>>
>> parent = of_find_node_by_name(pdev->dev.parent->of_node,
>> "regulators"); if (!parent) return -ENODEV;
>>
>> of_find_node_by_name will return NULL if the node passed is NULL
>> and the DT is non-existant.
>>
>> But, with your change we use this: parent =
>> of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
>>
>> Which will OOPS as it does not expect to have a NULL node passed as
>> argument.
>>
>> So please revert this patch.
>
> Sorry for the issue caused due to this patch. I would prefer to try
> to have this fixed before reverting. Can you please try with below
> change and let me know if it works:
>
> parent = of_get_child_by_name(pdev->dev->of_node, "regulators");
>

You meant pdev->dev.of_node here I guess ?
This still OOPS as the board is not booting with DT, so all of_node are
NULL.

> Also, how about adding NULL check to see it the node exists at all?
>

Could be an option. BTW, mc13xxx_parse_regulators_dt() has the same problem.

Regards,

Philippe

2014-02-24 15:10:29

[permalink] [raw]

Subject: Re: [PATCH 13/16] regulator: mc13xxx: Use of_get_child_by_name

On 24 February 2014 20:37, Philippe R?tornaz <[email protected]> wrote:
> Le 24/02/2014 15:51, Sachin Kamat a ?crit :
>
>> On 24 February 2014 19:14, Philippe R?tornaz
>> <[email protected]> wrote:
>>>
>>> Hi
>>>
>>> This patch breaks DT-Enabled kernel run on non-DT board: [
>>> 0.822977] Unable to handle kernel NULL pointer dereference at
>>> virtual address 0000001c (...) [ 0.880320] [<c02f3e94>]
>>> (of_get_next_child) from [<c02f5420>]
>>> (of_get_child_by_name+0x38/0x50) [ 0.881449] [<c02f5420>]
>>> (of_get_child_by_name) from [<c01f6258>]
>>> (mc13xxx_get_num_regulators_dt+0x18/0x64) [ 0.882707]
>>> [<c01f6258>] (mc13xxx_get_num_regulators_dt) from [<c01f5d38>]
>>> (mc13783_regulator_probe+0x34/0x17c) [ 0.884011] [<c01f5d38>]
>>> (mc13783_regulator_probe) from [<c021fed0>]
>>> (platform_drv_probe+0x20/0x54) [ 0.885182] [<c021fed0>]
>>> (platform_drv_probe) from [<c021e674>]
>>> (driver_probe_device+0x144/0x360) (...)
>>>
>>> Because mc13783-regulator do in its probe: static int
>>> mc13783_regulator_probe(struct platform_device *pdev) { (...) int
>>> i, num_regulators;
>>>
>>> num_regulators = mc13xxx_get_num_regulators_dt(pdev); if
>>> (num_regulators <= 0 && pdata) num_regulators =
>>> pdata->num_regulators
>>>
>>>
>>> and mc13xxx_get_num_regulators_dt() do, before your patch:
>>>
>>> parent = of_find_node_by_name(pdev->dev.parent->of_node,
>>> "regulators"); if (!parent) return -ENODEV;
>>>
>>> of_find_node_by_name will return NULL if the node passed is NULL
>>> and the DT is non-existant.
>>>
>>> But, with your change we use this: parent =
>>> of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
>>>
>>> Which will OOPS as it does not expect to have a NULL node passed as
>>> argument.
>>>
>>> So please revert this patch.
>>
>>
>> Sorry for the issue caused due to this patch. I would prefer to try
>> to have this fixed before reverting. Can you please try with below
>> change and let me know if it works:
>>
>> parent = of_get_child_by_name(pdev->dev->of_node, "regulators");
>>
>
> You meant pdev->dev.of_node here I guess ?

Yes right. Sorry for the typo.

> This still OOPS as the board is not booting with DT, so all of_node are
> NULL.

So this is non-DT mode. Please see further comments below.

>
>> Also, how about adding NULL check to see it the node exists at all?
>>
>
> Could be an option. BTW, mc13xxx_parse_regulators_dt() has the same problem.

I would suggest adding a check at the initial stage itself to see if
it is non-DT and then either
error out or skip appropriately.

--
With warm regards,
Sachin

2014-02-25 08:48:30

[permalink] [raw]

Subject: [PATCH 1/1] regulator: mc13xxx: check if DT is enabled

This fix a regression on non-DT board booted with a DT enabled kernel

Signed-off-by: Philippe Rétornaz <[email protected]>
---
drivers/regulator/mc13xxx-regulator-core.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
index 4498a3f..a10c999 100644
--- a/drivers/regulator/mc13xxx-regulator-core.c
+++ b/drivers/regulator/mc13xxx-regulator-core.c
@@ -167,6 +167,9 @@ int mc13xxx_get_num_regulators_dt(struct platform_device *pdev)
struct device_node *parent;
int num;

+ if(!pdev->dev.parent->of_node)
+ return -ENODEV;
+
of_node_get(pdev->dev.parent->of_node);
parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
if (!parent)
@@ -187,6 +190,9 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
struct device_node *parent, *child;
int i, parsed = 0;

+ if(!pdev->dev.parent->of_node)
+ return NULL;
+
of_node_get(pdev->dev.parent->of_node);
parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
if (!parent)
--
1.7.9.5

2014-02-25 09:13:06

[permalink] [raw]

Subject: Re: [PATCH 1/1] regulator: mc13xxx: check if DT is enabled

On 25 February 2014 14:17, Philippe R?tornaz <[email protected]> wrote:
> This fix a regression on non-DT board booted with a DT enabled kernel
>
> Signed-off-by: Philippe R?tornaz <[email protected]>
> ---
> drivers/regulator/mc13xxx-regulator-core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
> index 4498a3f..a10c999 100644
> --- a/drivers/regulator/mc13xxx-regulator-core.c
> +++ b/drivers/regulator/mc13xxx-regulator-core.c
> @@ -167,6 +167,9 @@ int mc13xxx_get_num_regulators_dt(struct platform_device *pdev)
> struct device_node *parent;
> int num;
>
> + if(!pdev->dev.parent->of_node)
> + return -ENODEV;
> +
> of_node_get(pdev->dev.parent->of_node);
> parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
> if (!parent)
> @@ -187,6 +190,9 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
> struct device_node *parent, *child;
> int i, parsed = 0;
>
> + if(!pdev->dev.parent->of_node)
> + return NULL;
> +
> of_node_get(pdev->dev.parent->of_node);
> parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
> if (!parent)
> --
> 1.7.9.5
>

Thanks for the patch. I was about to send a similar patch which also
does a little more cleanup.
If you could test it in DT and non-DT modes that would be great. I
attach the diff below. You can
even fold it into your patch if you wish.

------------------
diff --git a/drivers/regulator/mc13xxx-regulator-core.c
b/drivers/regulator/mc13xxx-regulator-core.c
index 4498a3f0733d..bf75fcabfa3c 100644
--- a/drivers/regulator/mc13xxx-regulator-core.c
+++ b/drivers/regulator/mc13xxx-regulator-core.c
@@ -167,8 +167,10 @@ int mc13xxx_get_num_regulators_dt(struct
platform_device *pdev)
struct device_node *parent;
int num;

- of_node_get(pdev->dev.parent->of_node);
- parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
+ if (!pdev->dev.of_node)
+ return -ENODEV;
+
+ parent = of_get_child_by_name(pdev->dev.of_node, "regulators");
if (!parent)
return -ENODEV;

@@ -187,8 +189,10 @@ struct mc13xxx_regulator_init_data
*mc13xxx_parse_regulators_dt(
struct device_node *parent, *child;
int i, parsed = 0;

- of_node_get(pdev->dev.parent->of_node);
- parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
+ if (!pdev->dev.of_node)
+ return NULL;
+
+ parent = of_get_child_by_name(pdev->dev.of_node, "regulators");
if (!parent)
return NULL;

--
With warm regards,
Sachin

2014-02-25 10:09:28

[permalink] [raw]

Subject: Re: [PATCH 1/1] regulator: mc13xxx: check if DT is enabled

Le 25/02/2014 10:12, Sachin Kamat a ?crit :
> On 25 February 2014 14:17, Philippe R?tornaz <[email protected]> wrote:
>> This fix a regression on non-DT board booted with a DT enabled kernel
>>
>> Signed-off-by: Philippe R?tornaz <[email protected]>
>> ---
>> drivers/regulator/mc13xxx-regulator-core.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
>> index 4498a3f..a10c999 100644
>> --- a/drivers/regulator/mc13xxx-regulator-core.c
>> +++ b/drivers/regulator/mc13xxx-regulator-core.c
>> @@ -167,6 +167,9 @@ int mc13xxx_get_num_regulators_dt(struct platform_device *pdev)
>> struct device_node *parent;
>> int num;
>>
>> + if(!pdev->dev.parent->of_node)
>> + return -ENODEV;
>> +
>> of_node_get(pdev->dev.parent->of_node);
>> parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
>> if (!parent)
>> @@ -187,6 +190,9 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
>> struct device_node *parent, *child;
>> int i, parsed = 0;
>>
>> + if(!pdev->dev.parent->of_node)
>> + return NULL;
>> +
>> of_node_get(pdev->dev.parent->of_node);
>> parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
>> if (!parent)
>> --
>> 1.7.9.5
>>
>
> Thanks for the patch. I was about to send a similar patch which also
> does a little more cleanup.
> If you could test it in DT and non-DT modes that would be great. I
> attach the diff below. You can
> even fold it into your patch if you wish.

It does fix the regression on the non-DT board.
I don't have any DT-enabled board with mc13xxx PMIC so I cannot test the
DT case. I will let you test the DT case & submit the final patch.

Regards,

Philippe

>
> ------------------
> diff --git a/drivers/regulator/mc13xxx-regulator-core.c
> b/drivers/regulator/mc13xxx-regulator-core.c
> index 4498a3f0733d..bf75fcabfa3c 100644
> --- a/drivers/regulator/mc13xxx-regulator-core.c
> +++ b/drivers/regulator/mc13xxx-regulator-core.c
> @@ -167,8 +167,10 @@ int mc13xxx_get_num_regulators_dt(struct
> platform_device *pdev)
> struct device_node *parent;
> int num;
>
> - of_node_get(pdev->dev.parent->of_node);
> - parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + parent = of_get_child_by_name(pdev->dev.of_node, "regulators");
> if (!parent)
> return -ENODEV;
>
> @@ -187,8 +189,10 @@ struct mc13xxx_regulator_init_data
> *mc13xxx_parse_regulators_dt(
> struct device_node *parent, *child;
> int i, parsed = 0;
>
> - of_node_get(pdev->dev.parent->of_node);
> - parent = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
> + if (!pdev->dev.of_node)
> + return NULL;
> +
> + parent = of_get_child_by_name(pdev->dev.of_node, "regulators");
> if (!parent)
> return NULL;
>
>

2014-02-25 10:12:10

[permalink] [raw]

Subject: Re: [PATCH 1/1] regulator: mc13xxx: check if DT is enabled

On 25 February 2014 15:39, Philippe R?tornaz <[email protected]> wrote:
> Le 25/02/2014 10:12, Sachin Kamat a ?crit :
>
>> On 25 February 2014 14:17, Philippe R?tornaz <[email protected]>
>> wrote:
>>
>> Thanks for the patch. I was about to send a similar patch which also
>> does a little more cleanup.
>> If you could test it in DT and non-DT modes that would be great. I
>> attach the diff below. You can
>> even fold it into your patch if you wish.
>
>
> It does fix the regression on the non-DT board.
> I don't have any DT-enabled board with mc13xxx PMIC so I cannot test the
> DT case. I will let you test the DT case & submit the final patch.

Thanks for the confirmation. I will post this patch.

--
With warm regards,
Sachin

2014-02-25 11:58:51