| Message ID | 148897089355.16106.305846051553789544.sendpatchset@little-apple (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Magnus, On 08/03/17 11:01, Magnus Damm wrote: > From: Magnus Damm <damm+renesas@opensource.se> > > Introduce struct ipmmu_features to track various hardware > and software implementation changes inside the driver for > different kinds of IPMMU hardware. Add use_ns_alias_offset > as a first example of a feature to control if the secure > register bank offset should be used or not. > > Signed-off-by: Magnus Damm <damm+renesas@opensource.se> > --- > > Changes since V2: > - None > > Changes since V1: > - Moved patch to front of the series > > drivers/iommu/ipmmu-vmsa.c | 35 ++++++++++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 7 deletions(-) > > --- 0007/drivers/iommu/ipmmu-vmsa.c > +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-07 12:25:47.000000000 +0900 > @@ -32,11 +32,15 @@ > > #define IPMMU_CTX_MAX 1 > > +struct ipmmu_features { > + bool use_ns_alias_offset; > +}; > + > struct ipmmu_vmsa_device { > struct device *dev; > void __iomem *base; > struct list_head list; > - > + const struct ipmmu_features *features; > unsigned int num_utlbs; > spinlock_t lock; /* Protects ctx and domains[] */ > DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); > @@ -999,13 +1003,33 @@ static void ipmmu_device_reset(struct ip > ipmmu_write(mmu, i * IM_CTX_SIZE + IMCTR, 0); > } > > +static const struct ipmmu_features ipmmu_features_default = { > + .use_ns_alias_offset = true, > +}; > + > +static const struct of_device_id ipmmu_of_ids[] = { > + { > + .compatible = "renesas,ipmmu-vmsa", > + .data = &ipmmu_features_default, > + }, { > + /* Terminator */ > + }, > +}; > + > +MODULE_DEVICE_TABLE(of, ipmmu_of_ids); > + > static int ipmmu_probe(struct platform_device *pdev) > { > struct ipmmu_vmsa_device *mmu; > + const struct of_device_id *match; > struct resource *res; > int irq; > int ret; > > + match = of_match_node(ipmmu_of_ids, pdev->dev.of_node); of_device_get_match_data() makes this a lot easier. > + if (!match) > + return -EINVAL; Also, if the driver is DT-only per the other series, note that this cannot happen anyway, since of_driver_match_device() would have to have found a match for your probe function to be called in the first place. Robin. > + > mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL); > if (!mmu) { > dev_err(&pdev->dev, "cannot allocate device data\n"); > @@ -1016,6 +1040,7 @@ static int ipmmu_probe(struct platform_d > mmu->num_utlbs = 32; > spin_lock_init(&mmu->lock); > bitmap_zero(mmu->ctx, IPMMU_CTX_MAX); > + mmu->features = match->data; > > /* Map I/O memory and request IRQ. */ > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -1035,7 +1060,8 @@ static int ipmmu_probe(struct platform_d > * Offset the registers base unconditionally to point to the non-secure > * alias space for now. > */ > - mmu->base += IM_NS_ALIAS_OFFSET; > + if (mmu->features->use_ns_alias_offset) > + mmu->base += IM_NS_ALIAS_OFFSET; > > irq = platform_get_irq(pdev, 0); > if (irq < 0) { > @@ -1084,11 +1110,6 @@ static int ipmmu_remove(struct platform_ > return 0; > } > > -static const struct of_device_id ipmmu_of_ids[] = { > - { .compatible = "renesas,ipmmu-vmsa", }, > - { } > -}; > - > static struct platform_driver ipmmu_driver = { > .driver = { > .name = "ipmmu-vmsa", >
Hi Robin, On Wed, Mar 8, 2017 at 8:53 PM, Robin Murphy <robin.murphy@arm.com> wrote: > Hi Magnus, > > On 08/03/17 11:01, Magnus Damm wrote: >> From: Magnus Damm <damm+renesas@opensource.se> >> >> Introduce struct ipmmu_features to track various hardware >> and software implementation changes inside the driver for >> different kinds of IPMMU hardware. Add use_ns_alias_offset >> as a first example of a feature to control if the secure >> register bank offset should be used or not. >> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se> >> --- >> >> Changes since V2: >> - None >> >> Changes since V1: >> - Moved patch to front of the series >> >> drivers/iommu/ipmmu-vmsa.c | 35 ++++++++++++++++++++++++++++------- >> 1 file changed, 28 insertions(+), 7 deletions(-) >> >> --- 0007/drivers/iommu/ipmmu-vmsa.c >> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-07 12:25:47.000000000 +0900 >> @@ -32,11 +32,15 @@ >> >> #define IPMMU_CTX_MAX 1 >> >> +struct ipmmu_features { >> + bool use_ns_alias_offset; >> +}; >> + >> struct ipmmu_vmsa_device { >> struct device *dev; >> void __iomem *base; >> struct list_head list; >> - >> + const struct ipmmu_features *features; >> unsigned int num_utlbs; >> spinlock_t lock; /* Protects ctx and domains[] */ >> DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); >> @@ -999,13 +1003,33 @@ static void ipmmu_device_reset(struct ip >> ipmmu_write(mmu, i * IM_CTX_SIZE + IMCTR, 0); >> } >> >> +static const struct ipmmu_features ipmmu_features_default = { >> + .use_ns_alias_offset = true, >> +}; >> + >> +static const struct of_device_id ipmmu_of_ids[] = { >> + { >> + .compatible = "renesas,ipmmu-vmsa", >> + .data = &ipmmu_features_default, >> + }, { >> + /* Terminator */ >> + }, >> +}; >> + >> +MODULE_DEVICE_TABLE(of, ipmmu_of_ids); >> + >> static int ipmmu_probe(struct platform_device *pdev) >> { >> struct ipmmu_vmsa_device *mmu; >> + const struct of_device_id *match; >> struct resource *res; >> int irq; >> int ret; >> >> + match = of_match_node(ipmmu_of_ids, pdev->dev.of_node); > > of_device_get_match_data() makes this a lot easier. > >> + if (!match) >> + return -EINVAL; > > Also, if the driver is DT-only per the other series, note that this > cannot happen anyway, since of_driver_match_device() would have to have > found a match for your probe function to be called in the first place. Yeah, you are right. As you know, in the IPMMU driver (with the r8a7795 V3 series applied) the init handling is a bit special with ARM32 and ARM64 being treated differently. I would like to clean it up and share a common implementation. Until that happens, how do you think we should handle the (!match) case? BUG_ON()? Cheers, / magnus
--- 0007/drivers/iommu/ipmmu-vmsa.c +++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-07 12:25:47.000000000 +0900 @@ -32,11 +32,15 @@ #define IPMMU_CTX_MAX 1 +struct ipmmu_features { + bool use_ns_alias_offset; +}; + struct ipmmu_vmsa_device { struct device *dev; void __iomem *base; struct list_head list; - + const struct ipmmu_features *features; unsigned int num_utlbs; spinlock_t lock; /* Protects ctx and domains[] */ DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); @@ -999,13 +1003,33 @@ static void ipmmu_device_reset(struct ip ipmmu_write(mmu, i * IM_CTX_SIZE + IMCTR, 0); } +static const struct ipmmu_features ipmmu_features_default = { + .use_ns_alias_offset = true, +}; + +static const struct of_device_id ipmmu_of_ids[] = { + { + .compatible = "renesas,ipmmu-vmsa", + .data = &ipmmu_features_default, + }, { + /* Terminator */ + }, +}; + +MODULE_DEVICE_TABLE(of, ipmmu_of_ids); + static int ipmmu_probe(struct platform_device *pdev) { struct ipmmu_vmsa_device *mmu; + const struct of_device_id *match; struct resource *res; int irq; int ret; + match = of_match_node(ipmmu_of_ids, pdev->dev.of_node); + if (!match) + return -EINVAL; + mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL); if (!mmu) { dev_err(&pdev->dev, "cannot allocate device data\n"); @@ -1016,6 +1040,7 @@ static int ipmmu_probe(struct platform_d mmu->num_utlbs = 32; spin_lock_init(&mmu->lock); bitmap_zero(mmu->ctx, IPMMU_CTX_MAX); + mmu->features = match->data; /* Map I/O memory and request IRQ. */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1035,7 +1060,8 @@ static int ipmmu_probe(struct platform_d * Offset the registers base unconditionally to point to the non-secure * alias space for now. */ - mmu->base += IM_NS_ALIAS_OFFSET; + if (mmu->features->use_ns_alias_offset) + mmu->base += IM_NS_ALIAS_OFFSET; irq = platform_get_irq(pdev, 0); if (irq < 0) { @@ -1084,11 +1110,6 @@ static int ipmmu_remove(struct platform_ return 0; } -static const struct of_device_id ipmmu_of_ids[] = { - { .compatible = "renesas,ipmmu-vmsa", }, - { } -}; - static struct platform_driver ipmmu_driver = { .driver = { .name = "ipmmu-vmsa",