All of lore.kernel.org
 help / color / mirror / Atom feed
From: eric@anholt.net (Eric Anholt)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/5] staging: vc04_services: Remove cache-line-size property.
Date: Wed, 07 Mar 2018 09:51:09 -0800	[thread overview]
Message-ID: <87371bydte.fsf@anholt.net> (raw)
In-Reply-To: <51dc71d7-e8f5-2785-ad13-a941bc9278ab@raspberrypi.org>

Phil Elwell <phil@raspberrypi.org> writes:

> On 07/03/2018 12:10, Stefan Wahren wrote:
>> Hi Phil,
>
> <snip>
>
>>> It is the L2 cache line size that matters, but as long as you end up with
>>> the numbers Stefan mentioned - 32 on BCM2835, 64 on BCM2836 and BCM2837 -
>>> I'm not too bothered how you get there.
>> 
>> i think a kernel with bcm2835_defconfig on RPi 2 could be such a corncase.
>> 
>> Am i right that the firmware doesn't rely on the existence of "cache-line-size"?
>
> Because of the way partial cache lines are handled it is more important that the
> two sides agree than that the value is correct. As a result, the firmware treats
> the absence of a "cache_line_size" DT parameter (that sets the "cache-line-size"
> property) in the DTB as an indication that the kernel driver pre-dates the ability
> to switch, and uses the old fixed value of 32 as a fallback. Otherwise it sets the
> parameter and the internal value used by the VPU-side VCHIQ to the correct value.
>
> There are a number of ways to fix this, the easiest of which is to assume that the kernel
> driver will either read the property or be able to work out the correct value, so
> the VPU should always use the correct value regardless of the success of applying
> the parameter/changing the property.

Oh, interesting.  So with my patch, we end up with a mismatch where VPU
is treating things as 32, and the kernel is using 64.  I wasn't seeing
errors in vchiq_test in this state, which is a bit concerning.

I'll go ahead and drop this patch and replace it with a comment in the
code about this discussion.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180307/eb12c671/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Eric Anholt <eric@anholt.net>
To: Phil Elwell <phil@raspberrypi.org>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Rob Herring <robh+dt@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	devicetree@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/5] staging: vc04_services: Remove cache-line-size property.
Date: Wed, 07 Mar 2018 09:51:09 -0800	[thread overview]
Message-ID: <87371bydte.fsf@anholt.net> (raw)
In-Reply-To: <51dc71d7-e8f5-2785-ad13-a941bc9278ab@raspberrypi.org>

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

Phil Elwell <phil@raspberrypi.org> writes:

> On 07/03/2018 12:10, Stefan Wahren wrote:
>> Hi Phil,
>
> <snip>
>
>>> It is the L2 cache line size that matters, but as long as you end up with
>>> the numbers Stefan mentioned - 32 on BCM2835, 64 on BCM2836 and BCM2837 -
>>> I'm not too bothered how you get there.
>> 
>> i think a kernel with bcm2835_defconfig on RPi 2 could be such a corncase.
>> 
>> Am i right that the firmware doesn't rely on the existence of "cache-line-size"?
>
> Because of the way partial cache lines are handled it is more important that the
> two sides agree than that the value is correct. As a result, the firmware treats
> the absence of a "cache_line_size" DT parameter (that sets the "cache-line-size"
> property) in the DTB as an indication that the kernel driver pre-dates the ability
> to switch, and uses the old fixed value of 32 as a fallback. Otherwise it sets the
> parameter and the internal value used by the VPU-side VCHIQ to the correct value.
>
> There are a number of ways to fix this, the easiest of which is to assume that the kernel
> driver will either read the property or be able to work out the correct value, so
> the VPU should always use the correct value regardless of the success of applying
> the parameter/changing the property.

Oh, interesting.  So with my patch, we end up with a mismatch where VPU
is treating things as 32, and the kernel is using 64.  I wasn't seeing
errors in vchiq_test in this state, which is a bit concerning.

I'll go ahead and drop this patch and replace it with a comment in the
code about this discussion.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2018-03-07 17:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-05 20:28 [PATCH 0/5] VCHI DT bindings Eric Anholt
2018-03-05 20:28 ` Eric Anholt
2018-03-05 20:28 ` [PATCH 1/5] staging: vc04_services: Remove dead FRAGMENTS_T Eric Anholt
2018-03-05 20:28   ` Eric Anholt
2018-03-05 20:28 ` [PATCH 2/5] staging: vc04_services: Remove cache-line-size property Eric Anholt
2018-03-05 20:28   ` Eric Anholt
2018-03-06 10:30   ` Stefan Wahren
2018-03-06 10:30     ` Stefan Wahren
2018-03-06 10:30     ` Stefan Wahren
2018-03-06 19:02     ` Eric Anholt
2018-03-06 19:02       ` Eric Anholt
2018-03-07  8:02       ` Phil Elwell
2018-03-07  8:02         ` Phil Elwell
2018-03-07  8:02         ` Phil Elwell
2018-03-07 12:10         ` Stefan Wahren
2018-03-07 12:10           ` Stefan Wahren
2018-03-07 12:10           ` Stefan Wahren
2018-03-07 12:39           ` Phil Elwell
2018-03-07 12:39             ` Phil Elwell
2018-03-07 12:39             ` Phil Elwell
2018-03-07 17:51             ` Eric Anholt [this message]
2018-03-07 17:51               ` Eric Anholt
2018-03-05 20:28 ` [PATCH 3/5] dt-bindings: soc: Add a binding for the Broadcom VCHI services Eric Anholt
2018-03-05 20:28   ` Eric Anholt
2018-03-05 20:51   ` Rob Herring
2018-03-05 20:51     ` Rob Herring
2018-03-05 20:28 ` [PATCH 4/5] ARM: dts: bcm2835: Add VCHI node to the Raspberry Pi boards Eric Anholt
2018-03-05 20:28   ` Eric Anholt
2018-03-05 20:28   ` Eric Anholt
2018-03-05 20:28 ` [PATCH 5/5] staging: vc04_services: Mark the "DT bindings" job done Eric Anholt
2018-03-05 20:28   ` Eric Anholt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87371bydte.fsf@anholt.net \
    --to=eric@anholt.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.