lkml.org 
[lkml]   [2019]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] arm-smmu: check for generic bindings first
From
Date
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))
>

\
 
 \ /
  Last update: 2019-08-28 20:57    [W:0.091 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site