On 2020-11-16 18:54, Rob Herring wrote:
On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx> wrote:
Hi again,
Any opinion on the suggested below?
Sorry, lost in the pile...
Thanks!
Gilad
On Tue, Sep 29, 2020 at 9:08 PM Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx> wrote:
On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@xxxxxxxxxx> wrote:
On Wed, Sep 16, 2020 at 10:19:49AM +0300, Gilad Ben-Yossef wrote:
Document ccree driver supporting new optional parameters allowing to
customize the DMA transactions cache parameters and ACE bus sharability
properties.
Signed-off-by: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>
---
Documentation/devicetree/bindings/crypto/arm-cryptocell.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
index 6130e6eb4af8..1a1603e457a8 100644
--- a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
+++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
@@ -13,6 +13,10 @@ Required properties:
Optional properties:
- clocks: Reference to the crypto engine clock.
- dma-coherent: Present if dma operations are coherent.
+- awcache: Set write transactions cache attributes
+- arcache: Set read transactions cache attributes
dma-coherent already implies these are 011x, 101x or 111x. In my limited
experience configuring these (Calxeda SATA and ethernet), writeback,
write-allocate was pretty much always optimal.
Indeed and these are the default. But not all SoC are born equal and
we got a request to allow setting these.
Maybe instead of numerical values have three possible verbal setting
would be better?
+- awdomain: Set write transactions ACE sharability domain (712, 703, 713 only)
+- ardomain: Set read transactions ACE sharability domain (712, 703, 713 only)
This probably needs something common. We may need something for Mali,
too. I don't think different settings for read and write makes much
sense nor does anything beyond IS or OS.
I agree. Maybe
sharability_domain: either "IS" or "OS"?
It's still an Arm thing, so it would need at least an 'arm,' prefix.
But ideally it wouldn't be Arm specific though I'm not sure if any
such thing is needed for other arches. If common either for Arm or
across arches, then it needs to be documented in a common doc with
some wider agreement than what a device specific property needs.
"dma-coherent" already encapsulates the shareability. To claim coherency
you need to be able to access memory with inner shareable inner-outer
write back attributes, because that's how regular kernel memory is
mapped. If you don't claim coherency, then you should be using
non-cacheable (and thus implicitly outer shareable) attributes, because
there may still be cacheable aliases hanging around and if you
inadvertently snoop those instead of going direct to DRAM as everything
else expects, you're going to have a bad time.
Essentially, Linux only uses two sets of memory attributes for DMA
memory, so any perceived need for more than that is already something
sufficiently esoteric that it probably doesn't need a generic binding.
I'm aware that it's against type for me to be arguing Linux details in a
DT binding, but I checked the TRMs for some of these devices and they
essentially say "you don't program the hardware directly, you use the
Linux driver we give you", so it seems like this binding is specifically
intended never to be consumed by anything else.
These could also just be implied by the compatible string (and requiring
an SoC specific one).
hm... we could do it but this will require us to know (and publicly
acknowledge) of every SoC making use of this piece of hardware design.
That's already a requirement in general. Sometimes we can avoid it,
but that's cases of getting lucky.
There is currently no other part of the driver that needs this.
If your DT is part of firmware, then waiting until adding some driver
feature or quirk based on a new DT property is too late. Whereas with
a SoC specific compatible, you can handle any new feature or quirk
without a DT change (e.g. just a stable kernel update). Some platforms
may not care about that model, but in general that's the policy we
follow. Not doing that, we end up with the DWC3 binding.
A fallback compatible is how we avoid updating drivers for every
single SoC unless needed.
IMO if this is like PL330 where you just stick some raw AXI attributes
in a register and that's what goes out on transactions then it's not
really part of the platform description anyway, it's just driver policy.
If folks want to tweak the driver's behaviour for secret SoC-specific
performance optimisation, then give them some module parameters or a
sysfs interface. I'm assuming this has to be purely a performance thing,
because I can't see how two different integrations could reasonably
depend on different baseline attributes for correctness, and even if
they did then you'd be able to determine that from the compatible
strings, because they would be different, because those integrations
would be fundamentally *not* compatible with each other.
Robin.
