Received: by 2002:a05:6902:102b:0:0:0:0 with SMTP id x11csp3514260ybt; Tue, 23 Jun 2020 04:19:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwcXEPVvG7gxZ7WvT7tKN/tSqu2O3tVg1NZTooUNSXQa/uHDHZzPQqC+k1a1pVSonJW5OO0 X-Received: by 2002:a17:906:19c8:: with SMTP id h8mr20735557ejd.512.1592911152235; Tue, 23 Jun 2020 04:19:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1592911152; cv=none; d=google.com; s=arc-20160816; b=XdQUuCiLCy07oNVuyOYq6xJ345HZ6fOncOnU8tQGf7AFfrJdm4QfcOahJqlM5LhlWu cqJSGcNoIT0YoEysZabBw8cKvOdowATjLHZirKJRvGKTkdEHwH4R0gaykA0zlm6Kpzca 89r4vKHwl6TLwvoxPCInzoO/WTkRrm/EMvBzUOcKAAeJ0QPcuwfaih4BJJ1LbbKl0DbA ZAmsQoIMF33Nc+pWFZCarE7orpQ8utNNFA4VqGJA94StOXgeetIL0jamJp9jw6Uq60eB ZJBcAyOq4x9mXpUiwgc1MnAphZKvVp2i+I7K68bDKh/Z8a20zxaZTI8Us3L1uyiwOldF M9gw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=VdX7ci4e6O+jgy8xPFcWBxW5OYGiV3gQpf5LL0WLVFo=; b=LndoP1uyIiHlp/7MIw+k6kXOYG9UJdl7n0CFDtHwLjxum58+9lD55DJrEViwabyRdv xLlsa0k2LdJuAXqYY6d8gUzl8fyDeoePY2c/ntAwwtclbV8zEHYbRXPw+YZfyAQ1Lf0p PVfQLIWXYiH5UrdJLkE8l/Zmhe653Ps+0qMznEfEgYOBNV2+gooamODYOGsZUoZWhQXU eQKK/cHCLhP9h8Px/I2cwsNzDxXjOz26nLBG0+TMVMPHv5QYEMnpo4hvAzA+6j6fG6xN WkqXpwp5b0D4kwKnKekYKSw43VoUqoqd8mmOgR7zL6o/pfD3+01o0BiNc4eYItG8GTXr VEEQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id d16si10241654ejp.131.2020.06.23.04.18.49; Tue, 23 Jun 2020 04:19:12 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732191AbgFWLRD (ORCPT + 99 others); Tue, 23 Jun 2020 07:17:03 -0400 Received: from foss.arm.com ([217.140.110.172]:57500 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732458AbgFWLRB (ORCPT ); Tue, 23 Jun 2020 07:17:01 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BAB441F1; Tue, 23 Jun 2020 04:16:58 -0700 (PDT) Received: from [10.57.9.128] (unknown [10.57.9.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3BD7E3F6CF; Tue, 23 Jun 2020 04:16:56 -0700 (PDT) Subject: Re: [PATCH v6 1/4] iommu/arm-smmu: add NVIDIA implementation for dual ARM MMU-500 usage To: Thierry Reding , Krishna Reddy Cc: treding@nvidia.com, bhuntsman@nvidia.com, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, mperttunen@nvidia.com, talho@nvidia.com, snikam@nvidia.com, nicolinc@nvidia.com, linux-tegra@vger.kernel.org, yhsu@nvidia.com, praithatha@nvidia.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, bbiswas@nvidia.com References: <20200604234414.21912-1-vdumpa@nvidia.com> <20200604234414.21912-2-vdumpa@nvidia.com> <20200623102927.GD4098287@ulmo> From: Robin Murphy Message-ID: <5f29c794-406a-db13-d6d0-75dcb0d0b0cc@arm.com> Date: Tue, 23 Jun 2020 12:16:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <20200623102927.GD4098287@ulmo> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2020-06-23 11:29, Thierry Reding wrote: [...] >> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c >> index c75b9d957b702..52c84c30f83e4 100644 >> --- a/drivers/iommu/arm-smmu-impl.c >> +++ b/drivers/iommu/arm-smmu-impl.c >> @@ -160,6 +160,9 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) >> */ >> switch (smmu->model) { >> case ARM_MMU500: >> + if (of_device_is_compatible(smmu->dev->of_node, >> + "nvidia,tegra194-smmu-500")) >> + return nvidia_smmu_impl_init(smmu); > > Should NVIDIA_TEGRA194_SMMU be a separate value for smmu->model, > perhaps? That way we avoid this somewhat odd check here. No, this is simply in the wrong place. The design here is that we pick up anything related to the basic SMMU IP (model) first, then make any platform-specific integration checks. That way a platform-specific init function can see the model impl set and subclass it if necessary (although nobody's actually done that yet). The setup for Cavium is just a short-cut since their model is unique to their integration, so the lines get a bit blurred and there's little benefit to trying to separate it out. In short, put this down below with the other of_device_is_compatible() checks. >> smmu->impl = &arm_mmu500_impl; >> break; >> case CAVIUM_SMMUV2: >> diff --git a/drivers/iommu/arm-smmu-nvidia.c b/drivers/iommu/arm-smmu-nvidia.c > > I wonder if it would be better to name this arm-smmu-tegra.c to make it > clearer that this is for a Tegra chip. We do have regular expressions in > MAINTAINERS that catch anything with "tegra" in it to make this easier. There was a notion that these would be grouped by vendor, but if there's a strong preference for all NVIDIA-SoC-related stuff to be named "Tegra" then I'm not going to complain too much. >> new file mode 100644 >> index 0000000000000..dafc293a45217 >> --- /dev/null >> +++ b/drivers/iommu/arm-smmu-nvidia.c >> @@ -0,0 +1,161 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +// Nvidia ARM SMMU v2 implementation quirks > > s/Nvidia/NVIDIA/ > >> +// Copyright (C) 2019 NVIDIA CORPORATION. All rights reserved. > > I suppose this should now also include 2020. > >> + >> +#define pr_fmt(fmt) "nvidia-smmu: " fmt > > Same here. Might be worth making this "tegra-smmu: " for consistency. On the other hand, a log prefix that is literally the name of a completely unrelated driver seems more confusing to users than useful. Same for the function naming - the tegra_smmu_* namespace is already owned by that driver. Robin.