All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rob Herring <robh@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Conor Dooley <conor.dooley@microchip.com>,
	Guo Ren <guoren@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Atish Patra <atishp@rivosinc.com>,
	Anup Patel <anup@brainfault.org>,
	Andrew Jones <ajones@ventanamicro.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [RFC PATCH v3 2/2] soc: renesas: Add L2 cache management for RZ/Five SoC
Date: Fri, 21 Oct 2022 23:32:01 +0100	[thread overview]
Message-ID: <Y1Md4bMYJHI34HuJ@spud> (raw)
In-Reply-To: <CA+V-a8v0jEFj+XKAtyAaTBFmX3bObBZgGd6n1LB3OnTU_W002w@mail.gmail.com>

On Fri, Oct 21, 2022 at 11:05:40PM +0100, Lad, Prabhakar wrote:
> Hi Rob,
> 
> Thank you for the review.
> 
> On Fri, Oct 21, 2022 at 3:05 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Oct 19, 2022 at 11:02:42PM +0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > On the AX45MP core, cache coherency is a specification option so it may
> > > not be supported. In this case DMA will fail. As a workaround, firstly we
> > > allocate a global dma coherent pool from which DMA allocations are taken
> > > and marked as non-cacheable + bufferable using the PMA region as specified
> > > in the device tree. Synchronization callbacks are implemented to
> > > synchronize when doing DMA transactions.
> > >
> > > The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> > > block that allows dynamic adjustment of memory attributes in the runtime.
> > > It contains a configurable amount of PMA entries implemented as CSR
> > > registers to control the attributes of memory locations in interest.
> > >
> > > Below are the memory attributes supported:
> > > * Device, Non-bufferable
> > > * Device, bufferable
> > > * Memory, Non-cacheable, Non-bufferable
> > > * Memory, Non-cacheable, Bufferable
> > > * Memory, Write-back, No-allocate
> > > * Memory, Write-back, Read-allocate
> > > * Memory, Write-back, Write-allocate
> > > * Memory, Write-back, Read and Write-allocate
> > >
> > > This patch adds support to configure the memory attributes of the memory
> > > regions as passed from the l2 cache node and exposes the cache management
> > > ops.
> > >
> > > More info about PMA (section 10.3):
> > > http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> > >
> > > This feature is based on the work posted [0] by Vincent Chen
> > > <vincentc@andestech.com> for the Andes AndeStart RISC-V CPU.
> > >
> > > [0] https://lore.kernel.org/lkml/1540982130-28248-1-git-send-email-vincentc@andestech.com/
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  arch/riscv/include/asm/cacheflush.h    |   8 +
> > >  arch/riscv/include/asm/errata_list.h   |   2 +
> > >  arch/riscv/mm/dma-noncoherent.c        |  20 ++
> > >  drivers/soc/renesas/Kconfig            |   5 +
> > >  drivers/soc/renesas/Makefile           |   4 +
> > >  drivers/soc/renesas/rzf/Kconfig        |   6 +
> > >  drivers/soc/renesas/rzf/Makefile       |   3 +
> > >  drivers/soc/renesas/rzf/ax45mp_cache.c | 431 +++++++++++++++++++++++++
> >
> > How many cache drivers do we have around now? I've seen a few bindings
> > go by. I'm guessing it is time to stop putting the drivers in the
> > drivers/soc/ dumping ground.
> >
> The main reason this driver is not in arch/riscv is that it has vendor
> specific extensions. Due to this reason it was agreed during the LPC
> that vendor specific extension should be maintained by SoC vendors and
> was agreed that this can go into drivers/soc/renesas folder instead.

Does not in drivers/soc mean they need to go into arch/riscv?
The outcome of the chat at the LPC BoF was more that the cache drivers
themselves should not be be routed via the arch maintainers, no?

> 
> > >  drivers/soc/renesas/rzf/ax45mp_sbi.h   |  29 ++
> > >  9 files changed, 508 insertions(+)
> > >  create mode 100644 drivers/soc/renesas/rzf/Kconfig
> > >  create mode 100644 drivers/soc/renesas/rzf/Makefile
> > >  create mode 100644 drivers/soc/renesas/rzf/ax45mp_cache.c
> > >  create mode 100644 drivers/soc/renesas/rzf/ax45mp_sbi.h
> > >
> > > diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
> > > index 8a5c246b0a21..40aa790be9a3 100644
> > > --- a/arch/riscv/include/asm/cacheflush.h
> > > +++ b/arch/riscv/include/asm/cacheflush.h
> > > @@ -65,6 +65,14 @@ static inline void riscv_noncoherent_supported(void) {}
> > >  #define SYS_RISCV_FLUSH_ICACHE_LOCAL 1UL
> > >  #define SYS_RISCV_FLUSH_ICACHE_ALL   (SYS_RISCV_FLUSH_ICACHE_LOCAL)
> > >
> > > +#ifdef CONFIG_AX45MP_L2_CACHE
> > > +void ax45mp_cpu_dma_inval_range(void *vaddr, size_t end);
> > > +void ax45mp_cpu_dma_wb_range(void *vaddr, size_t end);
> > > +
> > > +#define ALT_CMO_OP(_op, _start, _size, _cachesize)   \
> > > +                _op(_start, _size)
> > > +#endif
> > > +
> > >  #include <asm-generic/cacheflush.h>
> > >
> > >  #endif /* _ASM_RISCV_CACHEFLUSH_H */
> > > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > > index 19a771085781..d9cbf60c3b65 100644
> > > --- a/arch/riscv/include/asm/errata_list.h
> > > +++ b/arch/riscv/include/asm/errata_list.h
> > > @@ -89,6 +89,7 @@ asm volatile(ALTERNATIVE(                                           \
> > >  #define ALT_THEAD_PMA(_val)
> > >  #endif
> > >
> > > +#ifdef CONFIG_ERRATA_THEAD_CMO
> > >  /*
> > >   * dcache.ipa rs1 (invalidate, physical address)
> > >   * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > > @@ -143,5 +144,6 @@ asm volatile(ALTERNATIVE_2(                                               \
> > >       : "a0")
> > >
> > >  #endif /* __ASSEMBLY__ */
> > > +#endif
> > >
> > >  #endif
> > > diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
> > > index b0add983530a..5270acca6766 100644
> > > --- a/arch/riscv/mm/dma-noncoherent.c
> > > +++ b/arch/riscv/mm/dma-noncoherent.c
> > > @@ -24,13 +24,25 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> > >
> > >       switch (dir) {
> > >       case DMA_TO_DEVICE:
> > > +#ifdef CONFIG_ERRATA_THEAD_CMO
> > >               ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
> > > +#elif CONFIG_AX45MP_L2_CACHE
> > > +             ALT_CMO_OP(ax45mp_cpu_dma_wb_range, vaddr, size, 0x0);
> > > +#endif
> >
> > How do you support more than one platform in a build?
> >
> Yes, that's one concern which I have mentioned in the cover letter too
> (At that moment it's just a single platform). Suggestions welcome!

I think I said it on one of the earlier version, but it needs to be
implemented w/ runtime patching via alternatives just like the thead
stuff patches in their functions.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rob Herring <robh@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Conor Dooley <conor.dooley@microchip.com>,
	Guo Ren <guoren@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Atish Patra <atishp@rivosinc.com>,
	Anup Patel <anup@brainfault.org>,
	Andrew Jones <ajones@ventanamicro.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [RFC PATCH v3 2/2] soc: renesas: Add L2 cache management for RZ/Five SoC
Date: Fri, 21 Oct 2022 23:32:01 +0100	[thread overview]
Message-ID: <Y1Md4bMYJHI34HuJ@spud> (raw)
In-Reply-To: <CA+V-a8v0jEFj+XKAtyAaTBFmX3bObBZgGd6n1LB3OnTU_W002w@mail.gmail.com>

On Fri, Oct 21, 2022 at 11:05:40PM +0100, Lad, Prabhakar wrote:
> Hi Rob,
> 
> Thank you for the review.
> 
> On Fri, Oct 21, 2022 at 3:05 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Oct 19, 2022 at 11:02:42PM +0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > On the AX45MP core, cache coherency is a specification option so it may
> > > not be supported. In this case DMA will fail. As a workaround, firstly we
> > > allocate a global dma coherent pool from which DMA allocations are taken
> > > and marked as non-cacheable + bufferable using the PMA region as specified
> > > in the device tree. Synchronization callbacks are implemented to
> > > synchronize when doing DMA transactions.
> > >
> > > The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
> > > block that allows dynamic adjustment of memory attributes in the runtime.
> > > It contains a configurable amount of PMA entries implemented as CSR
> > > registers to control the attributes of memory locations in interest.
> > >
> > > Below are the memory attributes supported:
> > > * Device, Non-bufferable
> > > * Device, bufferable
> > > * Memory, Non-cacheable, Non-bufferable
> > > * Memory, Non-cacheable, Bufferable
> > > * Memory, Write-back, No-allocate
> > > * Memory, Write-back, Read-allocate
> > > * Memory, Write-back, Write-allocate
> > > * Memory, Write-back, Read and Write-allocate
> > >
> > > This patch adds support to configure the memory attributes of the memory
> > > regions as passed from the l2 cache node and exposes the cache management
> > > ops.
> > >
> > > More info about PMA (section 10.3):
> > > http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
> > >
> > > This feature is based on the work posted [0] by Vincent Chen
> > > <vincentc@andestech.com> for the Andes AndeStart RISC-V CPU.
> > >
> > > [0] https://lore.kernel.org/lkml/1540982130-28248-1-git-send-email-vincentc@andestech.com/
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  arch/riscv/include/asm/cacheflush.h    |   8 +
> > >  arch/riscv/include/asm/errata_list.h   |   2 +
> > >  arch/riscv/mm/dma-noncoherent.c        |  20 ++
> > >  drivers/soc/renesas/Kconfig            |   5 +
> > >  drivers/soc/renesas/Makefile           |   4 +
> > >  drivers/soc/renesas/rzf/Kconfig        |   6 +
> > >  drivers/soc/renesas/rzf/Makefile       |   3 +
> > >  drivers/soc/renesas/rzf/ax45mp_cache.c | 431 +++++++++++++++++++++++++
> >
> > How many cache drivers do we have around now? I've seen a few bindings
> > go by. I'm guessing it is time to stop putting the drivers in the
> > drivers/soc/ dumping ground.
> >
> The main reason this driver is not in arch/riscv is that it has vendor
> specific extensions. Due to this reason it was agreed during the LPC
> that vendor specific extension should be maintained by SoC vendors and
> was agreed that this can go into drivers/soc/renesas folder instead.

Does not in drivers/soc mean they need to go into arch/riscv?
The outcome of the chat at the LPC BoF was more that the cache drivers
themselves should not be be routed via the arch maintainers, no?

> 
> > >  drivers/soc/renesas/rzf/ax45mp_sbi.h   |  29 ++
> > >  9 files changed, 508 insertions(+)
> > >  create mode 100644 drivers/soc/renesas/rzf/Kconfig
> > >  create mode 100644 drivers/soc/renesas/rzf/Makefile
> > >  create mode 100644 drivers/soc/renesas/rzf/ax45mp_cache.c
> > >  create mode 100644 drivers/soc/renesas/rzf/ax45mp_sbi.h
> > >
> > > diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
> > > index 8a5c246b0a21..40aa790be9a3 100644
> > > --- a/arch/riscv/include/asm/cacheflush.h
> > > +++ b/arch/riscv/include/asm/cacheflush.h
> > > @@ -65,6 +65,14 @@ static inline void riscv_noncoherent_supported(void) {}
> > >  #define SYS_RISCV_FLUSH_ICACHE_LOCAL 1UL
> > >  #define SYS_RISCV_FLUSH_ICACHE_ALL   (SYS_RISCV_FLUSH_ICACHE_LOCAL)
> > >
> > > +#ifdef CONFIG_AX45MP_L2_CACHE
> > > +void ax45mp_cpu_dma_inval_range(void *vaddr, size_t end);
> > > +void ax45mp_cpu_dma_wb_range(void *vaddr, size_t end);
> > > +
> > > +#define ALT_CMO_OP(_op, _start, _size, _cachesize)   \
> > > +                _op(_start, _size)
> > > +#endif
> > > +
> > >  #include <asm-generic/cacheflush.h>
> > >
> > >  #endif /* _ASM_RISCV_CACHEFLUSH_H */
> > > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > > index 19a771085781..d9cbf60c3b65 100644
> > > --- a/arch/riscv/include/asm/errata_list.h
> > > +++ b/arch/riscv/include/asm/errata_list.h
> > > @@ -89,6 +89,7 @@ asm volatile(ALTERNATIVE(                                           \
> > >  #define ALT_THEAD_PMA(_val)
> > >  #endif
> > >
> > > +#ifdef CONFIG_ERRATA_THEAD_CMO
> > >  /*
> > >   * dcache.ipa rs1 (invalidate, physical address)
> > >   * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
> > > @@ -143,5 +144,6 @@ asm volatile(ALTERNATIVE_2(                                               \
> > >       : "a0")
> > >
> > >  #endif /* __ASSEMBLY__ */
> > > +#endif
> > >
> > >  #endif
> > > diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
> > > index b0add983530a..5270acca6766 100644
> > > --- a/arch/riscv/mm/dma-noncoherent.c
> > > +++ b/arch/riscv/mm/dma-noncoherent.c
> > > @@ -24,13 +24,25 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> > >
> > >       switch (dir) {
> > >       case DMA_TO_DEVICE:
> > > +#ifdef CONFIG_ERRATA_THEAD_CMO
> > >               ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
> > > +#elif CONFIG_AX45MP_L2_CACHE
> > > +             ALT_CMO_OP(ax45mp_cpu_dma_wb_range, vaddr, size, 0x0);
> > > +#endif
> >
> > How do you support more than one platform in a build?
> >
> Yes, that's one concern which I have mentioned in the cover letter too
> (At that moment it's just a single platform). Suggestions welcome!

I think I said it on one of the earlier version, but it needs to be
implemented w/ runtime patching via alternatives just like the thead
stuff patches in their functions.


  reply	other threads:[~2022-10-21 22:32 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-19 22:02 [RFC PATCH v3 0/2] AX45MP: Add support to non-coherent DMA Prabhakar
2022-10-19 22:02 ` Prabhakar
2022-10-19 22:02 ` [RFC PATCH v3 1/2] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller Prabhakar
2022-10-19 22:02   ` Prabhakar
2022-10-21  2:10   ` Rob Herring
2022-10-21  2:10     ` Rob Herring
2022-10-21 22:12     ` Lad, Prabhakar
2022-10-21 22:12       ` Lad, Prabhakar
2022-10-24 13:47   ` Geert Uytterhoeven
2022-10-24 13:47     ` Geert Uytterhoeven
2022-10-25 22:58     ` Lad, Prabhakar
2022-10-25 22:58       ` Lad, Prabhakar
2022-10-19 22:02 ` [RFC PATCH v3 2/2] soc: renesas: Add L2 cache management for RZ/Five SoC Prabhakar
2022-10-19 22:02   ` Prabhakar
2022-10-21  2:05   ` Rob Herring
2022-10-21  2:05     ` Rob Herring
2022-10-21 22:05     ` Lad, Prabhakar
2022-10-21 22:05       ` Lad, Prabhakar
2022-10-21 22:32       ` Conor Dooley [this message]
2022-10-21 22:32         ` Conor Dooley
2022-10-24 11:55         ` Lad, Prabhakar
2022-10-24 11:55           ` Lad, Prabhakar
2022-10-24 12:04           ` Heiko Stübner
2022-10-24 12:04             ` Heiko Stübner
2022-10-25 23:21             ` Lad, Prabhakar
2022-10-25 23:21               ` Lad, Prabhakar
2022-11-03  3:20         ` Rob Herring
2022-11-03  3:20           ` Rob Herring
2022-10-24 14:22   ` Geert Uytterhoeven
2022-10-24 14:22     ` Geert Uytterhoeven
2022-10-25 23:07     ` Lad, Prabhakar
2022-10-25 23:07       ` Lad, Prabhakar
2022-11-01 12:42   ` Geert Uytterhoeven
2022-11-01 12:42     ` Geert Uytterhoeven
2022-11-02  1:02     ` Lad, Prabhakar
2022-11-02  1:02       ` Lad, Prabhakar
2022-11-01 13:38   ` Geert Uytterhoeven
2022-11-01 13:38     ` Geert Uytterhoeven
2022-11-02  0:59     ` Lad, Prabhakar
2022-11-02  0:59       ` Lad, Prabhakar

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=Y1Md4bMYJHI34HuJ@spud \
    --to=conor@kernel.org \
    --cc=ajones@ventanamicro.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=atishp@rivosinc.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=conor.dooley@microchip.com \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=guoren@kernel.org \
    --cc=heiko@sntech.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=magnus.damm@gmail.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robh@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: link
Be 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.