Quoting Umang Jain (2024-03-14 10:06:05)
> The cache-line-size is cached in struct vchiq_drvdata.
> There is no need to cache this again via g_cache_line_size
> and use it. Instead use the value from vchiq_drvdata directly.
>
> Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 50 +++++++++++--------
> 1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 8c7520dee5f4..282f83b335d4 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -71,6 +71,16 @@ static struct vchiq_device *bcm2835_audio;
> static struct vchiq_device *bcm2835_camera;
>
> static struct vchiq_drvdata bcm2835_drvdata = {
> + /* This value is the size of the L2 cache lines as understood by the
This file has a mix of A)
/* bulk multiline
* comments like this
*/
and B)
/*
* Bulk multiline
* comments like this
*/
I'd probably use B) when moving this, but again, no point for a respin
just for that. Only if there's another revision. Or a patch on top to
normalise throughout if you felt like it, as this is only a move anyway.
Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> + * VPU firmware, which determines the required alignment of the
> + * offsets/sizes in pagelists.
> + *
> + * Modern VPU firmware looks for a DT "cache-line-size" property in
> + * the VCHIQ node and will overwrite it with the actual L2 cache size,
> + * which the kernel must then respect. That property was rejected
> + * upstream, so we have to use the VPU firmware's compatibility value
> + * of 32.
> + */
> .cache_line_size = 32,
> };
>
> @@ -130,17 +140,6 @@ struct vchiq_pagelist_info {
> };
>
> static void __iomem *g_regs;
> -/* This value is the size of the L2 cache lines as understood by the
> - * VPU firmware, which determines the required alignment of the
> - * offsets/sizes in pagelists.
> - *
> - * Modern VPU firmware looks for a DT "cache-line-size" property in
> - * the VCHIQ node and will overwrite it with the actual L2 cache size,
> - * which the kernel must then respect. That property was rejected
> - * upstream, so we have to use the VPU firmware's compatibility value
> - * of 32.
> - */
> -static unsigned int g_cache_line_size = 32;
> static unsigned int g_fragments_size;
> static char *g_fragments_base;
> static char *g_free_fragments;
> @@ -211,6 +210,8 @@ static struct vchiq_pagelist_info *
> create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
> size_t count, unsigned short type)
> {
> + struct platform_device *pdev;
> + struct vchiq_drvdata *drvdata;
> struct pagelist *pagelist;
> struct vchiq_pagelist_info *pagelistinfo;
> struct page **pages;
> @@ -225,6 +226,9 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
> if (count >= INT_MAX - PAGE_SIZE)
> return NULL;
>
> + pdev = to_platform_device(instance->state->dev->parent);
> + drvdata = platform_get_drvdata(pdev);
> +
> if (buf)
> offset = (uintptr_t)buf & (PAGE_SIZE - 1);
> else
> @@ -367,9 +371,9 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
>
> /* Partial cache lines (fragments) require special measures */
> if ((type == PAGELIST_READ) &&
> - ((pagelist->offset & (g_cache_line_size - 1)) ||
> + ((pagelist->offset & (drvdata->cache_line_size - 1)) ||
> ((pagelist->offset + pagelist->length) &
> - (g_cache_line_size - 1)))) {
> + (drvdata->cache_line_size - 1)))) {
> char *fragments;
>
> if (down_interruptible(&g_free_fragments_sema)) {
> @@ -395,12 +399,19 @@ static void
> free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagelistinfo,
> int actual)
> {
> + struct platform_device *pdev;
> + struct vchiq_drvdata *drvdata;
> struct pagelist *pagelist = pagelistinfo->pagelist;
> struct page **pages = pagelistinfo->pages;
> unsigned int num_pages = pagelistinfo->num_pages;
> + unsigned int cache_line_size;
>
> dev_dbg(instance->state->dev, "arm: %pK, %d\n", pagelistinfo->pagelist, actual);
>
> + pdev = to_platform_device(instance->state->dev->parent);
> + drvdata = platform_get_drvdata(pdev);
> + cache_line_size = drvdata->cache_line_size;
> +
> /*
> * NOTE: dma_unmap_sg must be called before the
> * cpu can touch any of the data/pages.
> @@ -416,10 +427,10 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> g_fragments_size;
> int head_bytes, tail_bytes;
>
> - head_bytes = (g_cache_line_size - pagelist->offset) &
> - (g_cache_line_size - 1);
> + head_bytes = (cache_line_size - pagelist->offset) &
> + (cache_line_size - 1);
> tail_bytes = (pagelist->offset + actual) &
> - (g_cache_line_size - 1);
> + (cache_line_size - 1);
>
> if ((actual >= 0) && (head_bytes != 0)) {
> if (head_bytes > actual)
> @@ -434,8 +445,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> (tail_bytes != 0))
> memcpy_to_page(pages[num_pages - 1],
> (pagelist->offset + actual) &
> - (PAGE_SIZE - 1) & ~(g_cache_line_size - 1),
> - fragments + g_cache_line_size,
> + (PAGE_SIZE - 1) & ~(cache_line_size - 1),
> + fragments + cache_line_size,
> tail_bytes);
>
> down(&g_free_fragments_mutex);
> @@ -478,8 +489,7 @@ static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state
> if (err < 0)
> return err;
>
> - g_cache_line_size = drvdata->cache_line_size;
> - g_fragments_size = 2 * g_cache_line_size;
> + g_fragments_size = 2 * drvdata->cache_line_size;
>
> /* Allocate space for the channels in coherent memory */
> slot_mem_size = PAGE_ALIGN(TOTAL_SLOTS * VCHIQ_SLOT_SIZE);
> --
> 2.43.0
>
