Hi Thomas,
Thank you for the comments!
On 9/10/24 18:58, Thomas Gleixner wrote:
> On Tue, Sep 10 2024 at 18:18, Stanimir Varbanov wrote:
>> +
>> +struct mip_priv {
>> + /* used to protect bitmap alloc/free */
>> + spinlock_t lock;
>> + void __iomem *base;
>> + u64 msg_addr;
>> + u32 msi_base;
>> + u32 num_msis;
>> + unsigned long *bitmap;
>> + struct irq_domain *parent;
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
>
> And please read the rest of the document too.
Sure.
>
>> +};
>> +
>> +static void mip_mask_msi_irq(struct irq_data *d)
>> +{
>> + pci_msi_mask_irq(d);
>> + irq_chip_mask_parent(d);
>> +}
>> +
>> +static void mip_unmask_msi_irq(struct irq_data *d)
>> +{
>> + pci_msi_unmask_irq(d);
>> + irq_chip_unmask_parent(d);
>
> This is asymmetric vs. mask(), but that's just the usual copy & pasta
> problem.
Correct, but this will disappear when convert to MSI parent.
>
>> +}
>> +static int mip_init_domains(struct mip_priv *priv, struct device_node *np)
>> +{
>> + struct irq_domain *middle_domain, *msi_domain;
>> +
>> + middle_domain = irq_domain_add_hierarchy(priv->parent, 0,
>> + priv->num_msis, np,
>> + &mip_middle_domain_ops,
>> + priv);
>> + if (!middle_domain)
>> + return -ENOMEM;
>> +
>> + msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(np),
>> + &mip_msi_domain_info,
>> + middle_domain);
>> + if (!msi_domain) {
>> + irq_domain_remove(middle_domain);
>> + return -ENOMEM;
>> + }
>
> This is not much different. Please convert this to a proper MSI parent
> domain and let the PCI/MSI core handle the PCI/MSI part.
I apologize, but I wasn't able to make it work on time. The good news is
that I made it now and will send it in next version of the series.
regards,
~Stan
