mbox series

[0/2] add optional cache params from DT

Message ID 20200916071950.1493-1-gilad@benyossef.com
Headers show
Series add optional cache params from DT | expand

Message

Gilad Ben-Yossef Sept. 16, 2020, 7:19 a.m. UTC
Rework the setting of AXI bus cache parameters, including
optionally allowing setting them from device tree

Gilad Ben-Yossef (2):
  dt-bindings: crypto: update ccree optional params
  crypto: ccree - add custom cache params from DT file

 .../bindings/crypto/arm-cryptocell.txt        |  4 +
 drivers/crypto/ccree/cc_driver.c              | 89 +++++++++++++++----
 drivers/crypto/ccree/cc_driver.h              |  4 +-
 drivers/crypto/ccree/cc_pm.c                  |  2 +-
 4 files changed, 81 insertions(+), 18 deletions(-)

-- 
2.27.0

Comments

Rob Herring (Arm) Sept. 23, 2020, 1:57 a.m. UTC | #1
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@benyossef.com>

> ---

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

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

These could also just be implied by the compatible string (and requiring 
an SoC specific one).

Rob
Gilad Ben-Yossef Sept. 29, 2020, 6:08 p.m. UTC | #2
Hןת

On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@kernel.org> 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@benyossef.com>

> > ---

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

>

> 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.
There is currently no other part of the driver that needs this.

Gilad




-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!
Rob Herring (Arm) Nov. 16, 2020, 6:54 p.m. UTC | #3
On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef <gilad@benyossef.com> 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@benyossef.com> wrote:

>>

>>

>> On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@kernel.org> 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@benyossef.com>

>> > > ---

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

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

Rob
Gilad Ben-Yossef Nov. 17, 2020, 7:39 a.m. UTC | #4
On Mon, Nov 16, 2020 at 8:54 PM Rob Herring <robh@kernel.org> wrote:
>

> On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef <gilad@benyossef.com> 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@benyossef.com> wrote:

> >>

> >>

> >> On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@kernel.org> 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@benyossef.com>

> >> > > ---

> >> > >  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!
Robin Murphy Nov. 17, 2020, 2:07 p.m. UTC | #5
On 2020-11-16 18:54, Rob Herring wrote:
> On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef <gilad@benyossef.com> 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@benyossef.com> wrote:

>>>

>>>

>>> On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@kernel.org> 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@benyossef.com>

>>>>> ---

>>>>>   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.
Rob Herring (Arm) Nov. 17, 2020, 2:58 p.m. UTC | #6
On Tue, Nov 17, 2020 at 1:39 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>
> On Mon, Nov 16, 2020 at 8:54 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef <gilad@benyossef.com> 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@benyossef.com> wrote:
> > >>
> > >>
> > >> On Wed, Sep 23, 2020 at 4:57 AM Rob Herring <robh@kernel.org> 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@benyossef.com>
> > >> > > ---
> > >> > >  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.

Yes, but in the end it's a judgement call as to what the boundary is.
Take clocks for example, in the beginning we were trying to describe
clocks on a mux/divider/gate level in DT. We realized this would
result in hundreds to thousands of DT nodes and it would never be
completely correct. So we model only the leaf clocks for the most part
and there's lots of SoC code for clock controller blocks still.

The questions for having properties or not to ask is:

Is this board specific? Yes, then use a property.

Who needs to change this and when? Should/would you want to control
this in your PC BIOS or via a BIOS update?


Zero SoC code was never a goal. It was about a standard way to define
SoCs and having common frameworks (we have a common clock api, but not
implementation at the time). We have *way* less SoC code per SoC than
we used to. At the start of the DT conversion for Arm, we had ~400
boards and now we're at ~1400 last I checked.

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

As I said, if the h/w is 'exactly the same' (hint: it rarely is), then
use a fallback compatible. Then the new SoC specific compatible is
there just in case.

Think of compatible just as a VID/PID in PCI and USB land (though the
closest thing to a fallback there is class codes). They are the only
way we can uniquely identify h/w.

Rob
Gilad Ben-Yossef Nov. 19, 2020, 11:41 a.m. UTC | #7
On Tue, Nov 17, 2020 at 4:58 PM Rob Herring <robh@kernel.org> wrote:

> >

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

>

> Yes, but in the end it's a judgement call as to what the boundary is.

> Take clocks for example, in the beginning we were trying to describe

> clocks on a mux/divider/gate level in DT. We realized this would

> result in hundreds to thousands of DT nodes and it would never be

> completely correct. So we model only the leaf clocks for the most part

> and there's lots of SoC code for clock controller blocks still.

>

> The questions for having properties or not to ask is:

>

> Is this board specific? Yes, then use a property.

>

> Who needs to change this and when? Should/would you want to control

> this in your PC BIOS or via a BIOS update?

>

>

> Zero SoC code was never a goal. It was about a standard way to define

> SoCs and having common frameworks (we have a common clock api, but not

> implementation at the time). We have *way* less SoC code per SoC than

> we used to. At the start of the DT conversion for Arm, we had ~400

> boards and now we're at ~1400 last I checked.

>

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

>

> As I said, if the h/w is 'exactly the same' (hint: it rarely is), then

> use a fallback compatible. Then the new SoC specific compatible is

> there just in case.

>

> Think of compatible just as a VID/PID in PCI and USB land (though the

> closest thing to a fallback there is class codes). They are the only

> way we can uniquely identify h/w.


Thanks Rob, this makes sense.

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!