Re: [RFC v4 6/8] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver

From: Julien Grall <julien.grall@linaro.org>
To: Manish Jaggi <mjaggi@caviumnetworks.com>,
	Sameer Goel <sameer.goel@linaro.org>,
	xen-devel@lists.xenproject.org, julien.grall@arm.com
Cc: sstabellini@kernel.org, shankerd@codeaurora.org,
	andre.przywara@linaro.org
Subject: Re: [RFC v4 6/8] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver
Date: Tue, 16 Jan 2018 13:13:47 +0000	[thread overview]
Message-ID: <a484db15-c165-183b-a088-788789086037@linaro.org> (raw)
In-Reply-To: <5122539b-0ec0-33d0-b5a4-6f721c754187@caviumnetworks.com>

Hi,

On 16/01/18 12:37, Manish Jaggi wrote:
> On 01/16/2018 02:04 AM, Julien Grall wrote:
>> On 01/03/2018 05:47 AM, Manish Jaggi wrote:
>>>> +int devm_request_threaded_irq(struct device *dev, unsigned int irq, 
>>>> irq_handler_t handler,
>>>> +                  irq_handler_t thread_fn, unsigned long irqflags,
>>>> +                  const char *devname, void *dev_id)
>>>> +{
>>>> +    return devm_request_irq(dev, irq, thread_fn, irqflags, devname, 
>>>> dev_id);
>>>> +}
>>> Is it possible to change the name from threaded to something more 
>>> meaningful as IIUC in xen we dont  have threaded irqs.
>>> Though the code is coming from linux, but it has to be called/named 
>>> in the place it is intended to be used
>>
>> What do you mean? This is a wrapper for Linux. So we should keep the 
>> name as it is.
> It creates confusion as xne doesnt use threads. So we should rename it.
> There is no reason why we cannot rename a function used in linux.

Read my previous e-mail until the end to understand the reason...

>>
>> [...]
>>
>>>> @@ -433,6 +807,7 @@ enum pri_resp {
>>>>       PRI_RESP_SUCC,
>>>>   };
>>>> +#if 0 /* Xen: No MSI support in this iteration */
>>>>   enum arm_smmu_msi_index {
>>>>       EVTQ_MSI_INDEX,
>>>>       GERROR_MSI_INDEX,
>>>> @@ -457,6 +832,7 @@ static phys_addr_t 
>>>> arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
>>>>           ARM_SMMU_PRIQ_IRQ_CFG2,
>>>>       },
>>>>   };
>>>> +#endif
>>> IMHO can we avoid #if 0 from the code, unless we intend to use the 
>>> code in future.
>>
>> In the past, I made the mistake to remove all unecessary code from 
>> SMMUv2. Few months after, we decided to delete it and import directly 
>> from Linux with limited modifications. This was the best choice 
>> because it is easier to track difference.
>>
>> We are in the same situation here. We want to stay as close as Linux. 
>> This means no renaming, no code removal, and very limited change in 
>> the code to accommodate Xen.

... particularly this paragraph.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel