Re: [PATCH v3 01/09] iommu/ipmmu-vmsa: Introduce features, break out alias

From: Robin Murphy
Date: Wed Mar 08 2017 - 07:20:08 EST


Hi Magnus,

On 08/03/17 11:01, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> ---
>
> 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",
>