Message ID | 20151215120239.26216.43390.sendpatchset@little-apple (mailing list archive) |
---|---|
State | Under Review |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Hi Magnus, On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > --- 0004/drivers/iommu/ipmmu-vmsa.c > +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000 +0900 > @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm > static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, > unsigned int utlb) > { > - struct ipmmu_vmsa_device *mmu = domain->mmu; > + ipmmu_write(domain->mmu, IMUCTR(utlb), 0); > +} > + > +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain, > + void (*fn)(struct ipmmu_vmsa_domain *, > + unsigned int utlb), struct device *dev) > +{ > + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev); > + unsigned int i; > > - ipmmu_write(mmu, IMUCTR(utlb), 0); > + for (i = 0; i < dev_data->num_utlbs; ++i) > + fn(domain, dev_data->utlbs[i]); > } Unless you have further plans with the "fn" parameter, I would simply pass a bool enable/disable flag instead of a function pointer. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus, On Tuesday 15 December 2015 16:48:42 Geert Uytterhoeven wrote: > On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > > --- 0004/drivers/iommu/ipmmu-vmsa.c > > +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000 > > +0900 > > @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm > > static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, > > unsigned int utlb) > > { > > - struct ipmmu_vmsa_device *mmu = domain->mmu; > > + ipmmu_write(domain->mmu, IMUCTR(utlb), 0); > > +} > > + > > +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain, > > + void (*fn)(struct ipmmu_vmsa_domain *, > > + unsigned int utlb), struct device > > *dev) > > +{ > > + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev); > > + unsigned int i; > > > > - ipmmu_write(mmu, IMUCTR(utlb), 0); > > + for (i = 0; i < dev_data->num_utlbs; ++i) > > + fn(domain, dev_data->utlbs[i]); > > > > } > > Unless you have further plans with the "fn" parameter, I would simply pass > a bool enable/disable flag instead of a function pointer. I agree with Geert. What's your plan here ?
--- 0004/drivers/iommu/ipmmu-vmsa.c +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:17:40.580513000 +0900 @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, unsigned int utlb) { - struct ipmmu_vmsa_device *mmu = domain->mmu; + ipmmu_write(domain->mmu, IMUCTR(utlb), 0); +} + +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain, + void (*fn)(struct ipmmu_vmsa_domain *, + unsigned int utlb), struct device *dev) +{ + struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev); + unsigned int i; - ipmmu_write(mmu, IMUCTR(utlb), 0); + for (i = 0; i < dev_data->num_utlbs; ++i) + fn(domain, dev_data->utlbs[i]); } static void ipmmu_tlb_flush_all(void *cookie) @@ -503,7 +512,6 @@ static int ipmmu_attach_device(struct io struct ipmmu_vmsa_device *mmu = dev_data->mmu; struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain); unsigned long flags; - unsigned int i; int ret = 0; if (!mmu) { @@ -529,24 +537,16 @@ static int ipmmu_attach_device(struct io spin_unlock_irqrestore(&domain->lock, flags); - if (ret < 0) - return ret; + if (!ret) + ipmmu_utlb_ctrl(domain, ipmmu_utlb_enable, dev); - for (i = 0; i < dev_data->num_utlbs; ++i) - ipmmu_utlb_enable(domain, dev_data->utlbs[i]); - - return 0; + return ret; } static void ipmmu_detach_device(struct iommu_domain *io_domain, struct device *dev) { - struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev); - struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain); - unsigned int i; - - for (i = 0; i < dev_data->num_utlbs; ++i) - ipmmu_utlb_disable(domain, dev_data->utlbs[i]); + ipmmu_utlb_ctrl(to_vmsa_domain(io_domain), ipmmu_utlb_disable, dev); /* * TODO: Optimize by disabling the context when no device is attached.