On Fri, Jan 06, 2023 at 06:55:25PM +0000, Prabhakar wrote: > From: Lad Prabhakar > > 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 > 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 > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include You don't actually need this anymore, do you? > +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. > + > + 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. With those two, I'm happy with this I guess.. Reviewed-by: Conor Dooley > + 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 "); > +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. If you resurrect the directory level maintainers entry from my v5.1, you can also add: Acked-by: Conor Dooley Thanks, Conor.