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@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 !


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

Thanks
Kefeng


> 
> Thanks,
> 
> 	M.
>