From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5ABC5C433E0 for ; Fri, 5 Mar 2021 16:11:00 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DB81765016 for ; Fri, 5 Mar 2021 16:10:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DB81765016 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=zhTOcfa7K6lLCLEnMB8MbyzMGzujBwM/UaGg0j02Vts=; b=dj6br/MA9lI1rcIhCgmSPighC tS88n3RJOyVj6+M2Wl36lfsawodynQE9Lcbm81mDUw0y1xMPCzS0GEzJxzEGkmloCcyr1MzobJe18 LVvAHDxoouI/oujl6uZNILT06txOfnZHxkJLHhavmsLK8KimboZN8KQ1nBEY5Yysz9fd15Jd3NONa L6Nx5ISXA4gGK7sKbywHXCmqhjYGTczPgGtvNhAGMo1wLJAmiee3IbucvyB2fyYaL6SfGkt2sRmd+ lNf0Huu8q9c3frgJNmSOh0EO54ejdgwEACtqxlblNXnyIYzUy1TFmNb71i4zIvO044xmoN1DQQBC2 6bd9B6x/g==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lID0W-00FZGl-8G; Fri, 05 Mar 2021 16:08:54 +0000 Received: from mail-pj1-x1032.google.com ([2607:f8b0:4864:20::1032]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lID0P-00FZED-7R for linux-arm-kernel@lists.infradead.org; Fri, 05 Mar 2021 16:08:47 +0000 Received: by mail-pj1-x1032.google.com with SMTP id h13so2336850pjt.0 for ; Fri, 05 Mar 2021 08:08:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=vzJbcn2Bf5oIu0/VNNTub9S9qN/KmbJZUnNKqWz1BGo=; b=rroJgfmQ2M/jLVa81ZRwdlOpZlc/i58e/utvcm+/9AmUuAkiK5M7N1AqYMbmsQcv1p KDydQUJGmINsBe/lgKRJFbnRl11l6g0ZBfDWJFlwUM81q36D4crLAORsUeTm5NXlCOMj Jnz6qps8GrCSfrNMi4kvIIvtCiTXfendATiKWpTTuLWJHZsKQi/V0tYOE2WVGC9Jkd/0 D6y2d3HEtTwbL0vPkmcScw2DUFzr+t5WHgBHRrVm7HQzddOMcueTHZoChTsd1Kig4B9U L0hr5NRhPfQSdNCl4II0uLIE6wJQ4ZUGmuLsYHyHTtTBuar+GM8phhfh3a6NrFL1jazX 5zXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=vzJbcn2Bf5oIu0/VNNTub9S9qN/KmbJZUnNKqWz1BGo=; b=QA2Rww8/To3GSXF+uuNM5sp/2XeJIrbbA7502xnF37nPyc7wJpuVKuR87GFVqdwKP1 ks9Jc/ArmCKnCZvQdJi4E0hjlmiFTwYCDOPhJOyeQLIYMkQFEQ2Ow25vmFKFnT9/NIll YPS4eCyhHbMdxnsmaK+nW3z1Azk5Gj7o+/7X0GWLnfMq4m79BXF9zZNXsMLa2nJuezAG usNyUZOv3Epkmb5BN5TUn0RxJTeTnMk6gaTQKC/51MgblzCOMC8AunTQChiEBd3U7BAY rn4PjClYC87m/yZlEyRqhqvYW4jsSwZ9c3YD8bDik2NTIDKUlV9Laa+pN5YuBZ68uKIs yghQ== X-Gm-Message-State: AOAM532+i3eTxwnli4LM0Bj1mXntKAzUbPMuPLGrb07BYVPHml90WSHV dGJ2Y1Y4SPrlNyviylakrThV7iSQtUKTPAhKs+k= X-Google-Smtp-Source: ABdhPJzLoPhSeJGxlr3MgS/Wnn4G2jekmS6UM0SXi3AaYSt6vBAtWIJWjPFM+bWm2F/zbWUX+E55FpdwKOjY9W6AbMU= X-Received: by 2002:a17:90a:db49:: with SMTP id u9mr11418889pjx.181.1614960523418; Fri, 05 Mar 2021 08:08:43 -0800 (PST) MIME-Version: 1.0 References: <20210304213902.83903-1-marcan@marcan.st> <20210304213902.83903-13-marcan@marcan.st> <04ea35d6-cd7d-d6de-75ae-59b1e0c77f04@marcan.st> In-Reply-To: <04ea35d6-cd7d-d6de-75ae-59b1e0c77f04@marcan.st> From: Andy Shevchenko Date: Fri, 5 Mar 2021 18:08:27 +0200 Message-ID: Subject: Re: [RFT PATCH v3 12/27] of/address: Add infrastructure to declare MMIO as non-posted To: Hector Martin Cc: linux-arm Mailing List , Marc Zyngier , Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , Mark Rutland , Greg Kroah-Hartman , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree , "open list:SERIAL DRIVERS" , Linux Documentation List , Linux Samsung SOC , Linux-Arch , Linux Kernel Mailing List X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210305_160845_558198_C94D90E7 X-CRM114-Status: GOOD ( 42.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 5, 2021 at 5:55 PM Hector Martin wrote: > On 06/03/2021 00.13, Andy Shevchenko wrote: ... > >> - return ioremap(res.start, resource_size(&res)); > >> + if (res.flags & IORESOURCE_MEM_NONPOSTED) > >> + return ioremap_np(res.start, resource_size(&res)); > >> + else > >> + return ioremap(res.start, resource_size(&res)); > > > > This doesn't sound right. Why _np is so exceptional? Why don't we have > > other flavours (it also rings a bell to my previous comment that the > > flag in ioresource is not in the right place)? > > This is different from other variants, because until now *drivers* have > made the choice of what ioremap mode to use based on device requirements > (which means ioremap() 99% of the time, and then framebuffers and other > memory-ish things such use something else). Now we have a *SoC fabric* > that is calling the shots on what ioremap mode we have to use - and > *every* non-PCIe driver needs to use ioremap_np() on these SoCs, or they > break. So it seems a lot cleaner to make the choice for drivers here to > upgrade ioremap() to ioremap_np() for SoCs that need it. Yes, that is a good idea. Once we discussed x86 and _uc cases and actually on x86 it makes a lot of sense to have ioremap() == ioremap_uc(). Can't be this a similar case here? Arnd, what do you think of actually providing an ioremap() as some kind of "best for the architecture the code is running on"? Otherwise if the same driver happens to be needed on different architectures, oops, ifdeffery or simple conditionals over the code is really not the best way to solve it. > If we don't do something like this here or in otherwise common code, > we'd have to have an open-coded "if apple then ioremap_np, else ioremap" > in every driver that runs on-die devices on these SoCs, even ones that > are otherwise standard and need few or no Apple-specific quirks. Exactly! But what about architectures where _uc is that one? So, why does your patch only take part of _np case? (Hint we have x86 Device Tree based platforms) > We're still going to have to patch some drivers to use managed APIs that > can properly hit this conditional (like I did for samsung_tty) in cases > where they currently don't, but that's a lot cleaner than an open-coded > conditional, I think (and often comes with other benefits anyway). > > Note that wholesale making ioremap() behave like ioremap_np() at the > arch level as as SoC quirk is not an option - for extenal PCIe devices, > we still need to use ioremap(). We tried this approach initially but it > doesn't work. Hence we arrived at this solution which describes the > required mode in the devicetree, at the bus level (which makes sense, > since that represents the fabric), and then these wrappers can use that > information, carried over via the bit in struct device, to pick the > right ioremap mode. > > It doesn't really make sense to include the other variants here, because > _np is strictly stronger than the default. Downgrading ioremap to any > other variant would break most drivers, badly. However, upgrading to > ioremap_np() is always correct (if possibly slower), on platforms where > it is allowed by the bus. In fact, I bet that on many systems nGnRE > already behaves like nGnRnE anyway. I don't know why Apple didn't just > allow nGnRE mappings to work (behaving like nGnRnE) instead of making > them explode, which is the whole reason we have to do this. Yep, and why not to make ioremap() == ioremap_nc() on architecture that requires it? Can it be detected at run time? ... > >> + while (node) { > >> + if (!of_property_read_bool(node, "ranges")) { > >> + break; > >> + } else if (of_property_read_bool(node, "nonposted-mmio")) { > >> + of_node_put(node); > >> + return true; > >> + } else if (of_property_read_bool(node, "posted-mmio")) { > >> + break; > >> + } > >> + parent = of_get_parent(node); > >> + of_node_put(node); > >> + node = parent; > >> + } > > > > I believe above can be slightly optimized. Don't we have helpers to > > traverse to all parents? > > Keep in mind the logic here is that it stops on the first instance of > either property, and does not traverse non-translatable boundaries. Are > there helpers that can implement this kind of complex logic? It's not a > simple recursive property lookup. I am aware of what it does and I believe if we don't have such a helper yet we may introduce it and maybe even existing users of something similar can utilize it. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel