[RFC] PCI: designware: Skip allocating own MSI domain if using external MSI domain

Message ID 1540435647-134674-1-git-send-email-shawn.lin@rock-chips.com
State New
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • [RFC] PCI: designware: Skip allocating own MSI domain if using external MSI domain
Related show

Commit Message

Shawn Lin Oct. 25, 2018, 2:47 a.m.
On some platform, external MSI domain is using instead of the one
created by designware driver. For instance, if using GIC-V3-ITS
as a MSI domain, we only need set msi-map in the devicetree but
never need any bit in the designware driver to handle MSI stuff.
So skip allocating its own MSI domain for that case.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Gustavo Pimentel Nov. 13, 2018, 5:45 a.m. | #1
Hi Shawn,

I'd suggest to change the designware by dwc on the patch title.

On 25/10/2018 03:47, Shawn Lin wrote:
> On some platform, external MSI domain is using instead of the one
> created by designware driver. For instance, if using GIC-V3-ITS
> as a MSI domain, we only need set msi-map in the devicetree but
> never need any bit in the designware driver to handle MSI stuff.
> So skip allocating its own MSI domain for that case.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 29a0575..39254b1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -278,6 +278,10 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
>  		return -ENOMEM;
>  	}
>  
> +	/* Rely on the external MSI domain */
> +	if (device_property_read_bool(dev, "msi-map"))
> +		return 0;
> +

I would suggest to read this property only in the init of the host and
configuring it in a Boolean variable to be created in the pp structure and use
it to verify the feature presence or not.

>  	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
>  						   &dw_pcie_msi_domain_info,
>  						   pp->irq_domain);
> @@ -433,7 +437,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	if (ret)
>  		pci->num_viewport = 2;
>  
> -	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +	if (IS_ENABLED(CONFIG_PCI_MSI) &&
> +	    !device_property_read_bool(dev, "msi-map")) {

You could use the previous boolean variable here also.

>  		/*
>  		 * If a specific SoC driver needs to change the
>  		 * default number of vectors, it needs to implement
> 

You should protect the dw_pcie_free_msi(), since it uses the pp->msi_domain and
pp->msi_irq also, maybe using the boolean variable also.

Regards,
Gustavo
Shawn Lin Nov. 13, 2018, 6:21 a.m. | #2
Hi Gustavo,

On 2018/11/13 13:45, Gustavo Pimentel wrote:
> Hi Shawn,
> 
> I'd suggest to change the designware by dwc on the patch title.
> 

Ok.

> On 25/10/2018 03:47, Shawn Lin wrote:
>> On some platform, external MSI domain is using instead of the one
>> created by designware driver. For instance, if using GIC-V3-ITS
>> as a MSI domain, we only need set msi-map in the devicetree but
>> never need any bit in the designware driver to handle MSI stuff.
>> So skip allocating its own MSI domain for that case.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>   drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 29a0575..39254b1 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -278,6 +278,10 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
>>   		return -ENOMEM;
>>   	}
>>   
>> +	/* Rely on the external MSI domain */
>> +	if (device_property_read_bool(dev, "msi-map"))
>> +		return 0;
>> +
> 
> I would suggest to read this property only in the init of the host and
> configuring it in a Boolean variable to be created in the pp structure and use
> it to verify the feature presence or not.
> 

Will do.

>>   	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
>>   						   &dw_pcie_msi_domain_info,
>>   						   pp->irq_domain);
>> @@ -433,7 +437,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>   	if (ret)
>>   		pci->num_viewport = 2;
>>   
>> -	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> +	if (IS_ENABLED(CONFIG_PCI_MSI) &&
>> +	    !device_property_read_bool(dev, "msi-map")) {
> 
> You could use the previous boolean variable here also.
> 
>>   		/*
>>   		 * If a specific SoC driver needs to change the
>>   		 * default number of vectors, it needs to implement
>>
> 
> You should protect the dw_pcie_free_msi(), since it uses the pp->msi_domain and
> pp->msi_irq also, maybe using the boolean variable also.

Yeah, will fix.

Thanks for your comment.

> 
> Regards,
> Gustavo
> 
> 
>

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 29a0575..39254b1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -278,6 +278,10 @@  int dw_pcie_allocate_domains(struct pcie_port *pp)
 		return -ENOMEM;
 	}
 
+	/* Rely on the external MSI domain */
+	if (device_property_read_bool(dev, "msi-map"))
+		return 0;
+
 	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
 						   &dw_pcie_msi_domain_info,
 						   pp->irq_domain);
@@ -433,7 +437,8 @@  int dw_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		pci->num_viewport = 2;
 
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+	if (IS_ENABLED(CONFIG_PCI_MSI) &&
+	    !device_property_read_bool(dev, "msi-map")) {
 		/*
 		 * If a specific SoC driver needs to change the
 		 * default number of vectors, it needs to implement