Re: [PATCH 1/2] dt-bindings: crypto: update ccree optional params

[Gilad Ben-Yossef]

On Mon, Nov 16, 2020 at 8:54 PM Rob Herring <robh@xxxxxxxxxx> 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...

No problem at all. I know how it is...


> >
> >
> > 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
> >> >

<snip>

> >> > 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.

OK, I now better understand what you meant and that does make some
sense and I will defer to your better judgment  about the proper way
to do this.

Having said that, there is still something that bugs me about this,
even just at the level of better understanding why we do things:

Way back when, before DT, we had SoC specific code that identified the
SoC somehow and set things up in a SoC specific way.
Then we introduced DT as a way to say - "hey look, this is how my
busses looks like, these are the devices I have, deal with it" and I
always assumed that this was meant as a way to release us from having
SoC specific setup code.

It seems now potentially every SoC vendor needs to modify not just the
device tree source (which makes sense of course) but also the driver
supporting their platform.
It now looks like we've come a full circle to me :-)

Thanks!
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!