From: Robin Murphy <robin.murphy@arm.com> To: Stefano Stabellini <sstabellini@kernel.org>, will@kernel.org Cc: linux-arm-kernel@lists.infradead.org, michal.simek@xilinx.com, git@xilinx.com, linux-kernel@vger.kernel.org, Stefano Stabellini <stefano.stabellini@xilinx.com> Subject: Re: [PATCH] arm-smmu: check for generic bindings first Date: Wed, 28 Aug 2019 19:55:25 +0100 [thread overview] Message-ID: <f6fe31f1-52e1-8ab8-4ba4-f23db262d1b5@arm.com> (raw) In-Reply-To: <20190828173837.29617-1-sstabellini@kernel.org> On 28/08/2019 18:38, Stefano Stabellini wrote: > From: Stefano Stabellini <stefano.stabellini@xilinx.com> > > Today, the arm-smmu driver checks for mmu-masters on device tree, the > legacy property, if it is absent it assumes that we are using the new > bindings. If it is present it assumes that we are using the legacy > bindings. All arm-smmus need to use the same bindings: legacy or new. > > There are two issues with this: > > - we are not actually checking for the new bindings explicitly > It would be better to have an explicit check for the new bindings rather > than assuming we must be using the new if the old are not there. The legacy binding is the special case, though. Isn't it perfectly logical to check for the special case, and assume the normal case otherwise? ;) > - old and new bindings cannot coexist > It would be nice to be able to provide both old and new bindings so > that software that hasn't been updated yet is still able to get IOMMU > information from the legacy bindings while at the same time newer > software can get the latest information via the new bindings. (Xen has > not been updated to use the new binding yet for instance.) The current > code breaks under these circumstances because if the old bindings are > present, the new are not even checked. FWIW that was a deliberate design decision. We didn't want to see DTs with both bindings at once - the legacy binding support remains only for the sake of DTBs deployed in fossilised firmware that can't/won't ever be updated. And I'm pretty sure the intervening 3 years have only reinforced that position. Only a couple more months now until it will have been deprecated for longer than it wasn't :) Do note that "mmu-masters" isn't just deprecated for giggles either - the binding made some flawed assumptions, and there are Stream ID topologies that it cannot possibly describe, which do exist in the real world. > This patch changes the scheme by checking for #iommu-cells, which is > only present with the new bindings, before checking for mmu-masters. > The new bindings are always favored when present. All SMMUs still need > to use the same bindings: mix-and-matching new and old bindings between > different SMMUs is not allowed. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> > --- > > Let me know if you'd like me to turn the two using_*_binding variables > into a single one. > > Also, please note that this is not meant as an excuse not to get Xen > updated to use the new binding. Fair enough, but conversely, what makes it Linux's responsibility to handle? If Xen wants to deal with funky hybrid DTs then why shouldn't it be Xen's job to just filter deprecated properties out of whatever it presents to Linux? Robin. > drivers/iommu/arm-smmu.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 64977c131ee6..79b518ff215c 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -2118,7 +2118,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, > { > const struct arm_smmu_match_data *data; > struct device *dev = &pdev->dev; > - bool legacy_binding; > + bool legacy_binding, generic_binding; > > if (of_property_read_u32(dev->of_node, "#global-interrupts", > &smmu->num_global_irqs)) { > @@ -2132,16 +2132,20 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, > > parse_driver_options(smmu); > > - legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL); > - if (legacy_binding && !using_generic_binding) { > - if (!using_legacy_binding) > - pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n"); > - using_legacy_binding = true; > - } else if (!legacy_binding && !using_legacy_binding) { > + generic_binding = of_find_property(dev->of_node, "#iommu-cells", NULL); > + if (generic_binding && !using_legacy_binding) { > using_generic_binding = true; > } else { > - dev_err(dev, "not probing due to mismatched DT properties\n"); > - return -ENODEV; > + legacy_binding = of_find_property(dev->of_node, "mmu-masters", > + NULL); > + if (legacy_binding && !using_generic_binding) { > + if (!using_legacy_binding) > + pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n"); > + using_legacy_binding = true; > + } else { > + dev_err(dev, "not probing due to mismatched DT properties\n"); > + return -ENODEV; > + } > } > > if (of_dma_is_coherent(dev->of_node)) >
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com> To: Stefano Stabellini <sstabellini@kernel.org>, will@kernel.org Cc: linux-kernel@vger.kernel.org, Stefano Stabellini <stefano.stabellini@xilinx.com>, michal.simek@xilinx.com, linux-arm-kernel@lists.infradead.org, git@xilinx.com Subject: Re: [PATCH] arm-smmu: check for generic bindings first Date: Wed, 28 Aug 2019 19:55:25 +0100 [thread overview] Message-ID: <f6fe31f1-52e1-8ab8-4ba4-f23db262d1b5@arm.com> (raw) In-Reply-To: <20190828173837.29617-1-sstabellini@kernel.org> On 28/08/2019 18:38, Stefano Stabellini wrote: > From: Stefano Stabellini <stefano.stabellini@xilinx.com> > > Today, the arm-smmu driver checks for mmu-masters on device tree, the > legacy property, if it is absent it assumes that we are using the new > bindings. If it is present it assumes that we are using the legacy > bindings. All arm-smmus need to use the same bindings: legacy or new. > > There are two issues with this: > > - we are not actually checking for the new bindings explicitly > It would be better to have an explicit check for the new bindings rather > than assuming we must be using the new if the old are not there. The legacy binding is the special case, though. Isn't it perfectly logical to check for the special case, and assume the normal case otherwise? ;) > - old and new bindings cannot coexist > It would be nice to be able to provide both old and new bindings so > that software that hasn't been updated yet is still able to get IOMMU > information from the legacy bindings while at the same time newer > software can get the latest information via the new bindings. (Xen has > not been updated to use the new binding yet for instance.) The current > code breaks under these circumstances because if the old bindings are > present, the new are not even checked. FWIW that was a deliberate design decision. We didn't want to see DTs with both bindings at once - the legacy binding support remains only for the sake of DTBs deployed in fossilised firmware that can't/won't ever be updated. And I'm pretty sure the intervening 3 years have only reinforced that position. Only a couple more months now until it will have been deprecated for longer than it wasn't :) Do note that "mmu-masters" isn't just deprecated for giggles either - the binding made some flawed assumptions, and there are Stream ID topologies that it cannot possibly describe, which do exist in the real world. > This patch changes the scheme by checking for #iommu-cells, which is > only present with the new bindings, before checking for mmu-masters. > The new bindings are always favored when present. All SMMUs still need > to use the same bindings: mix-and-matching new and old bindings between > different SMMUs is not allowed. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> > --- > > Let me know if you'd like me to turn the two using_*_binding variables > into a single one. > > Also, please note that this is not meant as an excuse not to get Xen > updated to use the new binding. Fair enough, but conversely, what makes it Linux's responsibility to handle? If Xen wants to deal with funky hybrid DTs then why shouldn't it be Xen's job to just filter deprecated properties out of whatever it presents to Linux? Robin. > drivers/iommu/arm-smmu.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 64977c131ee6..79b518ff215c 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -2118,7 +2118,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, > { > const struct arm_smmu_match_data *data; > struct device *dev = &pdev->dev; > - bool legacy_binding; > + bool legacy_binding, generic_binding; > > if (of_property_read_u32(dev->of_node, "#global-interrupts", > &smmu->num_global_irqs)) { > @@ -2132,16 +2132,20 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, > > parse_driver_options(smmu); > > - legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL); > - if (legacy_binding && !using_generic_binding) { > - if (!using_legacy_binding) > - pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n"); > - using_legacy_binding = true; > - } else if (!legacy_binding && !using_legacy_binding) { > + generic_binding = of_find_property(dev->of_node, "#iommu-cells", NULL); > + if (generic_binding && !using_legacy_binding) { > using_generic_binding = true; > } else { > - dev_err(dev, "not probing due to mismatched DT properties\n"); > - return -ENODEV; > + legacy_binding = of_find_property(dev->of_node, "mmu-masters", > + NULL); > + if (legacy_binding && !using_generic_binding) { > + if (!using_legacy_binding) > + pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n"); > + using_legacy_binding = true; > + } else { > + dev_err(dev, "not probing due to mismatched DT properties\n"); > + return -ENODEV; > + } > } > > if (of_dma_is_coherent(dev->of_node)) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-08-28 18:55 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-28 17:38 [PATCH] arm-smmu: check for generic bindings first Stefano Stabellini 2019-08-28 17:38 ` Stefano Stabellini 2019-08-28 18:55 ` Robin Murphy [this message] 2019-08-28 18:55 ` Robin Murphy 2019-08-28 20:48 ` Stefano Stabellini 2019-08-28 20:48 ` Stefano Stabellini
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=f6fe31f1-52e1-8ab8-4ba4-f23db262d1b5@arm.com \ --to=robin.murphy@arm.com \ --cc=git@xilinx.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=michal.simek@xilinx.com \ --cc=sstabellini@kernel.org \ --cc=stefano.stabellini@xilinx.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.