Hi Conor, Thank you for the review. On Sat, Jan 7, 2023 at 12:09 AM Conor Dooley <conor@xxxxxxxxxx> wrote: > > On Fri, Jan 06, 2023 at 06:55:25PM +0000, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > I/O Coherence Port (IOCP) provides an AXI interface for connecting > > external non-caching masters, such as DMA controllers. The accesses > > from IOCP are coherent with D-Caches and L2 Cache. > > > > IOCP is a specification option and is disabled on the Renesas RZ/Five > > SoC due to this reason IP blocks using DMA will fail. > > > > 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 > > > > More info about PMA (section 10.3): > > Link: http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf > > > > As a workaround for SoCs with IOCP disabled CMO needs to be handled by > > software. Firstly OpenSBI configures the memory region as > > "Memory, Non-cacheable, Bufferable" and passes this region as a global > > shared dma pool as a DT node. With DMA_GLOBAL_POOL enabled all DMA > > allocations happen from this region and synchronization callbacks are > > implemented to synchronize when doing DMA transactions. > > > > Example PMA region passes as a DT node from OpenSBI: > > reserved-memory { > > #address-cells = <2>; > > #size-cells = <2>; > > ranges; > > > > pma_resv0@58000000 { > > compatible = "shared-dma-pool"; > > reg = <0x0 0x58000000 0x0 0x08000000>; > > no-map; > > linux,dma-default; > > }; > > }; > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > diff --git a/drivers/cache/ax45mp_cache.c b/drivers/cache/ax45mp_cache.c > > new file mode 100644 > > index 000000000000..556e6875627c > > --- /dev/null > > +++ b/drivers/cache/ax45mp_cache.c > > @@ -0,0 +1,279 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * non-coherent cache functions for Andes AX45MP > > + * > > + * Copyright (C) 2022 Renesas Electronics Corp. > > + */ > > + > > +#include <asm/dma-noncoherent.h> > > +#include <linux/cacheflush.h> > > +#include <linux/cacheinfo.h> > > +#include <linux/dma-direction.h> > > +#include <linux/of_address.h> > > +#include <linux/of_platform.h> > > + > > +#include <asm/cacheflush.h> > > +#include <asm/sbi.h> > > You don't actually need this anymore, do you? > Agreed these can be dropped now. > > +static int ax45mp_l2c_probe(struct platform_device *pdev) > > +{ > > + struct riscv_cache_ops ax45mp_cmo_ops; > > + > > + /* > > + * riscv_cbom_block_size is set very much earlier so we can > > + * definitely rely on it and only if its being set we continue > > + * further in the probe path. > > + */ > > + if (!riscv_cbom_block_size) > > + return 0; > > Return 0? That's because we may actually have the IOCP & do not want to > install ops, right? > If so, please add that to the comment. > OK, I will add that. > > + > > + ax45mp_priv = devm_kzalloc(&pdev->dev, sizeof(*ax45mp_priv), GFP_KERNEL); > > + if (!ax45mp_priv) > > + return -ENOMEM; > > + > > + ax45mp_priv->l2c_base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(ax45mp_priv->l2c_base)) > > + return PTR_ERR(ax45mp_priv->l2c_base); > > + > > + ax45mp_get_l2_line_size(pdev); > > + > > + memset(&ax45mp_cmo_ops, 0x0, sizeof(ax45mp_cmo_ops)); > > + ax45mp_cmo_ops.riscv_dma_noncoherent_cmo_ops = &ax45mp_no_iocp_cmo; > > Yah, drop this dance and use a static struct foo_ops construct please. > Will do. > With those two, I'm happy with this I guess.. > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > > > + riscv_noncoherent_register_cache_ops(&ax45mp_cmo_ops); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id ax45mp_cache_ids[] = { > > + { .compatible = "andestech,ax45mp-cache" }, > > + { /* sentinel */ } > > +}; > > + > > +static struct platform_driver ax45mp_l2c_driver = { > > + .driver = { > > + .name = "ax45mp-l2c", > > + .of_match_table = ax45mp_cache_ids, > > + }, > > + .probe = ax45mp_l2c_probe, > > +}; > > + > > +static int __init ax45mp_cache_init(void) > > +{ > > + return platform_driver_register(&ax45mp_l2c_driver); > > +} > > +arch_initcall(ax45mp_cache_init); > > + > > +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Andes AX45MP L2 cache driver"); > > +MODULE_LICENSE("GPL"); > > BTW, I think these are surplus-to-requirements since this is never going > to be built as a module. > Agreed, I will drop it. > If you resurrect the directory level maintainers entry from my v5.1, you > can also add: Sure I'll add it in the next version.. > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > ... and also include your ACK. Cheers, Prabhakar
