Re: [PATCH] irqchip: fix the config HISILICON_IRQ_MBIGEN dependency error.

[Marc Zyngier]

On 21/06/16 13:01, Jiancheng Xue wrote:
> 
> 
> On 2016/6/21 19:30, Jiancheng Xue wrote:
>> Hi Marcï
>>
>> On 2016/6/21 18:36, Marc Zyngier wrote:
>>> On 21/06/16 10:26, Jiancheng Xue wrote:
>>>> This patch fixes the compiling error caused when
>>>> config HISILICON_IRQ_MBIGEN is selected but
>>>> PCI_MSI is not seleted.
>>>>
>>>> Signed-off-by: Jiancheng Xue <xuejiancheng@xxxxxxxxxxxxx>
>>>> ---
>>>>  drivers/irqchip/Kconfig | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>>> index fa33c50..23dcf3e 100644
>>>> --- a/drivers/irqchip/Kconfig
>>>> +++ b/drivers/irqchip/Kconfig
>>>> @@ -110,7 +110,7 @@ config DW_APB_ICTL
>>>>  config HISILICON_IRQ_MBIGEN
>>>>  	bool
>>>>  	select ARM_GIC_V3
>>>> -	select ARM_GIC_V3_ITS
>>>> +	select ARM_GIC_V3_ITS if PCI_MSI
>>>>  	select GENERIC_MSI_IRQ_DOMAIN
>>>
>>> How can this be correct? The MBIGEN uses platform MSI (not PCI) and
>>> relies on the ITS (it doesn't work without it). It seems that you're
>>> papering over another issue.
>>>
>> Sorry. I am not familiar with this part. But I encountered errors when
>> I compiled. I think there may be some problems about dependency.
>>
>> In this Kconfig file
>>
>> config ARM_GIC_V3_ITS
>>         bool
>>         select PCI_MSI_IRQ_DOMAIN
>>
>> config HISILICON_IRQ_MBIGEN
>>   	bool
>>  	select ARM_GIC_V3
>> 	select ARM_GIC_V3_ITS
>>   	select GENERIC_MSI_IRQ_DOMAIN
>>
>> In the file drivers/pci/Kconfig
>>
>> config PCI_MSI_IRQ_DOMAIN
>>         bool
>>         depends on PCI_MSI
>>         select GENERIC_MSI_IRQ_DOMAIN
>>
>> We can see if the HISILICON_IRQ_MBIGEN is selected, the ARM_GIC_V3_ITS and
>> PCI_MSI_IRQ_DOMAIN will be selected. But PCI_MSI_IRQ_DOMAIN depends on
>> PCI_MSI. If PCI_MSI is not selected, it will cause a compiling error like this
>> "drivers/irqchip/irq-gic-v3-its-pci-msi.c:52:12: error: implicit declaration of function 'pci_msi_vec_count' [-Werror=implicit-function-declaration]
>>   msi = max(pci_msi_vec_count(pdev), 0);"
>>
>> I found many other options which need ARM_GIC_V3_ITS or PCI_MSI_IRQ_DOMAIN
>> were configured like below:
>> select ARM_GIC_V3_ITS if PCI_MSI
>> or
>> select GENERIC_MSI_IRQ_DOMAIN if PCI_MSI
>>
> 
> This patch is really not correct. It will cause new problems.
> 
> In drivers/irqchip/Makefile
> 
> obj-$(CONFIG_ARM_GIC_V3_ITS)            += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
> 
> if CONFIG_ARM_GIC_V3_ITS is selected, irq-gic-v3-its-pci-msi.c will be compiled. But it depends on config PCI_MSI.
> 
> if irq-gic-v3-its-pci-msi.c is not relied on by irq-gic-v3-its.c, maybe it should be controlled by another config
> item instead of CONFIG_ARM_GIC_V3_ITS. Otherwise, CONFIG_ARM_GIC_V3_ITS should depend on PCI_MSI.

Can you check this patch out:

https://patchwork.kernel.org/patch/9179303/

and let people know if that solves your problem or not?

Thanks,

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