1. Patchew
  2. linux
  3. [PATCH 0/7] refactor Andes cache driver for generic platform support
  4. View patch

[PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations

Hui Min Mina Chou posted 7 patches 2 days, 17 hours ago
Only 6 patches received!
[PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations
Posted by Hui Min Mina Chou 2 days, 17 hours ago 
This patch cleans up the Andes LLC cache driver:
 - improved error handling in andes_cache_init() by using goto labels
 - updated andes_dma_cache_inv/wback() to check for !size instead of
   start == end
 - cache-line-size mismatch from an error to a warning
 - Use ALIGN and ALIGN_DOWN helpers instead of the alignment logic in
   andes_dma_cache_inv() and andes_dma_cache_wback().

Signed-off-by: Hui Min Mina Chou <minachou@andestech.com>
---
 drivers/cache/andes_llcache.c | 56 ++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/cache/andes_llcache.c b/drivers/cache/andes_llcache.c
index d5e382f3c801..d318b8009f7f 100644
--- a/drivers/cache/andes_llcache.c
+++ b/drivers/cache/andes_llcache.c
@@ -111,21 +111,17 @@ static void andes_dma_cache_inv(phys_addr_t paddr, size_t size)
 {
 	unsigned long start = (unsigned long)phys_to_virt(paddr);
 	unsigned long end = start + size;
-	unsigned long line_size;
+	unsigned long line_size = andes_priv.andes_cache_line_size;
 	unsigned long flags;
 
-	if (unlikely(start == end))
+	if (unlikely(!size))
 		return;
 
-	line_size = andes_priv.andes_cache_line_size;
-
-	start = start & (~(line_size - 1));
-	end = ((end + line_size - 1) & (~(line_size - 1)));
+	start = ALIGN_DOWN(start, line_size);
+	end = ALIGN(end, line_size);
 
 	local_irq_save(flags);
-
 	andes_cpu_dcache_inval_range(start, end);
-
 	local_irq_restore(flags);
 }
 
@@ -133,15 +129,15 @@ static void andes_dma_cache_wback(phys_addr_t paddr, size_t size)
 {
 	unsigned long start = (unsigned long)phys_to_virt(paddr);
 	unsigned long end = start + size;
-	unsigned long line_size;
+	unsigned long line_size = andes_priv.andes_cache_line_size;
 	unsigned long flags;
 
-	if (unlikely(start == end))
+	if (unlikely(!size))
 		return;
 
-	line_size = andes_priv.andes_cache_line_size;
-	start = start & (~(line_size - 1));
-	end = ((end + line_size - 1) & (~(line_size - 1)));
+	start = ALIGN_DOWN(start, line_size);
+	end = ALIGN(end, line_size);
+
 	local_irq_save(flags);
 	andes_cpu_dcache_wb_range(start, end);
 	local_irq_restore(flags);
@@ -159,14 +155,13 @@ static int andes_get_llc_line_size(struct device_node *np)
 
 	ret = of_property_read_u32(np, "cache-line-size", &andes_priv.andes_cache_line_size);
 	if (ret) {
-		pr_err("Failed to get cache-line-size, defaulting to 64 bytes\n");
+		pr_err("Cache: Failed to get cache-line-size\n");
 		return ret;
 	}
 
 	if (andes_priv.andes_cache_line_size != ANDES_CACHE_LINE_SIZE) {
-		pr_err("Expected cache-line-size to be 64 bytes (found:%u)\n",
-		       andes_priv.andes_cache_line_size);
-		return -EINVAL;
+		pr_warn("Cache: Expected cache-line-size to be 64 bytes (found:%u)\n",
+			andes_priv.andes_cache_line_size);
 	}
 
 	return 0;
@@ -186,16 +181,18 @@ static const struct of_device_id andes_cache_ids[] = {
 static int __init andes_cache_init(void)
 {
 	struct resource res;
-	int ret;
+	int ret = 0;
 
 	struct device_node *np __free(device_node) =
 		of_find_matching_node(NULL, andes_cache_ids);
-	if (!of_device_is_available(np))
-		return -ENODEV;
+	if (!of_device_is_available(np)) {
+		ret = -ENODEV;
+		goto err_ret;
+	}
 
 	ret = of_address_to_resource(np, 0, &res);
 	if (ret)
-		return ret;
+		goto err_ret;
 
 	/*
 	 * If IOCP is present on the Andes AX45MP core riscv_cbom_block_size
@@ -208,17 +205,22 @@ static int __init andes_cache_init(void)
 		return 0;
 
 	andes_priv.llc_base = ioremap(res.start, resource_size(&res));
-	if (!andes_priv.llc_base)
-		return -ENOMEM;
+	if (!andes_priv.llc_base) {
+		ret = -ENOMEM;
+		goto err_ret;
+	}
 
 	ret = andes_get_llc_line_size(np);
-	if (ret) {
-		iounmap(andes_priv.llc_base);
-		return ret;
-	}
+	if (ret)
+		goto err_unmap;
 
 	riscv_noncoherent_register_cache_ops(&andes_cmo_ops);
 
 	return 0;
+
+err_unmap:
+	iounmap(andes_priv.llc_base);
+err_ret:
+	return ret;
 }
 early_initcall(andes_cache_init);
-- 
2.34.1
Re: [PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations
Posted by Krzysztof Kozlowski 1 day, 19 hours ago 
On Mon, Mar 30, 2026 at 06:27:19PM +0800, Hui Min Mina Chou wrote:
>  	return 0;
> @@ -186,16 +181,18 @@ static const struct of_device_id andes_cache_ids[] = {
>  static int __init andes_cache_init(void)
>  {
>  	struct resource res;
> -	int ret;
> +	int ret = 0;
>  
>  	struct device_node *np __free(device_node) =
>  		of_find_matching_node(NULL, andes_cache_ids);
> -	if (!of_device_is_available(np))
> -		return -ENODEV;
> +	if (!of_device_is_available(np)) {
> +		ret = -ENODEV;
> +		goto err_ret;
> +	}

You just made antipattern. Conor mentioned briefly that he does not see
commit as doing right, but let's clarify: this is absolutely wrong.

It is explicitly discouraged by cleanup.h, but even without that remarks
one simple 'return' in one line is converted to three lines with goto
and you call it "simpler"?

No way this is simpler. You made code worse.

Best regards,
Krzysztof
Re: [PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations
Posted by Conor Dooley 2 days, 12 hours ago 
On Mon, Mar 30, 2026 at 06:27:19PM +0800, Hui Min Mina Chou wrote:
> This patch cleans up the Andes LLC cache driver:
>  - improved error handling in andes_cache_init() by using goto labels

I don't agree that this is an improvement. There's no meaningful
teardown shared.

>  - updated andes_dma_cache_inv/wback() to check for !size instead of
>    start == end
>  - cache-line-size mismatch from an error to a warning
>  - Use ALIGN and ALIGN_DOWN helpers instead of the alignment logic in
>    andes_dma_cache_inv() and andes_dma_cache_wback().
> 
> Signed-off-by: Hui Min Mina Chou <minachou@andestech.com>
> ---
>  drivers/cache/andes_llcache.c | 56 ++++++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/cache/andes_llcache.c b/drivers/cache/andes_llcache.c
> index d5e382f3c801..d318b8009f7f 100644
> --- a/drivers/cache/andes_llcache.c
> +++ b/drivers/cache/andes_llcache.c
> @@ -111,21 +111,17 @@ static void andes_dma_cache_inv(phys_addr_t paddr, size_t size)
>  {
>  	unsigned long start = (unsigned long)phys_to_virt(paddr);
>  	unsigned long end = start + size;
> -	unsigned long line_size;
> +	unsigned long line_size = andes_priv.andes_cache_line_size;
>  	unsigned long flags;
>  
> -	if (unlikely(start == end))
> +	if (unlikely(!size))
>  		return;
>  
> -	line_size = andes_priv.andes_cache_line_size;
> -
> -	start = start & (~(line_size - 1));
> -	end = ((end + line_size - 1) & (~(line_size - 1)));
> +	start = ALIGN_DOWN(start, line_size);
> +	end = ALIGN(end, line_size);
>  
>  	local_irq_save(flags);
> -
>  	andes_cpu_dcache_inval_range(start, end);
> -
>  	local_irq_restore(flags);
>  }
>  
> @@ -133,15 +129,15 @@ static void andes_dma_cache_wback(phys_addr_t paddr, size_t size)
>  {
>  	unsigned long start = (unsigned long)phys_to_virt(paddr);
>  	unsigned long end = start + size;
> -	unsigned long line_size;
> +	unsigned long line_size = andes_priv.andes_cache_line_size;
>  	unsigned long flags;
>  
> -	if (unlikely(start == end))
> +	if (unlikely(!size))
>  		return;
>  
> -	line_size = andes_priv.andes_cache_line_size;
> -	start = start & (~(line_size - 1));
> -	end = ((end + line_size - 1) & (~(line_size - 1)));
> +	start = ALIGN_DOWN(start, line_size);
> +	end = ALIGN(end, line_size);
> +
>  	local_irq_save(flags);
>  	andes_cpu_dcache_wb_range(start, end);
>  	local_irq_restore(flags);
> @@ -159,14 +155,13 @@ static int andes_get_llc_line_size(struct device_node *np)
>  
>  	ret = of_property_read_u32(np, "cache-line-size", &andes_priv.andes_cache_line_size);
>  	if (ret) {
> -		pr_err("Failed to get cache-line-size, defaulting to 64 bytes\n");
> +		pr_err("Cache: Failed to get cache-line-size\n");
>  		return ret;
>  	}
>  
>  	if (andes_priv.andes_cache_line_size != ANDES_CACHE_LINE_SIZE) {
> -		pr_err("Expected cache-line-size to be 64 bytes (found:%u)\n",
> -		       andes_priv.andes_cache_line_size);
> -		return -EINVAL;
> +		pr_warn("Cache: Expected cache-line-size to be 64 bytes (found:%u)\n",
> +			andes_priv.andes_cache_line_size);
>  	}
>  
>  	return 0;
> @@ -186,16 +181,18 @@ static const struct of_device_id andes_cache_ids[] = {
>  static int __init andes_cache_init(void)
>  {
>  	struct resource res;
> -	int ret;
> +	int ret = 0;
>  
>  	struct device_node *np __free(device_node) =
>  		of_find_matching_node(NULL, andes_cache_ids);
> -	if (!of_device_is_available(np))
> -		return -ENODEV;
> +	if (!of_device_is_available(np)) {
> +		ret = -ENODEV;
> +		goto err_ret;
> +	}
>  
>  	ret = of_address_to_resource(np, 0, &res);
>  	if (ret)
> -		return ret;
> +		goto err_ret;
>  
>  	/*
>  	 * If IOCP is present on the Andes AX45MP core riscv_cbom_block_size
> @@ -208,17 +205,22 @@ static int __init andes_cache_init(void)
>  		return 0;
>  
>  	andes_priv.llc_base = ioremap(res.start, resource_size(&res));
> -	if (!andes_priv.llc_base)
> -		return -ENOMEM;
> +	if (!andes_priv.llc_base) {
> +		ret = -ENOMEM;
> +		goto err_ret;
> +	}
>  
>  	ret = andes_get_llc_line_size(np);
> -	if (ret) {
> -		iounmap(andes_priv.llc_base);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_unmap;
>  
>  	riscv_noncoherent_register_cache_ops(&andes_cmo_ops);
>  
>  	return 0;
> +
> +err_unmap:
> +	iounmap(andes_priv.llc_base);
> +err_ret:
> +	return ret;
>  }
>  early_initcall(andes_cache_init);
> -- 
> 2.34.1
>
Re: [PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations
Posted by Krzysztof Kozlowski 2 days, 15 hours ago 
On 30/03/2026 12:27, Hui Min Mina Chou wrote:
> This patch cleans up the Andes LLC cache driver:
>  - improved error handling in andes_cache_init() by using goto labels
>  - updated andes_dma_cache_inv/wback() to check for !size instead of
>    start == end
>  - cache-line-size mismatch from an error to a warning
>  - Use ALIGN and ALIGN_DOWN helpers instead of the alignment logic in
>    andes_dma_cache_inv() and andes_dma_cache_wback().

Please read submitting patches document. One thing per commit with
proper rationale WHY you are doing this.

Best regards,
Krzysztof
Re: [PATCH 2/7] cache: andes_llcache: refactor initialization and cache operations
Posted by Conor Dooley 2 days, 12 hours ago 
On Mon, Mar 30, 2026 at 03:02:11PM +0200, Krzysztof Kozlowski wrote:
> On 30/03/2026 12:27, Hui Min Mina Chou wrote:
> > This patch cleans up the Andes LLC cache driver:
> >  - improved error handling in andes_cache_init() by using goto labels
> >  - updated andes_dma_cache_inv/wback() to check for !size instead of
> >    start == end
> >  - cache-line-size mismatch from an error to a warning
> >  - Use ALIGN and ALIGN_DOWN helpers instead of the alignment logic in
> >    andes_dma_cache_inv() and andes_dma_cache_wback().
> 
> Please read submitting patches document. One thing per commit with
> proper rationale WHY you are doing this.

Applies to multiple patches too. Anything here with a bullet list needs
to be several patches.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.