Re: [PATCH 1/7] dt-bindings: gpu: pvrsgx: add initial bindings

[H. Nikolaus Schaller]

> Am 21.10.2019 um 19:25 schrieb Tony Lindgren <tony@xxxxxxxxxxx>:
> 
> * H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> [191021 15:46]:
>>> Am 21.10.2019 um 17:07 schrieb Rob Herring <robh+dt@xxxxxxxxxx>:
>>> On Fri, Oct 18, 2019 at 1:46 PM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>>>> +Optional properties:
>>>> +- timer:       the timer to be used by the driver.
>>> 
>>> Needs a better description and vendor prefix at least.
>> 
>> I am not yet sure if it is vendor specific or if all
>> SGX implementations need some timer.
>> 
>>> 
>>> Why is this needed rather than using the OS's timers?
>> 
>> Because nobody understands the current (out of tree and
>> planned for staging) driver well enough what the timer
>> is doing. It is currently hard coded that some omap refer
>> to timer7 and others use timer11.
> 
> Just configure it in the driver based on the compatible
> value to keep it out of the dts. It's best to stick to
> standard bindings.

IMHO leads to ugly code... Since the timer is not part of
the SGX IPR module but one of the OMAP timers it is sort
of hardware connection that can be chosen a little arbitrarily.

This is the main reason why I think adding it to a device tree
source so that a board that really requires to use a timer
for a different purpose, can reassign it. This is not possible
if we hard-code that into the driver by scanning for
compatible. In that case the driver must check board compatible
names...

But if we gain a better understanding of its role in the driver
(does it really need a dedicated timer and for what and which
properties the timer must have) we can probably replace it.

> 
>>>> +- img,cores:   number of cores. Defaults to <1>.
>>> 
>>> Not discoverable?
>> 
>> Not sure if it is. This is probably available in undocumented
>> registers of the sgx.
> 
> This too, and whatever non-standrd other properities
> you might have.

Here it is a feature of the SGX IPR of the SoC, i.e.
describes that the hardware has one or two cores.

BR,
NIkolaus