From: hdoyu@nvidia•com (Hiroshi Doyu)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 4/6] iommu/arm-smmu: implement generic DT bindings
Date: Tue, 19 Aug 2014 18:54:52 +0300 [thread overview]
Message-ID: <87tx58h3ub.fsf@nvidia.com> (raw)
In-Reply-To: <20140819125449.GN23128@arm.com>
Will Deacon <will.deacon@arm•com> writes:
> [adding Rob, Mark, Arnd, Thierry and Hiroshi]
>
> On Wed, Aug 13, 2014 at 01:51:37AM +0100, Mitchel Humpherys wrote:
>> Generic IOMMU device tree bindings were recently added in
>> ["devicetree: Add generic IOMMU device tree bindings"]. Implement the
>> bindings in the ARM SMMU driver.
>>
>> See Documentation/devicetree/bindings/iommu/iommu.txt for the bindings
>> themselves.
>
> Could you look at moving the parsing code into a separate file please (maybe
> under lib/ ?). That way, other IOMMUs can use the same binding without the
> boilerplate having to be rewritten each time.
>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 63c6707fad..22e25f3172 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -538,25 +538,32 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
>> return 0;
>> }
>>
>> +struct iommus_entry {
>> + struct list_head list;
>> + struct device_node *node;
>> + u16 streamids[MAX_MASTER_STREAMIDS];
>> + int num_sids;
>> +};
>> +
>> static int register_smmu_master(struct arm_smmu_device *smmu,
>> - struct device *dev,
>> - struct of_phandle_args *masterspec)
>> + struct iommus_entry *entry)
>> {
>> int i;
>> struct arm_smmu_master *master;
>> + struct device *dev = smmu->dev;
>>
>> - master = find_smmu_master(smmu, masterspec->np);
>> + master = find_smmu_master(smmu, entry->node);
>> if (master) {
>> dev_err(dev,
>> "rejecting multiple registrations for master device %s\n",
>> - masterspec->np->name);
>> + entry->node->name);
>> return -EBUSY;
>> }
>>
>> - if (masterspec->args_count > MAX_MASTER_STREAMIDS) {
>> + if (entry->num_sids > MAX_MASTER_STREAMIDS) {
>> dev_err(dev,
>> "reached maximum number (%d) of stream IDs for master device %s\n",
>> - MAX_MASTER_STREAMIDS, masterspec->np->name);
>> + MAX_MASTER_STREAMIDS, entry->node->name);
>> return -ENOSPC;
>> }
>>
>> @@ -564,15 +571,58 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
>> if (!master)
>> return -ENOMEM;
>>
>> - master->of_node = masterspec->np;
>> - master->cfg.num_streamids = masterspec->args_count;
>> + master->of_node = entry->node;
>> + master->cfg.num_streamids = entry->num_sids;
>>
>> for (i = 0; i < master->cfg.num_streamids; ++i)
>> - master->cfg.streamids[i] = masterspec->args[i];
>> + master->cfg.streamids[i] = entry->streamids[i];
>>
>> return insert_smmu_master(smmu, master);
>> }
>>
>> +static int arm_smmu_parse_iommus_properties(struct arm_smmu_device *smmu,
>> + int *num_masters)
>> +{
>> + struct of_phandle_args iommuspec;
>> + struct device_node *dn;
>> +
>> + for_each_node_with_property(dn, "iommus") {
>> + int arg_ind = 0;
>> + struct iommus_entry *entry, *n;
>> + LIST_HEAD(iommus);
You may want to use the macro, "of_property_for_each_phandle_with_args()"
to parse "iommus=".
https://patchwork.ozlabs.org/patch/354073/
>> +
>> + while (!of_parse_phandle_with_args(dn, "iommus", "#iommu-cells",
>> + arg_ind, &iommuspec)) {
>
> We need to check that the phandle does indeed point at one of our SMMUs
> here, in case we have a system with multiple IOMMU types, all using the
> generic binding.
>
>> + int i;
>> +
>> + list_for_each_entry(entry, &iommus, list)
>> + if (entry->node == dn)
>> + break;
>
> Oh, yuck, this is really nasty to parse...
>
>> + if (&entry->list == &iommus) {
>
> Where is entry initialised the first time around?
>
>> + entry = devm_kzalloc(smmu->dev, sizeof(*entry),
>> + GFP_KERNEL);
>> + if (!entry)
>> + return -ENOMEM;
>
> You need to of_node_put the guys you've got back from the phandle parsing
> code.
>
>> + entry->node = dn;
>> + list_add(&entry->list, &iommus);
>> + }
>> + entry->num_sids = iommuspec.args_count;
>> + for (i = 0; i < entry->num_sids; ++i)
>> + entry->streamids[i] = iommuspec.args[i];
>> + arg_ind++;
>
> Isn't this defined by #iommu-cells?
>
>> + }
>> +
>> + list_for_each_entry_safe(entry, n, &iommus, list) {
>
> Why the _safe variant? This is a local list, right?
>
> Will
next prev parent reply other threads:[~2014-08-19 15:54 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-13 0:51 [PATCH 0/6] iommu/arm-smmu: misc features, new DT bindings Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 1/6] iommu/arm-smmu: add support for specifying clocks Mitchel Humpherys
2014-08-13 21:07 ` Mitchel Humpherys
2014-08-19 12:58 ` Will Deacon
2014-08-19 19:03 ` Olav Haugan
2014-08-26 14:27 ` Will Deacon
2014-09-10 1:29 ` Mitchel Humpherys
2014-09-10 18:27 ` Will Deacon
2014-09-10 19:09 ` Mitchel Humpherys
2014-09-15 18:38 ` Mitchel Humpherys
2014-08-19 19:28 ` Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 2/6] iommu/arm-smmu: add support for specifying regulators Mitchel Humpherys
2014-08-13 21:17 ` Mitchel Humpherys
2014-08-19 13:00 ` Will Deacon
2014-08-13 0:51 ` [PATCH 3/6] iommu/arm-smmu: add support for iova_to_phys through ATS1PR Mitchel Humpherys
2014-08-19 12:44 ` Will Deacon
2014-08-19 18:12 ` Mitchel Humpherys
2014-08-26 13:54 ` Will Deacon
2014-09-01 16:15 ` Will Deacon
2014-08-13 0:51 ` [PATCH 4/6] iommu/arm-smmu: implement generic DT bindings Mitchel Humpherys
2014-08-13 16:53 ` Mitchel Humpherys
2014-08-19 12:54 ` Will Deacon
2014-08-19 15:54 ` Hiroshi Doyu [this message]
2014-08-20 3:18 ` Arnd Bergmann
2014-08-13 0:51 ` [PATCH 5/6] iommu/arm-smmu: support buggy implementations with invalidate-on-map Mitchel Humpherys
2014-11-12 18:26 ` Will Deacon
2014-11-12 18:58 ` Mitchel Humpherys
2014-11-13 9:48 ` Will Deacon
2014-11-14 23:08 ` Mitchel Humpherys
2014-08-13 0:51 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set, get}_attr for coherent walk control Mitchel Humpherys
2014-08-19 12:48 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set,get}_attr " Will Deacon
2014-08-19 19:19 ` [PATCH 6/6] iommu/arm-smmu: add .domain_{set, get}_attr " Mitchel Humpherys
2014-08-13 17:22 ` [PATCH 0/6] iommu/arm-smmu: misc features, new DT bindings Mitchel Humpherys
2014-08-15 17:25 ` Will Deacon
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=87tx58h3ub.fsf@nvidia.com \
--to=hdoyu@nvidia$(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