linux-kernel - Re: [PATCH v2 -next 03/11] irqchip: mip: Add Broadcom bcm2712 MSI-X interrupt controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9e4253f-c817-4197-9c77-40d2ac8dec65@suse.de>
Date: Tue, 17 Sep 2024 13:36:20 +0300
From: Stanimir Varbanov <svarbanov@...e.de>
To: Thomas Gleixner <tglx@...utronix.de>,
 Stanimir Varbanov <svarbanov@...e.de>, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rpi-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
 Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Florian Fainelli <florian.fainelli@...adcom.com>,
 Jim Quinlan <jim2101024@...il.com>,
 Nicolas Saenz Julienne <nsaenz@...nel.org>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>, kw@...ux.com,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Andrea della Porta <andrea.porta@...e.com>,
 Phil Elwell <phil@...pberrypi.com>, Jonathan Bell <jonathan@...pberrypi.com>
Subject: Re: [PATCH v2 -next 03/11] irqchip: mip: Add Broadcom bcm2712 MSI-X
 interrupt controller

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