Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/5] staging: vc04_services: Remove cache-line-size
 property.
To:     Eric Anholt <eric@anholt.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Phil Elwell <phil@raspberrypi.org>
Cc:     linux-kernel@vger.kernel.org,
        bcm-kernel-feedback-list@broadcom.com,
        linux-rpi-kernel@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org
References: <20180305202806.21219-1-eric@anholt.net>
 <20180305202806.21219-3-eric@anholt.net>
From:   Stefan Wahren <stefan.wahren@i2se.com>
Message-ID: <a8f369f5-691c-c05f-4c7c-2eadcf4c48c8@i2se.com>
Date:   Tue, 6 Mar 2018 11:30:27 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <20180305202806.21219-3-eric@anholt.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Eric,


Am 05.03.2018 um 21:28 schrieb Eric Anholt:
> This was just a way for the DT-passed value to get out of sync with
> what Linux has configured the ARM for.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>   .../interface/vchiq_arm/vchiq_2835_arm.c           | 25 +++++++---------------
>   .../interface/vchiq_arm/vchiq_pagelist.h           |  1 -
>   2 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index b59ef14890aa..e0e01c812036 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -77,7 +77,6 @@ struct vchiq_pagelist_info {
>   };
>   
>   static void __iomem *g_regs;
> -static unsigned int g_cache_line_size = sizeof(CACHE_LINE_SIZE);
>   static unsigned int g_fragments_size;
>   static char *g_fragments_base;
>   static char *g_free_fragments;
> @@ -117,15 +116,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
>   	if (err < 0)
>   		return err;
>   
> -	err = of_property_read_u32(dev->of_node, "cache-line-size",
> -				   &g_cache_line_size);
> -
> -	if (err) {
> -		dev_err(dev, "Missing cache-line-size property\n");
> -		return -ENODEV;
> -	}
> -
> -	g_fragments_size = 2 * g_cache_line_size;
> +	g_fragments_size = 2 * cache_line_size();
>   
>   	/* Allocate space for the channels in coherent memory */
>   	slot_mem_size = PAGE_ALIGN(TOTAL_SLOTS * VCHIQ_SLOT_SIZE);
> @@ -548,9 +539,9 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
>   
>   	/* Partial cache lines (fragments) require special measures */
>   	if ((type == PAGELIST_READ) &&
> -		((pagelist->offset & (g_cache_line_size - 1)) ||
> +		((pagelist->offset & (cache_line_size() - 1)) ||
>   		((pagelist->offset + pagelist->length) &
> -		(g_cache_line_size - 1)))) {
> +		 (cache_line_size() - 1)))) {
>   		char *fragments;
>   
>   		if (down_interruptible(&g_free_fragments_sema) != 0) {
> @@ -598,10 +589,10 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
>   			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);

should it be so easy? Back in 2016 we said that cache_line_size() won't 
work. I always thought that we need the cache line size of L2 not of the 
L1 one.

Did you already test the behavior for these combinations?
BCM2835 ARM32, expected cache line size = 32
BCM2836 ARM32, expected cache line size = 64
BCM2837 ARM32, expected cache line size = 64
BCM2837 ARM64, expected cache line size = 64

Regards
Stefan

>   
>   		if ((actual >= 0) && (head_bytes != 0)) {
>   			if (head_bytes > actual)
> @@ -617,8 +608,8 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
>   			(tail_bytes != 0)) {
>   			memcpy((char *)kmap(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);
>   			kunmap(pages[num_pages - 1]);
>   		}
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
> index a6c5f7cc78f0..bec411061554 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
> @@ -34,7 +34,6 @@
>   #ifndef VCHIQ_PAGELIST_H
>   #define VCHIQ_PAGELIST_H
>   
> -#define CACHE_LINE_SIZE 32
>   #define PAGELIST_WRITE 0
>   #define PAGELIST_READ 1
>   #define PAGELIST_READ_WITH_FRAGMENTS 2