public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: robin.murphy@arm•com (Robin Murphy)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 6/8] iommu/arm-smmu: Implement of_xlate() for SMMUv3
Date: Fri, 15 Jul 2016 19:27:04 +0100	[thread overview]
Message-ID: <57892AF8.3020103@arm.com> (raw)
In-Reply-To: <20160715135506.GA4687@red-moon>

On 15/07/16 14:55, Lorenzo Pieralisi wrote:
> On Fri, Jul 01, 2016 at 05:50:15PM +0100, Robin Murphy wrote:
> 
> [...]
> 
>> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>> +{
>> +	int ret;
>> +
>> +	/* We only support PCI, for now */
>> +	if (!dev_is_pci(dev))
>> +		return -ENODEV;
> 
> Given that a) the check above is removed in a later patch and b)
> code below does not depend on SMMU v3, I think the aim should
> be to make this a core function (ie I am asking this since I will
> need it in IORT based translation and I do not want to add yet another
> *_xlate hook to iommu_op), iommu_fwspec_xlate() ?

Indeed, this is only tied to OF by the current datatypes, and that's
straightforward to change. Ultimately the purpose is just for
firmware/bus code to pass in some words of configuration data, and the
driver to respond with what corresponding runtime data it wants to
associate with the device. As I suggested over on the fsl-mc discussion,
the caller might not even really be 'firmware' at all.

> What I will do with my next RFC is move the iommu_fwspec out of
> OF_IOMMU code in a separate compilation unit and we will take the
> discussion from there.

Sounds good. If the end result starts looking clear, it might be an idea
to squash some patches and skip this intermediate OF-specific step
entirely (I was just hesitant to do that myself without a clear view of
the IORT side).

>> +
>> +	ret = iommu_fwspec_init(dev, args->np);
>> +	if (!ret)
>> +		ret = iommu_fwspec_add_ids(dev, &args->args[0], 1);
>> +
>> +	return ret;
>> +}
>> +
>>  static struct iommu_ops arm_smmu_ops = {
>>  	.capable		= arm_smmu_capable,
>>  	.domain_alloc		= arm_smmu_domain_alloc,
>> @@ -1947,6 +1894,7 @@ static struct iommu_ops arm_smmu_ops = {
>>  	.device_group		= pci_device_group,
>>  	.domain_get_attr	= arm_smmu_domain_get_attr,
>>  	.domain_set_attr	= arm_smmu_domain_set_attr,
>> +	.of_xlate		= arm_smmu_of_xlate,
>>  	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
>>  };
>>  
>> @@ -2697,6 +2645,22 @@ static void __exit arm_smmu_exit(void)
>>  subsys_initcall(arm_smmu_init);
>>  module_exit(arm_smmu_exit);
>>  
>> +static int __init arm_smmu_of_init(struct device_node *np)
>> +{
>> +	static bool registered;
>> +
>> +	if (!registered)
>> +		registered = !arm_smmu_init();
> 
> We also need a static variable in arm_smmu_init() to make sure
> we do not try to execute it multiple times :( (here and
> subsys_initcall).

Strictly, yes, although since there didn't seem to be any real issue
with just letting the initcall fail when register_driver() detects the
collision, I'd hoped we might be able to keep this bodge together in one
place. I guess it might end up printing some unwanted failure message
though, so I'll take another look.

Thanks,
Robin.

> Thanks,
> Lorenzo
> 
>> +
>> +	if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
>> +		return -ENODEV;
>> +
>> +	of_iommu_set_ops(np, &arm_smmu_ops);
>> +
>> +	return 0;
>> +}
>> +IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
>> +
>>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>>  MODULE_AUTHOR("Will Deacon <will.deacon@arm•com>");
>>  MODULE_LICENSE("GPL v2");
>> -- 
>> 2.8.1.dirty
>>
> 

  reply	other threads:[~2016-07-15 18:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-01 16:50 [PATCH v4 0/8] Generic DT bindings for PCI IOMMUs and ARM SMMUv3 Robin Murphy
2016-07-01 16:50 ` [PATCH v4 1/8] arm64: mm: change IOMMU notifier action to attach DMA ops Robin Murphy
2016-07-08 14:55   ` Will Deacon
2016-07-08 17:06     ` Catalin Marinas
2016-07-01 16:50 ` [PATCH v4 2/8] Docs: dt: add PCI IOMMU map bindings Robin Murphy
2016-07-01 16:50 ` [PATCH v4 3/8] of/irq: Break out msi-map lookup (again) Robin Murphy
2016-07-07 16:51   ` Will Deacon
2016-07-18 17:54   ` Rob Herring
2016-07-01 16:50 ` [PATCH v4 4/8] iommu/of: Handle iommu-map property for PCI Robin Murphy
2016-07-01 16:50 ` [PATCH v4 5/8] iommu/of: Introduce iommu_fwspec Robin Murphy
2016-07-07 16:56   ` Lorenzo Pieralisi
2016-07-11 11:07     ` Robin Murphy
2016-07-01 16:50 ` [PATCH v4 6/8] iommu/arm-smmu: Implement of_xlate() for SMMUv3 Robin Murphy
2016-07-15 13:55   ` Lorenzo Pieralisi
2016-07-15 18:27     ` Robin Murphy [this message]
2016-07-29 14:46   ` Jean-Philippe Brucker
2016-07-29 18:55     ` Robin Murphy
2016-08-24 15:08       ` Robin Murphy
2016-07-01 16:50 ` [PATCH v4 7/8] iommu/arm-smmu: Support non-PCI devices with SMMUv3 Robin Murphy
2016-07-01 16:50 ` [PATCH v4 8/8] iommu/arm-smmu: Set PRIVCFG in stage 1 STEs Robin Murphy

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=57892AF8.3020103@arm.com \
    --to=robin.murphy@arm$(echo .)com \
    --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