Received-SPF: pass (google.com: domain of linux-kernel+bounces-75188-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Thomas Gleixner <tglx@linutronix.de>
To: "Gupta, Nipun" <nipun.gupta@amd.com>, gregkh@linuxfoundation.org,
 maz@kernel.org, jgg@ziepe.ca, linux-kernel@vger.kernel.org
Cc: git@amd.com, harpreet.anand@amd.com, pieter.jansen-van-vuuren@amd.com,
 nikhil.agarwal@amd.com, michal.simek@amd.com, abhijit.gangurde@amd.com,
 srivatsa@csail.mit.edu
Subject: Re: [PATCH v7] cdx: add MSI support for CDX bus
In-Reply-To: <929bf3bd-fb5a-4d1d-aa46-a5ce43ce574b@amd.com>
References: <20240202113811.2546311-1-nipun.gupta@amd.com>
 <877cj0a4fj.ffs@tglx> <929bf3bd-fb5a-4d1d-aa46-a5ce43ce574b@amd.com>
Date: Wed, 21 Feb 2024 18:20:09 +0100
Message-ID: <87wmqxg2eu.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On Wed, Feb 21 2024 at 18:10, Gupta, Nipun wrote:
> On 2/19/2024 8:26 PM, Thomas Gleixner wrote:
>>> +int cdx_msi_domain_alloc_irqs(struct device *dev, unsigned int irq_count)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = msi_setup_device_data(dev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = msi_domain_alloc_irqs_range(dev, MSI_DEFAULT_DOMAIN,
>>> +					  0, irq_count - 1);
>>> +	if (ret)
>>> +		dev_err(dev, "Failed to allocate IRQs: %d\n", ret);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(cdx_msi_domain_alloc_irqs);
>> 
>> Why does this need a special allocation function instead of setting the
>> irq domain of the device and using the generic allocation function
>> directly?
>
> Do you suggest we call msi_setup_device_data() during the 
> cdx_device_add() in the CDX bus where we also assign the IRQ domain to 
> the device; so CDX device driver can directly call 
> msi_domain_alloc_irqs_range() API to allocate the IRQs?

That sounds about right. It avoids doing it in the drivers and does not
require a special function. Maybe an inline wrapper to convert the
simple count to the range required.

>>> +
>>> +#ifdef GENERIC_MSI_DOMAIN_OPS
>>> +	/* Set the device Id to be passed to the GIC-ITS */
>>> +	info->scratchpad[0].ul = dev_id;
>>> +#endif
>> 
>> Is this ever used on a platform which does not have
>> GENERIC_MSI_DOMAIN_OPS ?
>
> This was added to enable the COMPILE_TEST.

Ah. Ok.

Thanks,

        tglx