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@xxxxxxxxxx> wrote:

> 
> 
> On 2016/4/8 16:09, Marc Zyngier wrote:
> > On Fri, 8 Apr 2016 15:16:02 +0800
> > Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >>  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.