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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel