From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com> To: Arnd Bergmann <arnd@kernel.org> Cc: Russell King <rmk+kernel@arm.linux.org.uk>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Will Deacon <will.deacon@arm.com>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, Arnd Bergmann <arnd@arndb.de>, Rob Herring <robh+dt@kernel.org>, Wei Xu <xuwei5@hisilicon.com>, devicetree <devicetree@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, linux-kernel <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v6 4/4] ARM: Add support for Hisilicon Kunpeng L3 cache controller Date: Mon, 1 Feb 2021 19:38:56 +0800 [thread overview] Message-ID: <301c82eb-68ce-1a96-a0b8-d46a29bf6f36@huawei.com> (raw) In-Reply-To: <CAK8P3a0=Aj0Ss3xbgh1ELyB+4d94ybugbza_xUqW_=yVsMwEqg@mail.gmail.com> On 2021/2/1 16:31, Arnd Bergmann wrote: > On Mon, Feb 1, 2021 at 4:36 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >> Add support for the Hisilicon Kunpeng L3 cache controller as used with >> Kunpeng506 and Kunpeng509 SoCs. >> >> These Hisilicon SoCs support LPAE, so the physical addresses is wider than >> 32-bits, but the actual bit width does not exceed 36 bits. When the cache >> operation is performed based on the address range, the upper 30 bits of >> the physical address are recorded in registers L3_MAINT_START and >> L3_MAINT_END, and ignore the lower 6 bits cacheline offset. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > If you add one more thing: > >> +static void l3cache_maint_common(u32 range, u32 op_type) >> +{ >> + u32 reg; >> + >> + reg = readl_relaxed(l3_ctrl_base + L3_MAINT_CTRL); >> + reg &= ~(L3_MAINT_RANGE_MASK | L3_MAINT_TYPE_MASK); >> + reg |= range | op_type; >> + reg |= L3_MAINT_STATUS_START; >> + writel(reg, l3_ctrl_base + L3_MAINT_CTRL); >> + >> + /* Wait until the hardware maintenance operation is complete. */ >> + do { >> + cpu_relax(); >> + reg = readl(l3_ctrl_base + L3_MAINT_CTRL); >> + } while ((reg & L3_MAINT_STATUS_MASK) != L3_MAINT_STATUS_END); >> +} >> + >> +static void l3cache_maint_range(phys_addr_t start, phys_addr_t end, u32 op_type) >> +{ >> + start = start >> L3_CACHE_LINE_SHITF; >> + end = ((end - 1) >> L3_CACHE_LINE_SHITF) + 1; >> + >> + writel_relaxed(start, l3_ctrl_base + L3_MAINT_START); >> + writel_relaxed(end, l3_ctrl_base + L3_MAINT_END); >> + >> + l3cache_maint_common(L3_MAINT_RANGE_ADDR, op_type); >> +} > > As mentioned, I'd like to see a code comment that explains the use > the of relaxed() vs non-relaxed MMIO accessors, as it will be impossible > for a reader to later understand why you picked a mix of the two, > and it also ensures that you have considered which one is the best > option to use here and that your explanation matches what you do. OK, I'll test the performance and add the comment. > > Based on Russell's comments, I had expected that you would use > only relaxed accessors, plus explicit barriers if you change it, matching > what l2x0 does (l2x0 has to do it because of __l2c210_cache_sync(), > while you don't have a sync callback and don't need to). I might have been a little conservative, I'll change all of them to _relaxed and then test it. Thanks. > > Arnd > > . >
WARNING: multiple messages have this Message-ID (diff)
From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com> To: Arnd Bergmann <arnd@kernel.org> Cc: devicetree <devicetree@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Will Deacon <will.deacon@arm.com>, linux-kernel <linux-kernel@vger.kernel.org>, Haojian Zhuang <haojian.zhuang@gmail.com>, Rob Herring <robh+dt@kernel.org>, Wei Xu <xuwei5@hisilicon.com>, Russell King <rmk+kernel@arm.linux.org.uk>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v6 4/4] ARM: Add support for Hisilicon Kunpeng L3 cache controller Date: Mon, 1 Feb 2021 19:38:56 +0800 [thread overview] Message-ID: <301c82eb-68ce-1a96-a0b8-d46a29bf6f36@huawei.com> (raw) In-Reply-To: <CAK8P3a0=Aj0Ss3xbgh1ELyB+4d94ybugbza_xUqW_=yVsMwEqg@mail.gmail.com> On 2021/2/1 16:31, Arnd Bergmann wrote: > On Mon, Feb 1, 2021 at 4:36 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >> Add support for the Hisilicon Kunpeng L3 cache controller as used with >> Kunpeng506 and Kunpeng509 SoCs. >> >> These Hisilicon SoCs support LPAE, so the physical addresses is wider than >> 32-bits, but the actual bit width does not exceed 36 bits. When the cache >> operation is performed based on the address range, the upper 30 bits of >> the physical address are recorded in registers L3_MAINT_START and >> L3_MAINT_END, and ignore the lower 6 bits cacheline offset. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > If you add one more thing: > >> +static void l3cache_maint_common(u32 range, u32 op_type) >> +{ >> + u32 reg; >> + >> + reg = readl_relaxed(l3_ctrl_base + L3_MAINT_CTRL); >> + reg &= ~(L3_MAINT_RANGE_MASK | L3_MAINT_TYPE_MASK); >> + reg |= range | op_type; >> + reg |= L3_MAINT_STATUS_START; >> + writel(reg, l3_ctrl_base + L3_MAINT_CTRL); >> + >> + /* Wait until the hardware maintenance operation is complete. */ >> + do { >> + cpu_relax(); >> + reg = readl(l3_ctrl_base + L3_MAINT_CTRL); >> + } while ((reg & L3_MAINT_STATUS_MASK) != L3_MAINT_STATUS_END); >> +} >> + >> +static void l3cache_maint_range(phys_addr_t start, phys_addr_t end, u32 op_type) >> +{ >> + start = start >> L3_CACHE_LINE_SHITF; >> + end = ((end - 1) >> L3_CACHE_LINE_SHITF) + 1; >> + >> + writel_relaxed(start, l3_ctrl_base + L3_MAINT_START); >> + writel_relaxed(end, l3_ctrl_base + L3_MAINT_END); >> + >> + l3cache_maint_common(L3_MAINT_RANGE_ADDR, op_type); >> +} > > As mentioned, I'd like to see a code comment that explains the use > the of relaxed() vs non-relaxed MMIO accessors, as it will be impossible > for a reader to later understand why you picked a mix of the two, > and it also ensures that you have considered which one is the best > option to use here and that your explanation matches what you do. OK, I'll test the performance and add the comment. > > Based on Russell's comments, I had expected that you would use > only relaxed accessors, plus explicit barriers if you change it, matching > what l2x0 does (l2x0 has to do it because of __l2c210_cache_sync(), > while you don't have a sync callback and don't need to). I might have been a little conservative, I'll change all of them to _relaxed and then test it. Thanks. > > Arnd > > . > _______________________________________________ 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:[~2021-02-01 11:40 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-01 3:35 [PATCH v6 0/4] ARM: Add support for Hisilicon Kunpeng L3 cache controller Zhen Lei 2021-02-01 3:35 ` Zhen Lei 2021-02-01 3:35 ` [PATCH v6 1/4] ARM: LPAE: Use phys_addr_t instead of unsigned long in outercache hooks Zhen Lei 2021-02-01 3:35 ` Zhen Lei 2021-02-01 3:35 ` [PATCH v6 2/4] ARM: hisi: add support for Kunpeng50x SoC Zhen Lei 2021-02-01 3:35 ` Zhen Lei 2021-02-01 8:35 ` Arnd Bergmann 2021-02-01 8:35 ` Arnd Bergmann 2021-02-01 11:49 ` Leizhen (ThunderTown) 2021-02-01 11:49 ` Leizhen (ThunderTown) 2021-02-01 12:14 ` Arnd Bergmann 2021-02-01 12:14 ` Arnd Bergmann 2021-02-01 3:36 ` [PATCH v6 3/4] dt-bindings: arm: hisilicon: Add binding for Kunpeng L3 cache controller Zhen Lei 2021-02-01 3:36 ` Zhen Lei 2021-02-01 3:36 ` [PATCH v6 4/4] ARM: Add support for Hisilicon " Zhen Lei 2021-02-01 3:36 ` Zhen Lei 2021-02-01 8:31 ` Arnd Bergmann 2021-02-01 8:31 ` Arnd Bergmann 2021-02-01 11:38 ` Leizhen (ThunderTown) [this message] 2021-02-01 11:38 ` Leizhen (ThunderTown)
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=301c82eb-68ce-1a96-a0b8-d46a29bf6f36@huawei.com \ --to=thunder.leizhen@huawei.com \ --cc=arnd@arndb.de \ --cc=arnd@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=haojian.zhuang@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=rmk+kernel@arm.linux.org.uk \ --cc=robh+dt@kernel.org \ --cc=will.deacon@arm.com \ --cc=xuwei5@hisilicon.com \ /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.