[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

Add message of MBIGEN domain created, it's useful for check
which MBIGEN domain is created.

Meanwhile, drop module owner, it will be set by driver core.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index d67baa2..a4dc7a0 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
+	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
+
 	for_each_child_of_node(pdev->dev.of_node, np) {
 		if (!of_property_read_bool(np, "interrupt-controller"))
 			continue;
 
 		parent = platform_bus_type.dev_root;
 		child = of_platform_device_create(np, NULL, parent);
-		if (IS_ERR(child))
+		if (IS_ERR(child)) {
+			dev_err(&pdev->dev, "failed to create for %s\n",
+				np->full_name);
 			return PTR_ERR(child);
+		}
 
 		if (of_property_read_u32(child->dev.of_node, "num-pins",
 					 &num_pins) < 0) {
@@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
 							   mbigen_write_msg,
 							   &mbigen_domain_ops,
 							   mgn_chip);
-		if (!domain)
+		if (!domain) {
+			dev_info(&pdev->dev, "unable to create %s domain\n",
+				 np->full_name);
 			return -ENOMEM;
+		}
+
+		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
 	}
 
 	platform_set_drvdata(pdev, mgn_chip);
@@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
 static struct platform_driver mbigen_platform_driver = {
 	.driver = {
 		.name		= "Hisilicon MBIGEN-V2",
-		.owner		= THIS_MODULE,
 		.of_match_table	= mbigen_of_match,
 	},
 	.probe			= mbigen_device_probe,
-- 
1.7.12.4

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

Add message of MBIGEN domain created, it's useful for check
which MBIGEN domain is created.

Meanwhile, drop module owner, it will be set by driver core.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index d67baa2..a4dc7a0 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
+	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
+
 	for_each_child_of_node(pdev->dev.of_node, np) {
 		if (!of_property_read_bool(np, "interrupt-controller"))
 			continue;
 
 		parent = platform_bus_type.dev_root;
 		child = of_platform_device_create(np, NULL, parent);
-		if (IS_ERR(child))
+		if (IS_ERR(child)) {
+			dev_err(&pdev->dev, "failed to create for %s\n",
+				np->full_name);
 			return PTR_ERR(child);
+		}
 
 		if (of_property_read_u32(child->dev.of_node, "num-pins",
 					 &num_pins) < 0) {
@@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
 							   mbigen_write_msg,
 							   &mbigen_domain_ops,
 							   mgn_chip);
-		if (!domain)
+		if (!domain) {
+			dev_info(&pdev->dev, "unable to create %s domain\n",
+				 np->full_name);
 			return -ENOMEM;
+		}
+
+		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
 	}
 
 	platform_set_drvdata(pdev, mgn_chip);
@@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
 static struct platform_driver mbigen_platform_driver = {
 	.driver = {
 		.name		= "Hisilicon MBIGEN-V2",
-		.owner		= THIS_MODULE,
 		.of_match_table	= mbigen_of_match,
 	},
 	.probe			= mbigen_device_probe,
-- 
1.7.12.4

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On Fri, 8 Apr 2016 15:16:02 +0800
Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

> Add message of MBIGEN domain created, it's useful for check
> which MBIGEN domain is created.
> 
> Meanwhile, drop module owner, it will be set by driver core.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index d67baa2..a4dc7a0 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  	if (IS_ERR(mgn_chip->base))
>  		return PTR_ERR(mgn_chip->base);
>  
> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
> +

How is that a useful information?

>  	for_each_child_of_node(pdev->dev.of_node, np) {
>  		if (!of_property_read_bool(np, "interrupt-controller"))
>  			continue;
>  
>  		parent = platform_bus_type.dev_root;
>  		child = of_platform_device_create(np, NULL, parent);
> -		if (IS_ERR(child))
> +		if (IS_ERR(child)) {
> +			dev_err(&pdev->dev, "failed to create for %s\n",

Failed to create what?

> +				np->full_name);
>  			return PTR_ERR(child);
> +		}
>  
>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
>  					 &num_pins) < 0) {
> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  							   mbigen_write_msg,
>  							   &mbigen_domain_ops,
>  							   mgn_chip);
> -		if (!domain)
> +		if (!domain) {
> +			dev_info(&pdev->dev, "unable to create %s domain\n",
> +				 np->full_name);

And what about failure to read num_pin? No need for a debug print in
this case?

>  			return -ENOMEM;
> +		}
> +
> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
>  	}
>  
>  	platform_set_drvdata(pdev, mgn_chip);
> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
>  static struct platform_driver mbigen_platform_driver = {
>  	.driver = {
>  		.name		= "Hisilicon MBIGEN-V2",
> -		.owner		= THIS_MODULE,
>  		.of_match_table	= mbigen_of_match,
>  	},
>  	.probe			= mbigen_device_probe,


Overall, this doesn't look like a critical patch to me. I think Ma Jun
is working on separate series reworking the way the mgigen is getting
probed, so I'd advise you to work with him in order to integrate this
patch in his series, as it would make a lot more sense.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On Fri, 8 Apr 2016 15:16:02 +0800
Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

> Add message of MBIGEN domain created, it's useful for check
> which MBIGEN domain is created.
> 
> Meanwhile, drop module owner, it will be set by driver core.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index d67baa2..a4dc7a0 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  	if (IS_ERR(mgn_chip->base))
>  		return PTR_ERR(mgn_chip->base);
>  
> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
> +

How is that a useful information?

>  	for_each_child_of_node(pdev->dev.of_node, np) {
>  		if (!of_property_read_bool(np, "interrupt-controller"))
>  			continue;
>  
>  		parent = platform_bus_type.dev_root;
>  		child = of_platform_device_create(np, NULL, parent);
> -		if (IS_ERR(child))
> +		if (IS_ERR(child)) {
> +			dev_err(&pdev->dev, "failed to create for %s\n",

Failed to create what?

> +				np->full_name);
>  			return PTR_ERR(child);
> +		}
>  
>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
>  					 &num_pins) < 0) {
> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  							   mbigen_write_msg,
>  							   &mbigen_domain_ops,
>  							   mgn_chip);
> -		if (!domain)
> +		if (!domain) {
> +			dev_info(&pdev->dev, "unable to create %s domain\n",
> +				 np->full_name);

And what about failure to read num_pin? No need for a debug print in
this case?

>  			return -ENOMEM;
> +		}
> +
> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
>  	}
>  
>  	platform_set_drvdata(pdev, mgn_chip);
> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
>  static struct platform_driver mbigen_platform_driver = {
>  	.driver = {
>  		.name		= "Hisilicon MBIGEN-V2",
> -		.owner		= THIS_MODULE,
>  		.of_match_table	= mbigen_of_match,
>  	},
>  	.probe			= mbigen_device_probe,


Overall, this doesn't look like a critical patch to me. I think Ma Jun
is working on separate series reworking the way the mgigen is getting
probed, so I'd advise you to work with him in order to integrate this
patch in his series, as it would make a lot more sense.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 16:09, Marc Zyngier wrote:
> On Fri, 8 Apr 2016 15:16:02 +0800
> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
>> Add message of MBIGEN domain created, it's useful for check
>> which MBIGEN domain is created.
>>
>> Meanwhile, drop module owner, it will be set by driver core.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>> index d67baa2..a4dc7a0 100644
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>  	if (IS_ERR(mgn_chip->base))
>>  		return PTR_ERR(mgn_chip->base);
>>  
>> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
>> +
> 
> How is that a useful information?
> 
>>  	for_each_child_of_node(pdev->dev.of_node, np) {
>>  		if (!of_property_read_bool(np, "interrupt-controller"))
>>  			continue;
>>  
>>  		parent = platform_bus_type.dev_root;
>>  		child = of_platform_device_create(np, NULL, parent);
>> -		if (IS_ERR(child))
>> +		if (IS_ERR(child)) {
>> +			dev_err(&pdev->dev, "failed to create for %s\n",
> 
> Failed to create what?
> 
>> +				np->full_name);
>>  			return PTR_ERR(child);
>> +		}
>>  
>>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
>>  					 &num_pins) < 0) {
>> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>  							   mbigen_write_msg,
>>  							   &mbigen_domain_ops,
>>  							   mgn_chip);
>> -		if (!domain)
>> +		if (!domain) {
>> +			dev_info(&pdev->dev, "unable to create %s domain\n",
>> +				 np->full_name);
> 
> And what about failure to read num_pin? No need for a debug print in
> this case?
> 
>>  			return -ENOMEM;
>> +		}
>> +
>> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
>>  	}
>>  
>>  	platform_set_drvdata(pdev, mgn_chip);
>> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
>>  static struct platform_driver mbigen_platform_driver = {
>>  	.driver = {
>>  		.name		= "Hisilicon MBIGEN-V2",
>> -		.owner		= THIS_MODULE,
>>  		.of_match_table	= mbigen_of_match,
>>  	},
>>  	.probe			= mbigen_device_probe,
> 
> 
> Overall, this doesn't look like a critical patch to me. I think Ma Jun
> is working on separate series reworking the way the mgigen is getting
> probed, so I'd advise you to work with him in order to integrate this
> patch in his series, as it would make a lot more sense.

When try to enable hip06 d03 board[1], we met following error log, so I add
some debug message. The mbigen driver use module_platform_driver, the driver
initialization is too late, and it is without any message,  we don't know
about any info of mbigen.  I think we should show something about the mbigen
domain creation at least. What's your option?

Is there a way to solve this improper print？
-----------
[    1.345945] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
[    1.353660] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !


[1] http://www.spinics.net/lists/arm-kernel/msg495732.html

Thanks
Kefeng


> 
> Thanks,
> 
> 	M.
> 

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 16:09, Marc Zyngier wrote:
> On Fri, 8 Apr 2016 15:16:02 +0800
> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
>> Add message of MBIGEN domain created, it's useful for check
>> which MBIGEN domain is created.
>>
>> Meanwhile, drop module owner, it will be set by driver core.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>> index d67baa2..a4dc7a0 100644
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>  	if (IS_ERR(mgn_chip->base))
>>  		return PTR_ERR(mgn_chip->base);
>>  
>> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
>> +
> 
> How is that a useful information?
> 
>>  	for_each_child_of_node(pdev->dev.of_node, np) {
>>  		if (!of_property_read_bool(np, "interrupt-controller"))
>>  			continue;
>>  
>>  		parent = platform_bus_type.dev_root;
>>  		child = of_platform_device_create(np, NULL, parent);
>> -		if (IS_ERR(child))
>> +		if (IS_ERR(child)) {
>> +			dev_err(&pdev->dev, "failed to create for %s\n",
> 
> Failed to create what?
> 
>> +				np->full_name);
>>  			return PTR_ERR(child);
>> +		}
>>  
>>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
>>  					 &num_pins) < 0) {
>> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>  							   mbigen_write_msg,
>>  							   &mbigen_domain_ops,
>>  							   mgn_chip);
>> -		if (!domain)
>> +		if (!domain) {
>> +			dev_info(&pdev->dev, "unable to create %s domain\n",
>> +				 np->full_name);
> 
> And what about failure to read num_pin? No need for a debug print in
> this case?
> 
>>  			return -ENOMEM;
>> +		}
>> +
>> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
>>  	}
>>  
>>  	platform_set_drvdata(pdev, mgn_chip);
>> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
>>  static struct platform_driver mbigen_platform_driver = {
>>  	.driver = {
>>  		.name		= "Hisilicon MBIGEN-V2",
>> -		.owner		= THIS_MODULE,
>>  		.of_match_table	= mbigen_of_match,
>>  	},
>>  	.probe			= mbigen_device_probe,
> 
> 
> Overall, this doesn't look like a critical patch to me. I think Ma Jun
> is working on separate series reworking the way the mgigen is getting
> probed, so I'd advise you to work with him in order to integrate this
> patch in his series, as it would make a lot more sense.

When try to enable hip06 d03 board[1], we met following error log, so I add
some debug message. The mbigen driver use module_platform_driver, the driver
initialization is too late, and it is without any message,  we don't know
about any info of mbigen.  I think we should show something about the mbigen
domain creation at least. What's your option?

Is there a way to solve this improper print?
-----------
[    1.345945] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
[    1.353660] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !


[1] http://www.spinics.net/lists/arm-kernel/msg495732.html

Thanks
Kefeng


> 
> Thanks,
> 
> 	M.
> 

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On Fri, 8 Apr 2016 16:26:21 +0800
Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

> 
> 
> On 2016/4/8 16:09, Marc Zyngier wrote:
> > On Fri, 8 Apr 2016 15:16:02 +0800
> > Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> > 
> >> Add message of MBIGEN domain created, it's useful for check
> >> which MBIGEN domain is created.
> >>
> >> Meanwhile, drop module owner, it will be set by driver core.
> >>
> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >> ---
> >>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> >> index d67baa2..a4dc7a0 100644
> >> --- a/drivers/irqchip/irq-mbigen.c
> >> +++ b/drivers/irqchip/irq-mbigen.c
> >> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
> >>  	if (IS_ERR(mgn_chip->base))
> >>  		return PTR_ERR(mgn_chip->base);
> >>  
> >> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
> >> +
> > 
> > How is that a useful information?
> > 
> >>  	for_each_child_of_node(pdev->dev.of_node, np) {
> >>  		if (!of_property_read_bool(np, "interrupt-controller"))
> >>  			continue;
> >>  
> >>  		parent = platform_bus_type.dev_root;
> >>  		child = of_platform_device_create(np, NULL, parent);
> >> -		if (IS_ERR(child))
> >> +		if (IS_ERR(child)) {
> >> +			dev_err(&pdev->dev, "failed to create for %s\n",
> > 
> > Failed to create what?
> > 
> >> +				np->full_name);
> >>  			return PTR_ERR(child);
> >> +		}
> >>  
> >>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
> >>  					 &num_pins) < 0) {
> >> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
> >>  							   mbigen_write_msg,
> >>  							   &mbigen_domain_ops,
> >>  							   mgn_chip);
> >> -		if (!domain)
> >> +		if (!domain) {
> >> +			dev_info(&pdev->dev, "unable to create %s domain\n",
> >> +				 np->full_name);
> > 
> > And what about failure to read num_pin? No need for a debug print in
> > this case?
> > 
> >>  			return -ENOMEM;
> >> +		}
> >> +
> >> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
> >>  	}
> >>  
> >>  	platform_set_drvdata(pdev, mgn_chip);
> >> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
> >>  static struct platform_driver mbigen_platform_driver = {
> >>  	.driver = {
> >>  		.name		= "Hisilicon MBIGEN-V2",
> >> -		.owner		= THIS_MODULE,
> >>  		.of_match_table	= mbigen_of_match,
> >>  	},
> >>  	.probe			= mbigen_device_probe,
> > 
> > 
> > Overall, this doesn't look like a critical patch to me. I think Ma Jun
> > is working on separate series reworking the way the mgigen is getting
> > probed, so I'd advise you to work with him in order to integrate this
> > patch in his series, as it would make a lot more sense.
> 
> When try to enable hip06 d03 board[1], we met following error log, so I add
> some debug message. The mbigen driver use module_platform_driver, the driver
> initialization is too late, and it is without any message,  we don't know
> about any info of mbigen.  I think we should show something about the mbigen
> domain creation at least. What's your option?
> 
> Is there a way to solve this improper print？
> -----------
> [    1.345945] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
> [    1.353660] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !

How can printing anything solve this issue? Furthermore, the error
message you quote is pretty explicit: no mbigen for you, move along.

There is a long standing dependency issue for interrupt controllers
that are also platform devices, and until you resolve (or help
resolving) that issue, you will get that kind of problem. As I
mentioned countless times (both on list and in person), you only have
two options:

- either you defer probing devices behind the mbigen until the mbigen
  itself is up and running
- or you solve the generic dependency problem.

I feel a bit like a stuck record here.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On Fri, 8 Apr 2016 16:26:21 +0800
Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

> 
> 
> On 2016/4/8 16:09, Marc Zyngier wrote:
> > On Fri, 8 Apr 2016 15:16:02 +0800
> > Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> > 
> >> Add message of MBIGEN domain created, it's useful for check
> >> which MBIGEN domain is created.
> >>
> >> Meanwhile, drop module owner, it will be set by driver core.
> >>
> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >> ---
> >>  drivers/irqchip/irq-mbigen.c | 15 ++++++++++++---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> >> index d67baa2..a4dc7a0 100644
> >> --- a/drivers/irqchip/irq-mbigen.c
> >> +++ b/drivers/irqchip/irq-mbigen.c
> >> @@ -257,14 +257,19 @@ static int mbigen_device_probe(struct platform_device *pdev)
> >>  	if (IS_ERR(mgn_chip->base))
> >>  		return PTR_ERR(mgn_chip->base);
> >>  
> >> +	dev_info(&pdev->dev, "%s\n", pdev->dev.of_node->full_name);
> >> +
> > 
> > How is that a useful information?
> > 
> >>  	for_each_child_of_node(pdev->dev.of_node, np) {
> >>  		if (!of_property_read_bool(np, "interrupt-controller"))
> >>  			continue;
> >>  
> >>  		parent = platform_bus_type.dev_root;
> >>  		child = of_platform_device_create(np, NULL, parent);
> >> -		if (IS_ERR(child))
> >> +		if (IS_ERR(child)) {
> >> +			dev_err(&pdev->dev, "failed to create for %s\n",
> > 
> > Failed to create what?
> > 
> >> +				np->full_name);
> >>  			return PTR_ERR(child);
> >> +		}
> >>  
> >>  		if (of_property_read_u32(child->dev.of_node, "num-pins",
> >>  					 &num_pins) < 0) {
> >> @@ -276,8 +281,13 @@ static int mbigen_device_probe(struct platform_device *pdev)
> >>  							   mbigen_write_msg,
> >>  							   &mbigen_domain_ops,
> >>  							   mgn_chip);
> >> -		if (!domain)
> >> +		if (!domain) {
> >> +			dev_info(&pdev->dev, "unable to create %s domain\n",
> >> +				 np->full_name);
> > 
> > And what about failure to read num_pin? No need for a debug print in
> > this case?
> > 
> >>  			return -ENOMEM;
> >> +		}
> >> +
> >> +		dev_info(&pdev->dev, "%s domain created\n", np->full_name);
> >>  	}
> >>  
> >>  	platform_set_drvdata(pdev, mgn_chip);
> >> @@ -293,7 +303,6 @@ MODULE_DEVICE_TABLE(of, mbigen_of_match);
> >>  static struct platform_driver mbigen_platform_driver = {
> >>  	.driver = {
> >>  		.name		= "Hisilicon MBIGEN-V2",
> >> -		.owner		= THIS_MODULE,
> >>  		.of_match_table	= mbigen_of_match,
> >>  	},
> >>  	.probe			= mbigen_device_probe,
> > 
> > 
> > Overall, this doesn't look like a critical patch to me. I think Ma Jun
> > is working on separate series reworking the way the mgigen is getting
> > probed, so I'd advise you to work with him in order to integrate this
> > patch in his series, as it would make a lot more sense.
> 
> When try to enable hip06 d03 board[1], we met following error log, so I add
> some debug message. The mbigen driver use module_platform_driver, the driver
> initialization is too late, and it is without any message,  we don't know
> about any info of mbigen.  I think we should show something about the mbigen
> domain creation at least. What's your option?
> 
> Is there a way to solve this improper print?
> -----------
> [    1.345945] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
> [    1.353660] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !

How can printing anything solve this issue? Furthermore, the error
message you quote is pretty explicit: no mbigen for you, move along.

There is a long standing dependency issue for interrupt controllers
that are also platform devices, and until you resolve (or help
resolving) that issue, you will get that kind of problem. As I
mentioned countless times (both on list and in person), you only have
two options:

- either you defer probing devices behind the mbigen until the mbigen
  itself is up and running
- or you solve the generic dependency problem.

I feel a bit like a stuck record here.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 16:53, Marc Zyngier wrote:
> On Fri, 8 Apr 2016 16:26:21 +0800
> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

>>> Overall, this doesn't look like a critical patch to me. I think Ma Jun
>>> is working on separate series reworking the way the mgigen is getting
>>> probed, so I'd advise you to work with him in order to integrate this
>>> patch in his series, as it would make a lot more sense.
>>
>> When try to enable hip06 d03 board[1], we met following error log, so I add
>> some debug message. The mbigen driver use module_platform_driver, the driver
>> initialization is too late, and it is without any message,  we don't know
>> about any info of mbigen.  I think we should show something about the mbigen
>> domain creation at least. What's your option?
>>
>> Is there a way to solve this improper print？
>> -----------
>> [    1.345945] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
>> [    1.353660] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
> 
> How can printing anything solve this issue? Furthermore, the error
> message you quote is pretty explicit: no mbigen for you, move along.
> 
> There is a long standing dependency issue for interrupt controllers
> that are also platform devices, and until you resolve (or help
> resolving) that issue, you will get that kind of problem. As I
> mentioned countless times (both on list and in person), you only have
> two options:
> 
> - either you defer probing devices behind the mbigen until the mbigen
>   itself is up and running
> - or you solve the generic dependency problem.
> 
Ok，got it，thanks for your explanation.
> I feel a bit like a stuck record here.

But there is still one issue in the for_each_child_of_node loop of mbigen_device_probe.
Assume that there are 3+ child node(mbigen_gmac, mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa,
if one of them with incorrect configuration, then the loop will end, but we still need
parse the left child node. we should consider this situation, right?

How about split that part into a new mbigen_create_domain(), shown below, and display
the result of mbigen domain creation in it.


diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index d67baa2..9c1d22e 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -236,15 +236,41 @@ static struct irq_domain_ops mbigen_domain_ops = {
 	.free		= irq_domain_free_irqs_common,
 };

-static int mbigen_device_probe(struct platform_device *pdev)
+static void mbigen_create_domain(struct mbigen_device *mgn_dev, struct device_node *np)
 {
-	struct mbigen_device *mgn_chip;
+	struct device *parent = platform_bus_type.dev_root;
+	struct device *dev = &mgn_dev->pdev->dev;
 	struct platform_device *child;
 	struct irq_domain *domain;
+	u32 num_pins;
+
+	if (!of_property_read_bool(np, "interrupt-controller"))
+		goto err;
+
+	if (of_property_read_u32(np, "num-pins", &num_pins))
+		goto err;
+
+	child = of_platform_device_create(np, NULL, parent);
+	if (IS_ERR(child))
+		goto err;
+
+	domain = platform_msi_create_device_domain(&child->dev, num_pins,
+						   mbigen_write_msg,
+						   &mbigen_domain_ops,
+						   mgn_dev);
+	if (!domain)
+		goto err;
+
+	dev_info(dev, "%s domain created\n", np->full_name);
+err:
+	dev_err(dev, "unable to create %s domain\n", np->full_name);
+}
+
+static int mbigen_device_probe(struct platform_device *pdev)
+{
+	struct mbigen_device *mgn_chip;
 	struct device_node *np;
-	struct device *parent;
 	struct resource *res;
-	u32 num_pins;

 	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
 	if (!mgn_chip)
@@ -257,28 +283,8 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);

-	for_each_child_of_node(pdev->dev.of_node, np) {
-		if (!of_property_read_bool(np, "interrupt-controller"))
-			continue;
-
-		parent = platform_bus_type.dev_root;
-		child = of_platform_device_create(np, NULL, parent);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		if (of_property_read_u32(child->dev.of_node, "num-pins",
-					 &num_pins) < 0) {
-			dev_err(&pdev->dev, "No num-pins property\n");
-			return -EINVAL;
-		}
-
-		domain = platform_msi_create_device_domain(&child->dev, num_pins,
-							   mbigen_write_msg,
-							   &mbigen_domain_ops,
-							   mgn_chip);
-		if (!domain)
-			return -ENOMEM;
-	}
+	for_each_child_of_node(pdev->dev.of_node, np)
+		mbigen_create_domain(mgn_chip, np);

 	platform_set_drvdata(pdev, mgn_chip);


Thanks,
Kefeng

> 
> Thanks,
> 
> 	M.
> 

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 16:53, Marc Zyngier wrote:
> On Fri, 8 Apr 2016 16:26:21 +0800
> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:

>>> Overall, this doesn't look like a critical patch to me. I think Ma Jun
>>> is working on separate series reworking the way the mgigen is getting
>>> probed, so I'd advise you to work with him in order to integrate this
>>> patch in his series, as it would make a lot more sense.
>>
>> When try to enable hip06 d03 board[1], we met following error log, so I add
>> some debug message. The mbigen driver use module_platform_driver, the driver
>> initialization is too late, and it is without any message,  we don't know
>> about any info of mbigen.  I think we should show something about the mbigen
>> domain creation at least. What's your option?
>>
>> Is there a way to solve this improper print?
>> -----------
>> [    1.345945] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
>> [    1.353660] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
> 
> How can printing anything solve this issue? Furthermore, the error
> message you quote is pretty explicit: no mbigen for you, move along.
> 
> There is a long standing dependency issue for interrupt controllers
> that are also platform devices, and until you resolve (or help
> resolving) that issue, you will get that kind of problem. As I
> mentioned countless times (both on list and in person), you only have
> two options:
> 
> - either you defer probing devices behind the mbigen until the mbigen
>   itself is up and running
> - or you solve the generic dependency problem.
> 
Ok?got it?thanks for your explanation.
> I feel a bit like a stuck record here.

But there is still one issue in the for_each_child_of_node loop of mbigen_device_probe.
Assume that there are 3+ child node(mbigen_gmac, mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa,
if one of them with incorrect configuration, then the loop will end, but we still need
parse the left child node. we should consider this situation, right?

How about split that part into a new mbigen_create_domain(), shown below, and display
the result of mbigen domain creation in it.


diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index d67baa2..9c1d22e 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -236,15 +236,41 @@ static struct irq_domain_ops mbigen_domain_ops = {
 	.free		= irq_domain_free_irqs_common,
 };

-static int mbigen_device_probe(struct platform_device *pdev)
+static void mbigen_create_domain(struct mbigen_device *mgn_dev, struct device_node *np)
 {
-	struct mbigen_device *mgn_chip;
+	struct device *parent = platform_bus_type.dev_root;
+	struct device *dev = &mgn_dev->pdev->dev;
 	struct platform_device *child;
 	struct irq_domain *domain;
+	u32 num_pins;
+
+	if (!of_property_read_bool(np, "interrupt-controller"))
+		goto err;
+
+	if (of_property_read_u32(np, "num-pins", &num_pins))
+		goto err;
+
+	child = of_platform_device_create(np, NULL, parent);
+	if (IS_ERR(child))
+		goto err;
+
+	domain = platform_msi_create_device_domain(&child->dev, num_pins,
+						   mbigen_write_msg,
+						   &mbigen_domain_ops,
+						   mgn_dev);
+	if (!domain)
+		goto err;
+
+	dev_info(dev, "%s domain created\n", np->full_name);
+err:
+	dev_err(dev, "unable to create %s domain\n", np->full_name);
+}
+
+static int mbigen_device_probe(struct platform_device *pdev)
+{
+	struct mbigen_device *mgn_chip;
 	struct device_node *np;
-	struct device *parent;
 	struct resource *res;
-	u32 num_pins;

 	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
 	if (!mgn_chip)
@@ -257,28 +283,8 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);

-	for_each_child_of_node(pdev->dev.of_node, np) {
-		if (!of_property_read_bool(np, "interrupt-controller"))
-			continue;
-
-		parent = platform_bus_type.dev_root;
-		child = of_platform_device_create(np, NULL, parent);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		if (of_property_read_u32(child->dev.of_node, "num-pins",
-					 &num_pins) < 0) {
-			dev_err(&pdev->dev, "No num-pins property\n");
-			return -EINVAL;
-		}
-
-		domain = platform_msi_create_device_domain(&child->dev, num_pins,
-							   mbigen_write_msg,
-							   &mbigen_domain_ops,
-							   mgn_chip);
-		if (!domain)
-			return -ENOMEM;
-	}
+	for_each_child_of_node(pdev->dev.of_node, np)
+		mbigen_create_domain(mgn_chip, np);

 	platform_set_drvdata(pdev, mgn_chip);


Thanks,
Kefeng

> 
> Thanks,
> 
> 	M.
> 

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On 08/04/16 11:33, Kefeng Wang wrote:
> 
> 
> On 2016/4/8 16:53, Marc Zyngier wrote:
>> On Fri, 8 Apr 2016 16:26:21 +0800
>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
>>>> Overall, this doesn't look like a critical patch to me. I think Ma Jun
>>>> is working on separate series reworking the way the mgigen is getting
>>>> probed, so I'd advise you to work with him in order to integrate this
>>>> patch in his series, as it would make a lot more sense.
>>>
>>> When try to enable hip06 d03 board[1], we met following error log, so I add
>>> some debug message. The mbigen driver use module_platform_driver, the driver
>>> initialization is too late, and it is without any message,  we don't know
>>> about any info of mbigen.  I think we should show something about the mbigen
>>> domain creation at least. What's your option?
>>>
>>> Is there a way to solve this improper print？
>>> -----------
>>> [    1.345945] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
>>> [    1.353660] irq: no irq domain found for /mbigen_pcie@a0080000/intc_usb !
>>
>> How can printing anything solve this issue? Furthermore, the error
>> message you quote is pretty explicit: no mbigen for you, move along.
>>
>> There is a long standing dependency issue for interrupt controllers
>> that are also platform devices, and until you resolve (or help
>> resolving) that issue, you will get that kind of problem. As I
>> mentioned countless times (both on list and in person), you only have
>> two options:
>>
>> - either you defer probing devices behind the mbigen until the mbigen
>>   itself is up and running
>> - or you solve the generic dependency problem.
>>
> Ok，got it，thanks for your explanation.
>> I feel a bit like a stuck record here.
> 
> But there is still one issue in the for_each_child_of_node loop of
> mbigen_device_probe. Assume that there are 3+ child node(mbigen_gmac,
> mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa, if one of them with
> incorrect configuration, then the loop will end, but we still need 
> parse the left child node. we should consider this situation, right?

That's up to whoever designed the driver to decide, really. I don't know
if it is worth continuing in that case (and you are in a better position
than me to find out).

> How about split that part into a new mbigen_create_domain(), shown below, and display
> the result of mbigen domain creation in it.
> 
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index d67baa2..9c1d22e 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -236,15 +236,41 @@ static struct irq_domain_ops mbigen_domain_ops = {
>  	.free		= irq_domain_free_irqs_common,
>  };
> 
> -static int mbigen_device_probe(struct platform_device *pdev)
> +static void mbigen_create_domain(struct mbigen_device *mgn_dev, struct device_node *np)
>  {
> -	struct mbigen_device *mgn_chip;
> +	struct device *parent = platform_bus_type.dev_root;
> +	struct device *dev = &mgn_dev->pdev->dev;
>  	struct platform_device *child;
>  	struct irq_domain *domain;
> +	u32 num_pins;
> +
> +	if (!of_property_read_bool(np, "interrupt-controller"))
> +		goto err;
> +
> +	if (of_property_read_u32(np, "num-pins", &num_pins))
> +		goto err;
> +
> +	child = of_platform_device_create(np, NULL, parent);
> +	if (IS_ERR(child))
> +		goto err;
> +
> +	domain = platform_msi_create_device_domain(&child->dev, num_pins,
> +						   mbigen_write_msg,
> +						   &mbigen_domain_ops,
> +						   mgn_dev);
> +	if (!domain)
> +		goto err;
> +
> +	dev_info(dev, "%s domain created\n", np->full_name);

You're probably missing a "return" here.

> +err:
> +	dev_err(dev, "unable to create %s domain\n", np->full_name);
> +}
> +
> +static int mbigen_device_probe(struct platform_device *pdev)
> +{
> +	struct mbigen_device *mgn_chip;
>  	struct device_node *np;
> -	struct device *parent;
>  	struct resource *res;
> -	u32 num_pins;
> 
>  	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
>  	if (!mgn_chip)
> @@ -257,28 +283,8 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  	if (IS_ERR(mgn_chip->base))
>  		return PTR_ERR(mgn_chip->base);
> 
> -	for_each_child_of_node(pdev->dev.of_node, np) {
> -		if (!of_property_read_bool(np, "interrupt-controller"))
> -			continue;
> -
> -		parent = platform_bus_type.dev_root;
> -		child = of_platform_device_create(np, NULL, parent);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		if (of_property_read_u32(child->dev.of_node, "num-pins",
> -					 &num_pins) < 0) {
> -			dev_err(&pdev->dev, "No num-pins property\n");
> -			return -EINVAL;
> -		}
> -
> -		domain = platform_msi_create_device_domain(&child->dev, num_pins,
> -							   mbigen_write_msg,
> -							   &mbigen_domain_ops,
> -							   mgn_chip);
> -		if (!domain)
> -			return -ENOMEM;
> -	}
> +	for_each_child_of_node(pdev->dev.of_node, np)
> +		mbigen_create_domain(mgn_chip, np);
> 
>  	platform_set_drvdata(pdev, mgn_chip);

In general, I'd suggest you work with Ma Jun, as he maintains that driver.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Marc Zyngier]

On 08/04/16 11:33, Kefeng Wang wrote:
> 
> 
> On 2016/4/8 16:53, Marc Zyngier wrote:
>> On Fri, 8 Apr 2016 16:26:21 +0800
>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
>>>> Overall, this doesn't look like a critical patch to me. I think Ma Jun
>>>> is working on separate series reworking the way the mgigen is getting
>>>> probed, so I'd advise you to work with him in order to integrate this
>>>> patch in his series, as it would make a lot more sense.
>>>
>>> When try to enable hip06 d03 board[1], we met following error log, so I add
>>> some debug message. The mbigen driver use module_platform_driver, the driver
>>> initialization is too late, and it is without any message,  we don't know
>>> about any info of mbigen.  I think we should show something about the mbigen
>>> domain creation at least. What's your option?
>>>
>>> Is there a way to solve this improper print?
>>> -----------
>>> [    1.345945] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
>>> [    1.353660] irq: no irq domain found for /mbigen_pcie at a0080000/intc_usb !
>>
>> How can printing anything solve this issue? Furthermore, the error
>> message you quote is pretty explicit: no mbigen for you, move along.
>>
>> There is a long standing dependency issue for interrupt controllers
>> that are also platform devices, and until you resolve (or help
>> resolving) that issue, you will get that kind of problem. As I
>> mentioned countless times (both on list and in person), you only have
>> two options:
>>
>> - either you defer probing devices behind the mbigen until the mbigen
>>   itself is up and running
>> - or you solve the generic dependency problem.
>>
> Ok?got it?thanks for your explanation.
>> I feel a bit like a stuck record here.
> 
> But there is still one issue in the for_each_child_of_node loop of
> mbigen_device_probe. Assume that there are 3+ child node(mbigen_gmac,
> mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa, if one of them with
> incorrect configuration, then the loop will end, but we still need 
> parse the left child node. we should consider this situation, right?

That's up to whoever designed the driver to decide, really. I don't know
if it is worth continuing in that case (and you are in a better position
than me to find out).

> How about split that part into a new mbigen_create_domain(), shown below, and display
> the result of mbigen domain creation in it.
> 
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index d67baa2..9c1d22e 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -236,15 +236,41 @@ static struct irq_domain_ops mbigen_domain_ops = {
>  	.free		= irq_domain_free_irqs_common,
>  };
> 
> -static int mbigen_device_probe(struct platform_device *pdev)
> +static void mbigen_create_domain(struct mbigen_device *mgn_dev, struct device_node *np)
>  {
> -	struct mbigen_device *mgn_chip;
> +	struct device *parent = platform_bus_type.dev_root;
> +	struct device *dev = &mgn_dev->pdev->dev;
>  	struct platform_device *child;
>  	struct irq_domain *domain;
> +	u32 num_pins;
> +
> +	if (!of_property_read_bool(np, "interrupt-controller"))
> +		goto err;
> +
> +	if (of_property_read_u32(np, "num-pins", &num_pins))
> +		goto err;
> +
> +	child = of_platform_device_create(np, NULL, parent);
> +	if (IS_ERR(child))
> +		goto err;
> +
> +	domain = platform_msi_create_device_domain(&child->dev, num_pins,
> +						   mbigen_write_msg,
> +						   &mbigen_domain_ops,
> +						   mgn_dev);
> +	if (!domain)
> +		goto err;
> +
> +	dev_info(dev, "%s domain created\n", np->full_name);

You're probably missing a "return" here.

> +err:
> +	dev_err(dev, "unable to create %s domain\n", np->full_name);
> +}
> +
> +static int mbigen_device_probe(struct platform_device *pdev)
> +{
> +	struct mbigen_device *mgn_chip;
>  	struct device_node *np;
> -	struct device *parent;
>  	struct resource *res;
> -	u32 num_pins;
> 
>  	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
>  	if (!mgn_chip)
> @@ -257,28 +283,8 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  	if (IS_ERR(mgn_chip->base))
>  		return PTR_ERR(mgn_chip->base);
> 
> -	for_each_child_of_node(pdev->dev.of_node, np) {
> -		if (!of_property_read_bool(np, "interrupt-controller"))
> -			continue;
> -
> -		parent = platform_bus_type.dev_root;
> -		child = of_platform_device_create(np, NULL, parent);
> -		if (IS_ERR(child))
> -			return PTR_ERR(child);
> -
> -		if (of_property_read_u32(child->dev.of_node, "num-pins",
> -					 &num_pins) < 0) {
> -			dev_err(&pdev->dev, "No num-pins property\n");
> -			return -EINVAL;
> -		}
> -
> -		domain = platform_msi_create_device_domain(&child->dev, num_pins,
> -							   mbigen_write_msg,
> -							   &mbigen_domain_ops,
> -							   mgn_chip);
> -		if (!domain)
> -			return -ENOMEM;
> -	}
> +	for_each_child_of_node(pdev->dev.of_node, np)
> +		mbigen_create_domain(mgn_chip, np);
> 
>  	platform_set_drvdata(pdev, mgn_chip);

In general, I'd suggest you work with Ma Jun, as he maintains that driver.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 18:46, Marc Zyngier wrote:
> On 08/04/16 11:33, Kefeng Wang wrote:
>>
>>
>> On 2016/4/8 16:53, Marc Zyngier wrote:
>>> On Fri, 8 Apr 2016 16:26:21 +0800
>>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
[...]
>> But there is still one issue in the for_each_child_of_node loop of
>> mbigen_device_probe. Assume that there are 3+ child node(mbigen_gmac,
>> mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa, if one of them with
>> incorrect configuration, then the loop will end, but we still need 
>> parse the left child node. we should consider this situation, right?
> 
> That's up to whoever designed the driver to decide, really. I don't know
> if it is worth continuing in that case (and you are in a better position
> than me to find out).

Ok, will talk with Ma Jun directly, and if the change is needed, I will send
a new patch after reviewed by Ma Jun.

Thanks,
Kefeng

[PATCH] irqchip/mbigen: Display message of MBIGEN domain created

[Kefeng Wang]

On 2016/4/8 18:46, Marc Zyngier wrote:
> On 08/04/16 11:33, Kefeng Wang wrote:
>>
>>
>> On 2016/4/8 16:53, Marc Zyngier wrote:
>>> On Fri, 8 Apr 2016 16:26:21 +0800
>>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
[...]
>> But there is still one issue in the for_each_child_of_node loop of
>> mbigen_device_probe. Assume that there are 3+ child node(mbigen_gmac,
>> mbigen_i2c, mbigen_xxx, etc) in mbigen_chip_dsa, if one of them with
>> incorrect configuration, then the loop will end, but we still need 
>> parse the left child node. we should consider this situation, right?
> 
> That's up to whoever designed the driver to decide, really. I don't know
> if it is worth continuing in that case (and you are in a better position
> than me to find out).

Ok, will talk with Ma Jun directly, and if the change is needed, I will send
a new patch after reviewed by Ma Jun.

Thanks,
Kefeng