Re: [PATCH v2 2/5] PCI: dwc: Teach dwc core to parse additional MSI interrupts

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 25/04/2022 04:50, Bjorn Helgaas wrote:
> On Sat, Apr 23, 2022 at 04:39:36PM +0300, Dmitry Baryshkov wrote:
>> DWC driver parses a single "msi" interrupt which gets fired when the EP
>> sends an MSI interrupt, however for some devices (Qualcomm) devies MSI
>> vectors are handled in groups of 32 vectors. Add support for parsing
>> "split" MSI interrupts.
> 
> devies?  Maybe spurious?

Devices, of course :D

> 
>> In addition to the "msi" interrupt, the code will lookup the "msi2",
>> "msi3", etc. IRQs and use them for the MSI group interrupts. For
>> backwards compatibility with existing DTS files, the code will not error
>> out if any of these interrupts is missing. Instead it will limit itself
>> to the amount of MSI group IRQs declared in the DT file.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   .../pci/controller/dwc/pcie-designware-host.c | 23 +++++++++++++++++++
>>   drivers/pci/controller/dwc/pcie-designware.h  |  1 +
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 5d90009a0f73..ce7071095006 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -382,6 +382,29 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>   				pp->msi_irq[0] = irq;
>>   			}
>>   
>> +			if (pp->has_split_msi_irq) {
>> +				char irq_name[] = "msiXXX";
>> +				int irq;
>> +
>> +				for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
>> +					if (pp->msi_irq[ctrl])
>> +						continue;
>> +
>> +					snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl + 1);
>> +					irq = platform_get_irq_byname_optional(pdev, irq_name);
>> +					if (irq == -ENXIO) {
>> +						num_ctrls = ctrl;
>> +						pp->num_vectors = num_ctrls * MAX_MSI_IRQS_PER_CTRL;
>> +						dev_warn(dev, "Limiting amount of MSI irqs to %d\n", pp->num_vectors);
>> +						break;
>> +					}
>> +					if (irq < 0)
>> +						return irq;
>> +
>> +					pp->msi_irq[ctrl] = irq;
>> +				}
>> +			}
> 
> This is getting pretty deeply nested, which means it's impractical to
> fit in 80 columns like the rest of the file, which means it's ripe for
> refactoring to reduce the indentation.
> 
> s/amount of/number of/
> s/MSI irqs/MSI IRQs/
> 
>>   			pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
>>   
>>   			ret = dw_pcie_allocate_domains(pp);
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 9c1a38b0a6b3..3aa840a5b19c 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -179,6 +179,7 @@ struct dw_pcie_host_ops {
>>   
>>   struct pcie_port {
>>   	bool			has_msi_ctrl:1;
>> +	bool			has_split_msi_irq:1;
>>   	u64			cfg0_base;
>>   	void __iomem		*va_cfg0_base;
>>   	u32			cfg0_size;
>> -- 
>> 2.35.1
>>


-- 
With best wishes
Dmitry