public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCHv7 08/12] iommu/tegra: smmu: calculate ASID register offset by ID
Date: Mon, 16 Dec 2013 12:02:42 -0700	[thread overview]
Message-ID: <52AF4E52.9010000@wwwdotorg.org> (raw)
In-Reply-To: <1386835033-4701-9-git-send-email-hdoyu@nvidia.com>

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> ASID register offset is caclulated by SWGROUP ID so that we can get
> rid of old SoC specific MACROs. This ID conversion is needed for the
> unified SMMU driver over Tegra SoCs. We use dt-bindings MACRO instead
> of SoC dependent MACROs. The formula is:
> 
>   MC_SMMU_<swgroup name>_ASID_0 = MC_SMMU_AFI_ASID_0 + ID * 4;
> 
> Now SWGROUP ID is the global HardWare Accelerator(HWA) identifier
> among all Tegra SoC except Tegra2.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> -				   unsigned long map, int on)
> +				   unsigned long *map, int on)
>  {
>  	int i;
>  	struct smmu_as *as = c->as;
>  	u32 val, offs, mask = SMMU_ASID_ENABLE(as->asid);
>  	struct smmu_device *smmu = as->smmu;
>  
>  	if (!on)
> +		map = c->hwgrp;
>  
> +	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
>  		offs = HWGRP_ASID_REG(i);
>  		val = smmu_read(smmu, offs);
>  		if (on) {
>  			if (WARN_ON(val & mask))
>  				goto err_hw_busy;
>  			val |= mask;
> +			memcpy(c->hwgrp, map, sizeof(u64));
>  		} else {
>  			WARN_ON((val & mask) == mask);
>  			val &= ~mask;
>  		}
>  		smmu_write(smmu, val, offs);
>  	}

This function doesn't make a lot of sense to me. The registers it's
manipulating aren't a bitmask, so I don't see why the code is performing
bitmask AND/OR operations on the register. Instead, don't you want the
following:

#define SMMU_ASID_ENABLE	(1 << 31)
#define SMMU_ASID_ENABLED(asid)	(SMMU_ASID_ENABLE | asid)
#define SMMU_ASID_DISABLE	0

static int __smmu_client_set_hwgrp(struct smmu_client *c,
				   unsigned long *map, int on)
{
	int i;
	struct smmu_as *as = c->as;
	u32 val, offs, new_val;
	struct smmu_device *smmu = as->smmu;

	if (!on)
		map = c->hwgrp;

	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
		offs = HWGRP_ASID_REG(i);
		val = smmu_read(smmu, offs);
		if (on)
			new_val = SMMU_ASID_ENABLED(as->asid);
		else
			new_val = 0;
		WARN_ON(val & SMMU_ASID_ENABLE ==
			new_val & SMMU_ASID_ENABLE);
		if (on) {
			if (val & SMMU_ASID_ENABLE)
				goto err_hw_busy;
			memcpy(c->hwgrp, map, sizeof(u64));
		}
		smmu_write(smmu, val, offs);
	}

> @@ -804,7 +727,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
>  		return -ENOMEM;
>  	client->dev = dev;
>  	client->as = as;
> -	map = (unsigned long)dev->platform_data;
> +	map = (unsigned long *)dev->platform_data;
>  	if (!map)
>  		return -EINVAL;

Presumably we can simply delete that now since everything using this
driver is on DT; we can't rely on the platform_data of client struct
devices...

  reply	other threads:[~2013-12-16 19:02 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-12  7:57 [PATCHv7 00/12] Unifying SMMU driver among Tegra SoCs Hiroshi Doyu
2013-12-12  7:57 ` [PATCHv7 01/12] of: introduce of_property_for_each_phandle_with_args() Hiroshi Doyu
2013-12-16 18:29   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 02/12] iommu/of: introduce a global iommu device list Hiroshi Doyu
2013-12-16 18:32   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 03/12] iommu/of: check if dependee iommu is ready or not Hiroshi Doyu
2013-12-16 18:34   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs Hiroshi Doyu
2013-12-12 11:39   ` Grant Likely
2013-12-13  2:14     ` Greg KH
2013-12-14 12:24       ` Thierry Reding
2013-12-14 14:28         ` Hiroshi Doyu
2013-12-16 18:26       ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 05/12] iommu/core: add ops->{bound,unbind}_driver() Hiroshi Doyu
2013-12-16 18:42   ` Stephen Warren
2013-12-30 13:45   ` Joerg Roedel
2013-12-12  7:57 ` [PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP ID Hiroshi Doyu
2013-12-18  8:02   ` Mark Zhang
2013-12-18 16:27     ` Stephen Warren
2013-12-20 12:35       ` Thierry Reding
2013-12-20 17:36         ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 07/12] iommu/tegra: smmu: register device to iommu dynamically Hiroshi Doyu
2013-12-16 18:46   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 08/12] iommu/tegra: smmu: calculate ASID register offset by ID Hiroshi Doyu
2013-12-16 19:02   ` Stephen Warren [this message]
2013-12-12  7:57 ` [PATCHv7 09/12] iommu/tegra: smmu: get swgroups from DT "iommus=" Hiroshi Doyu
2013-12-16 19:09   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 10/12] iommu/tegra: smmu: allow duplicate ASID wirte Hiroshi Doyu
2013-12-16 19:19   ` Stephen Warren
2013-12-12  7:57 ` [PATCHv7 11/12] iommu/tegra: smmu: Rename hwgrp -> swgroups Hiroshi Doyu
2013-12-12  7:57 ` [PATCHv7 12/12] iommu/tegra: smmu: add SMMU to an global iommu list Hiroshi Doyu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52AF4E52.9010000@wwwdotorg.org \
    --to=swarren@wwwdotorg$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox