Hi,

On Wed, Apr 15, 2020 at 10:35:08AM +0200, H. Nikolaus Schaller wrote:

The Imagination PVR/SGX GPU is part of several SoC from multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, Allwinner A83 and others.

With this binding, we describe how the SGX processor is interfaced to the SoC (registers, interrupt etc.).

In most cases, Clock, Reset and power management is handled by a parent node or elsewhere (e.g. code in the driver).

Wouldn't the "code in the driver" still require the clock / reset / power domain to be set in the DT?

Maxime

Hi,

On 15/04/2020 14:43, H. Nikolaus Schaller wrote:

...

Am 15.04.2020 um 12:12 schrieb Maxime Ripard maxime@cerno.tech:

Hi,

On Wed, Apr 15, 2020 at 10:35:08AM +0200, H. Nikolaus Schaller wrote:

...

The Imagination PVR/SGX GPU is part of several SoC from multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, Allwinner A83 and others.

With this binding, we describe how the SGX processor is interfaced to the SoC (registers, interrupt etc.).

In most cases, Clock, Reset and power management is handled by a parent node or elsewhere (e.g. code in the driver).

Wouldn't the "code in the driver" still require the clock / reset / power domain to be set in the DT?

Well, some SoC seem to use existing clocks and have no reset. Or, although not recommended, they may have the io-address range hard-coded.

The possible clocks and resets should be added, even if optional.

Please look at the arm utgard, midgard and bifrost bindings.

Neil

BR, Nikolaus

---

linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Wed, Apr 15, 2020 at 03:17:25PM +0200, H. Nikolaus Schaller wrote:

Hi Neil,

...

Am 15.04.2020 um 14:54 schrieb Neil Armstrong narmstrong@baylibre.com:

Hi,

On 15/04/2020 14:43, H. Nikolaus Schaller wrote:

...

...

Am 15.04.2020 um 12:12 schrieb Maxime Ripard maxime@cerno.tech:

Hi,

On Wed, Apr 15, 2020 at 10:35:08AM +0200, H. Nikolaus Schaller wrote:

...

The Imagination PVR/SGX GPU is part of several SoC from multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, Allwinner A83 and others.

With this binding, we describe how the SGX processor is interfaced to the SoC (registers, interrupt etc.).

In most cases, Clock, Reset and power management is handled by a parent node or elsewhere (e.g. code in the driver).

Wouldn't the "code in the driver" still require the clock / reset / power domain to be set in the DT?

Well, some SoC seem to use existing clocks and have no reset. Or, although not recommended, they may have the io-address range hard-coded.

The possible clocks and resets should be added, even if optional.

Please look at the arm utgard, midgard and bifrost bindings.

Interesting to compare to. Maybe we should also add the $nodename: pattern: '^gpu@[a-f0-9]+$'

But the sgx binding is difficult to grasp here. Some SoC like the omap series have their own ti,sysc based target modules and the gpu nodes is a child of it lacking any clock and reset references for purpose.

The jz4780 and some other need a clocks definition, but no reset. Having a reset seems to be an option for the SoC designer and not mandated by img. So is it part of the pvrsgx bindings or the SoC?

Well we could add clocks and resets as optional but that would allow to wrongly define omap.

Or delegate them to a parent "simple-pm-bus" node.

I have to study that material more to understand what you seem to expect.

The thing is, once that binding is in, it has to be backward compatible. So every thing that you leave out is something that you'll need to support in the driver eventually.

If you don't want it to be a complete nightmare, you'll want to figure out as much as possible on how the GPU is integrated and make a binding out of that. If OMAP is too much of a pain, you can also make a separate binding for it, and a generic one for the rest of us.

I'd say that it's pretty unlikely that the clocks, interrupts (and even regulators) are optional. It might be fixed on some SoCs, but that's up to the DT to express that using fixed clocks / regulators, not the GPU binding itself.

Maxime

On Wed, Apr 15, 2020 at 05:09:45PM +0200, H. Nikolaus Schaller wrote:

Hi Maxime,

...

Am 15.04.2020 um 16:21 schrieb Maxime Ripard maxime@cerno.tech:

...

Well we could add clocks and resets as optional but that would allow to wrongly define omap.

Or delegate them to a parent "simple-pm-bus" node.

I have to study that material more to understand what you seem to expect.

The thing is, once that binding is in, it has to be backward compatible. So every thing that you leave out is something that you'll need to support in the driver eventually.

...

If you don't want it to be a complete nightmare, you'll want to figure out as much as possible on how the GPU is integrated and make a binding out of that.

Hm. Yes. We know that there likely are clocks and maybe reset but for some SoC this seems to be undocumented and the reset line the VHDL of the sgx gpu provides may be permanently tied to "inactive".

So if clocks are optional and not provided, a driver simply can assume they are enabled somewhere else and does not have to care about. If they are specified, the driver can enable/disable them.

Except that at the hardware level, the clock is always going to be there. You can't control it, but it's there.

...

If OMAP is too much of a pain, you can also make a separate binding for it, and a generic one for the rest of us.

No, omap isn't any pain at all.

The pain is that some other SoC are most easily defined by clocks in the gpu node which the omap doesn't need to explicitly specify.

I would expect a much bigger nightmare if we split this into two bindings variants.

...

I'd say that it's pretty unlikely that the clocks, interrupts (and even regulators) are optional. It might be fixed on some SoCs, but that's up to the DT to express that using fixed clocks / regulators, not the GPU binding itself.

omap already has these defined them not to be part of the GPU binding. The reason seems to be that this needs special clock gating control especially for idle states which is beyond simple clock-enable.

This sysc target-module@56000000 node is already merged and therefore we are only adding the gpu child node. Without defining clocks.

For example:

sgx_module: target-module@56000000 {
	compatible = "ti,sysc-omap4", "ti,sysc";
	reg = <0x5600fe00 0x4>,
	      <0x5600fe10 0x4>;
	reg-names = "rev", "sysc";
	ti,sysc-midle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	ti,sysc-sidle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
	clock-names = "fck";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x56000000 0x2000000>;

	gpu: gpu@0 {
		compatible = "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544";
		reg = <0x0 0x10000>;
		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
	};
};

The jz4780 example will like this:

gpu: gpu@13040000 { compatible = "ingenic,jz4780-sgx540-130", "img,sgx540-130", "img,sgx540"; reg = <0x13040000 0x4000>;

clocks = <&cgu JZ4780_CLK_GPU>;
clock-names = "gpu";

interrupt-parent = <&intc>;
interrupts = <63>;

};

So the question is which one is "generic for the rest of us"?

I'd say the latter.

And how can we make a single binding for the sgx. Not one for each special SoC variant that may exist.

IMHO the best answer is to make clocks an optional property. Or if we do not want to define them explicitly, we use additionalProperties: true.

If your clock is optional, then you define it but don't mandate it. Not documenting it will only result in a mess where everyone will put some clock into it, possibly with different semantics each and every time.

An alternative could be to use a simple-pm-bus like:

sgx_module: sgx_module@13040000 { compatible = "simple-pm-bus";

clocks = <&cgu JZ4780_CLK_GPU>;
clock-names = "gpu";

#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x13040000 0x10000>;

gpu: gpu@0 {
	compatible = "ingenic,jz4780-sgx540-130", "img,sgx540-130", "img,sgx540";
	reg = <0x0 0x4000>;

	interrupt-parent = <&intc>;
	interrupts = <63>;
};

};

This gets rid of any clock, reset and pm definitions for the sgx bindings. But how this is done is outside this sgx bindings.

With such a scheme, the binding I propose here would be complete and fully generic. We can even add additionalProperties: false.

This has nothing to do with the binding being complete. And if you use a binding like this one, you'll be severely limited when you'll want to implement things like DVFS.

Maxime

* H. Nikolaus Schaller hns@goldelico.com [200415 16:43]:

If you agree I can add the clocks/clock-names property as an optional property. This should solve omap and all others.

Yes the clock can be optional property no problem. If we have a clock, we just enable/disable it from the pvr_runtime_suspend() and pvr_runtime_resume() we alaready have in pvr-drv.c.

Regards,

Tony

Hi Maxime,

Am 17.04.2020 um 12:25 schrieb Maxime Ripard maxime@cerno.tech:

Hi,

On Wed, Apr 15, 2020 at 06:42:18PM +0200, H. Nikolaus Schaller wrote:

...

...

Am 15.04.2020 um 18:21 schrieb Maxime Ripard maxime@cerno.tech:

On Wed, Apr 15, 2020 at 05:09:45PM +0200, H. Nikolaus Schaller wrote:

...

Hi Maxime,

Hm. Yes. We know that there likely are clocks and maybe reset but for some SoC this seems to be undocumented and the reset line the VHDL of the sgx gpu provides may be permanently tied to "inactive".

So if clocks are optional and not provided, a driver simply can assume they are enabled somewhere else and does not have to care about. If they are specified, the driver can enable/disable them.

Except that at the hardware level, the clock is always going to be there. You can't control it, but it's there.

Sure, we can deduce that from general hardware design knowledge. But not every detail must be described in DT. Only the important ones.

...

...

...

If OMAP is too much of a pain, you can also make a separate binding for it, and a generic one for the rest of us.

No, omap isn't any pain at all.

The pain is that some other SoC are most easily defined by clocks in the gpu node which the omap doesn't need to explicitly specify.

I would expect a much bigger nightmare if we split this into two bindings variants.

...

I'd say that it's pretty unlikely that the clocks, interrupts (and even regulators) are optional. It might be fixed on some SoCs, but that's up to the DT to express that using fixed clocks / regulators, not the GPU binding itself.

omap already has these defined them not to be part of the GPU binding. The reason seems to be that this needs special clock gating control especially for idle states which is beyond simple clock-enable.

This sysc target-module@56000000 node is already merged and therefore we are only adding the gpu child node. Without defining clocks.

For example:

sgx_module: target-module@56000000 {
	compatible = "ti,sysc-omap4", "ti,sysc";
	reg = <0x5600fe00 0x4>,
	      <0x5600fe10 0x4>;
	reg-names = "rev", "sysc";
	ti,sysc-midle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	ti,sysc-sidle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
	clock-names = "fck";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x56000000 0x2000000>;

	gpu: gpu@0 {
		compatible = "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544";
		reg = <0x0 0x10000>;
		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
	};
};

The jz4780 example will like this:

gpu: gpu@13040000 { compatible = "ingenic,jz4780-sgx540-130", "img,sgx540-130", "img,sgx540"; reg = <0x13040000 0x4000>;

clocks = <&cgu JZ4780_CLK_GPU>;
clock-names = "gpu";

interrupt-parent = <&intc>;
interrupts = <63>;

};

So the question is which one is "generic for the rest of us"?

I'd say the latter.

Why?

TI SoC seem to be the broadest number of available users of sgx5xx in the past and nowadays. Others are more the exception.

And maybe TI has some complicated stuff around the GPU that others don't have?

Looks so.

If I look quickly at the Allwinner stuff, I see nothing looking alike in the SoC, so making the binding like that for everyone just because TI did something doesn't really make much sense.

That is why I propose to make the clocks optional. This solves both cases in a simple and neat way.

...

...

If your clock is optional, then you define it but don't mandate it. Not documenting it will only result in a mess where everyone will put some clock into it, possibly with different semantics each and every time.

So you mean that we should require a dummy clock for the omap gpu node or did I misunderstand that?

Well, yes there is of course a clock connection between the omap target-module and the sgx but it is IMHO pointless to describe it because it can't and does not need to be controlled separately.

As said the target-module is already accepted and upstream and my proposal is to get the gpu node described there. There is simply no need for a clocks node for the omap.

There is no need for a clocks property *currently* *on the OMAP*.

Yes. But why "currently"? Do you think the OMAPs we already have defined and tested will change?

...

What I also assume is that developers of DTS know what they do. So the risk that there is different semantics is IMHO very low.

Well, they know what they do if you document the binding. Let's say I have two clocks now on my SoC, and you just document that you want a clocks property, with a generic name in clock-names like "gpu".

Yes, that is what I want to propose for v7:

clocks: maxItems: 1

clock-names: maxItems: 1 items: - const: gpu

...

If you agree I can add the clocks/clock-names property as an optional property. This should solve omap and all others.

With the above example, what clock should I put in there? In which order? This isn't some random example pulled out of nowhere. The Allwinner A31 has (at least) 4 clocks for the GPU, 1 reset line and 1 regulator, so I can only assume that the GPU actually needs at least that amount to be properly integrated into an SoC.

Ah, now I understand your motivation: you have access and experience with the A31 and you know that our proposal doesn't fit to it.

From what I know from your description is that the A31 is quite special with 4 GPU clocks... Are they all really for the GPU or 3 of them for the interface logic (like on OMAP which separates between "functional clocks" and "interface clocks")? Or are there 4 groups of GPU cores with a separate clock for each one?

So what would be your proposal for the A31 DT?

Then I get a chance to compare DT snippets and try to make a mixture for the bindings.

...

...

This has nothing to do with the binding being complete. And if you use a binding like this one, you'll be severely limited when you'll want to implement things like DVFS.

Now you have unhooked me... Nobody seems to know if and how DVFS can be applied to SGX. IMHO we should bake small bread first and get initial support into mainline.

On the software side, yes, of course. But the discussion here doesn't have much to do with software support, this is about the hardware. No matter if you enable DVFS or not, you'll have those resources connected to the GPU.

And if you want to enable the strict minimum in DT for now and expand it later as the software gains support for more stuff, then you'll have to deal with the minimal stuff in software later-on to keep the backward compatibility.

That is IMHO common practise everywhere. Sometimes you even have to adapt years old DT to new limitations of the drivers (this happened recently for combination of SPI and GPIO).

And you can still write two different drivers for a single bindings document or use the .data field of the compatibility table. And I think clocks and regulators can also be referenced by name if they are not defined in DT. This is not a "single variety" style, but a potential solution.

What I want to say: there are many roads to Rome.

But given that the current state on the Allwinner SoCs (at least) is that you can't even read a register, it might be a good idea to delay the introduction of that binding until you have something that works to avoid drowning under the number of special cases to deal with backward compatibility.

Philipp has something minimal working on the A83 which works with the proposed binding and enabled him to read out the sgx revision register.

So if you are a specialist for the A31 SGX, please make a proposal for a binding that covers all the A31 needs and all other SoC we know. Or define a separate bindings for the A31.

Thank you very much, Nikolaus Schaller

On Fri, Apr 17, 2020 at 02:15:44PM +0200, H. Nikolaus Schaller wrote:

...

Am 17.04.2020 um 12:25 schrieb Maxime Ripard maxime@cerno.tech: On Wed, Apr 15, 2020 at 06:42:18PM +0200, H. Nikolaus Schaller wrote:

...

...

Am 15.04.2020 um 18:21 schrieb Maxime Ripard maxime@cerno.tech:

On Wed, Apr 15, 2020 at 05:09:45PM +0200, H. Nikolaus Schaller wrote:

...

Hi Maxime,

Hm. Yes. We know that there likely are clocks and maybe reset but for some SoC this seems to be undocumented and the reset line the VHDL of the sgx gpu provides may be permanently tied to "inactive".

So if clocks are optional and not provided, a driver simply can assume they are enabled somewhere else and does not have to care about. If they are specified, the driver can enable/disable them.

Except that at the hardware level, the clock is always going to be there. You can't control it, but it's there.

Sure, we can deduce that from general hardware design knowledge. But not every detail must be described in DT. Only the important ones.

...

...

...

If OMAP is too much of a pain, you can also make a separate binding for it, and a generic one for the rest of us.

No, omap isn't any pain at all.

The pain is that some other SoC are most easily defined by clocks in the gpu node which the omap doesn't need to explicitly specify.

I would expect a much bigger nightmare if we split this into two bindings variants.

...

I'd say that it's pretty unlikely that the clocks, interrupts (and even regulators) are optional. It might be fixed on some SoCs, but that's up to the DT to express that using fixed clocks / regulators, not the GPU binding itself.

omap already has these defined them not to be part of the GPU binding. The reason seems to be that this needs special clock gating control especially for idle states which is beyond simple clock-enable.

This sysc target-module@56000000 node is already merged and therefore we are only adding the gpu child node. Without defining clocks.

For example:

sgx_module: target-module@56000000 {
	compatible = "ti,sysc-omap4", "ti,sysc";
	reg = <0x5600fe00 0x4>,
	      <0x5600fe10 0x4>;
	reg-names = "rev", "sysc";
	ti,sysc-midle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	ti,sysc-sidle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
	clock-names = "fck";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x56000000 0x2000000>;

	gpu: gpu@0 {
		compatible = "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544";
		reg = <0x0 0x10000>;
		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
	};
};

The jz4780 example will like this:

gpu: gpu@13040000 { compatible = "ingenic,jz4780-sgx540-130", "img,sgx540-130", "img,sgx540"; reg = <0x13040000 0x4000>;

clocks = <&cgu JZ4780_CLK_GPU>;
clock-names = "gpu";

interrupt-parent = <&intc>;
interrupts = <63>;

};

So the question is which one is "generic for the rest of us"?

I'd say the latter.

Why?

TI SoC seem to be the broadest number of available users of sgx5xx in the past and nowadays. Others are more the exception.

And maybe TI has some complicated stuff around the GPU that others don't have?

Looks so.

...

If I look quickly at the Allwinner stuff, I see nothing looking alike in the SoC, so making the binding like that for everyone just because TI did something doesn't really make much sense.

That is why I propose to make the clocks optional. This solves both cases in a simple and neat way.

...

...

...

If your clock is optional, then you define it but don't mandate it. Not documenting it will only result in a mess where everyone will put some clock into it, possibly with different semantics each and every time.

So you mean that we should require a dummy clock for the omap gpu node or did I misunderstand that?

Well, yes there is of course a clock connection between the omap target-module and the sgx but it is IMHO pointless to describe it because it can't and does not need to be controlled separately.

As said the target-module is already accepted and upstream and my proposal is to get the gpu node described there. There is simply no need for a clocks node for the omap.

There is no need for a clocks property *currently* *on the OMAP*.

Yes. But why "currently"? Do you think the OMAPs we already have defined and tested will change?

Like I said, DVFS is likely to be one in the future.

...

...

What I also assume is that developers of DTS know what they do. So the risk that there is different semantics is IMHO very low.

Well, they know what they do if you document the binding. Let's say I have two clocks now on my SoC, and you just document that you want a clocks property, with a generic name in clock-names like "gpu".

Yes, that is what I want to propose for v7:

clocks: maxItems: 1

clock-names: maxItems: 1 items: - const: gpu

If you document what the "gpu" clock is supposed to be.

Is it the clock for the bus (clocking the register part of the GPU), the clock for the GPU cores? Something else?

...

...

If you agree I can add the clocks/clock-names property as an optional property. This should solve omap and all others.

With the above example, what clock should I put in there? In which order? This isn't some random example pulled out of nowhere. The Allwinner A31 has (at least) 4 clocks for the GPU, 1 reset line and 1 regulator, so I can only assume that the GPU actually needs at least that amount to be properly integrated into an SoC.

Ah, now I understand your motivation: you have access and experience with the A31 and you know that our proposal doesn't fit to it.

Not only the A31. If you don't document what your expectations are for a generic component like that, every SoC will assume that your GPU clock is something different and you won't be able to make any sense of it in your driver.

From what I know from your description is that the A31 is quite special with 4 GPU clocks... Are they all really for the GPU or 3 of them for the interface logic (like on OMAP which separates between "functional clocks" and "interface clocks")? Or are there 4 groups of GPU cores with a separate clock for each one?

1 is the equivalent of the interface clock, the others seem to be for the functional clocks.

So what would be your proposal for the A31 DT?

Then I get a chance to compare DT snippets and try to make a mixture for the bindings.

You'd have to know the GPU to do that, and I don't.

...

...

...

This has nothing to do with the binding being complete. And if you use a binding like this one, you'll be severely limited when you'll want to implement things like DVFS.

Now you have unhooked me... Nobody seems to know if and how DVFS can be applied to SGX. IMHO we should bake small bread first and get initial support into mainline.

On the software side, yes, of course. But the discussion here doesn't have much to do with software support, this is about the hardware. No matter if you enable DVFS or not, you'll have those resources connected to the GPU.

And if you want to enable the strict minimum in DT for now and expand it later as the software gains support for more stuff, then you'll have to deal with the minimal stuff in software later-on to keep the backward compatibility.

That is IMHO common practise everywhere. Sometimes you even have to adapt years old DT to new limitations of the drivers (this happened recently for combination of SPI and GPIO).

To some extent, yes. But those old bindings that turn out to be wrong at least contain most infos about the hardware, even though it's incomplete or flawed. Your proposal doesn't.

And you can still write two different drivers for a single bindings document or use the .data field of the compatibility table. And I think clocks and regulators can also be referenced by name if they are not defined in DT. This is not a "single variety" style, but a potential solution.

What I want to say: there are many roads to Rome.

What I want to say is: all the roads you listed above are going to be painful. Take your time, have a generic driver running from your generic binding you want to introduce on all the SoCs you want to support, and *then* start this discussion again.

Maxime

Hi Rob,

Am 16.04.2020 um 22:41 schrieb Rob Herring robh@kernel.org:

On Wed, 15 Apr 2020 10:35:08 +0200, "H. Nikolaus Schaller" wrote:

...

The Imagination PVR/SGX GPU is part of several SoC from multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, Allwinner A83 and others.

With this binding, we describe how the SGX processor is interfaced to the SoC (registers, interrupt etc.).

In most cases, Clock, Reset and power management is handled by a parent node or elsewhere (e.g. code in the driver).

Tested by make dt_binding_check dtbs_check

Signed-off-by: H. Nikolaus Schaller hns@goldelico.com

.../devicetree/bindings/gpu/img,pvrsgx.yaml | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml: while parsing a block mapping in "<unicode string>", line 74, column 13

It turned out that there was a stray " in line 74 from the last editing phase. Will be fixed in v7.

Would it be possible to make dt_binding_check not only report line/column but the contents of the line instead of "<unicode string>"?

did not find expected key in "<unicode string>", line 117, column 21 Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/gpu/img,pvrsgx.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/gpu/img,pvrsgx.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... Makefile:1264: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1270997

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

BR and thanks, Nikolaus Schaller

On Wed, 15 Apr 2020 at 10:36, H. Nikolaus Schaller hns@goldelico.com wrote:

and add interrupt.

Tested-by: H. Nikolaus Schaller hns@goldelico.com # Pyra-Handheld.

Don't add your own Tested-by tags. These are implied by authorship, otherwise all patches people make should have such tag.

Best regards, Krzysztof

On Wed, Apr 15, 2020 at 01:46:06PM +0200, H. Nikolaus Schaller wrote:

Hi Krzysztof,

...

Am 15.04.2020 um 13:38 schrieb Krzysztof Kozlowski krzk@kernel.org:

On Wed, 15 Apr 2020 at 10:36, H. Nikolaus Schaller hns@goldelico.com wrote:

...

and add interrupt.

Tested-by: H. Nikolaus Schaller hns@goldelico.com # Pyra-Handheld.

Don't add your own Tested-by tags. These are implied by authorship, otherwise all patches people make should have such tag.

Ok I see. AFAIR it originates in several phases of editing to report on which device it was tested.

Is there a canonical way of writing "tested-on: ${HARDWARE}"?

E.g. would this be ok?

Signed-off: H. Nikolaus Schaller hns@goldelico.com # tested on Pyra-Handheld

If you think tested platform is worth mentioning in the commit msg (it will stay there forever, ever, ever) then just add a line like:

"Add SGX GPU node. Tested on Pyra-Handheld."

From time to time we add such information to note that only one platform

was actually tested. I am not sure what benefit it brings to most cases... but your commit msg is so short that adding one more sentence seems reasonable. :)

Best regards, Krzysztof

Hi Maxime,

Am 15.04.2020 um 12:10 schrieb Maxime Ripard maxime@cerno.tech:

Hi,

On Wed, Apr 15, 2020 at 10:35:07AM +0200, H. Nikolaus Schaller wrote:

...

• rebased to v5.7-rc1
• added DTS for for a31, a31s, a83t - by Philipp Rossak embed3d@gmail.com
• added DTS for "samsung,s5pv210-sgx540-120" - by Jonathan Bakker xc-racer2@live.ca
• bindings.yaml fixes:

• added a31, a31
• fixes for omap4470
• jz4780 contains an sgx540-130 and not -120
• a83t contains an sgx544-115 and not -116
• removed "additionalProperties: false" because some SoC may need additional properties

PATCH V5 2020-03-29 19:38:32:

• reworked YAML bindings to pass dt_binding_check and be better grouped
• rename all nodes to "gpu: gpu@<address>"
• removed "img,sgx5" from example - suggested by Rob Herring robh+dt@kernel.org

PATCH V4 2019-12-17 19:02:11:

• MIPS: DTS: jz4780: removed "img,sgx5" from bindings
• YAML bindings: updated according to suggestions by Rob Herring
• MIPS: DTS: jz4780: insert-sorted gpu node by register address - suggested by Paul Cercueil

PATCH V3 2019-11-24 12:40:33:

• reworked YAML format with help by Rob Herring
• removed .txt binding document
• change compatible "ti,am335x-sgx" to "ti,am3352-sgx" - suggested by Tony Lindgren

PATCH V2 2019-11-07 12:06:17:

• tried to convert bindings to YAML format - suggested by Rob Herring
• added JZ4780 DTS node (proven to load the driver)
• removed timer and img,cores properties until we know we really need them - suggested by Rob Herring

PATCH V1 2019-10-18 20:46:35:

This patch series defines child nodes for the SGX5xx interface inside different SoC so that a driver can be found and probed by the compatible strings and can retrieve information about the SGX revision that is included in a specific SoC. It also defines the interrupt number to be used by the SGX driver.

There is currently no mainline driver for these GPUs, but a project [1] is ongoing with the goal to get the open-source part as provided by TI/IMG and others into drivers/gpu/drm/pvrsgx.

Just a heads up, DRM requires an open-source user-space, so if your plan is to move the open-source kernel driver while using the closed-source library (as that page seem to suggest), that might change a few things.

The far future goal is to arrive at a completely open implementation, but nobody knows how to get there. Therefore we bake smaller bread :)

step 1: get SoC integration right and stable (this is what this series is for) step 2: make the open source kernel driver work with closed-source libs step 3: write open-source replacements for user-space

...

The kernel modules built from this project have successfully demonstrated to work with the DTS definitions from this patch set on AM335x BeagleBone Black, DM3730 and OMAP5 Pyra and Droid 4. They partially work on OMAP3530 and PandaBoard ES but that is likely a problem in the kernel driver or the (non-free) user-space libraries and binaries.

Wotk for JZ4780 (CI20 board) is in progress and there is potential to extend this work to e.g. BananaPi-M3 (A83) and some Intel Poulsbo and CedarView devices.

If it's not been tested on any Allwinner board yet, I'll leave it aside until it's been properly shown to work.

Phillip has testes something on a83.

BR and thanks, Nikolaus

On Wed, Apr 15, 2020 at 02:41:52PM +0200, H. Nikolaus Schaller wrote:

...

...

The kernel modules built from this project have successfully demonstrated to work with the DTS definitions from this patch set on AM335x BeagleBone Black, DM3730 and OMAP5 Pyra and Droid 4. They partially work on OMAP3530 and PandaBoard ES but that is likely a problem in the kernel driver or the (non-free) user-space libraries and binaries.

Wotk for JZ4780 (CI20 board) is in progress and there is potential to extend this work to e.g. BananaPi-M3 (A83) and some Intel Poulsbo and CedarView devices.

If it's not been tested on any Allwinner board yet, I'll leave it aside until it's been properly shown to work.

Phillip has testes something on a83.

I'm a bit skeptical on that one since it doesn't even list the interrupts connected to the GPU that the binding mandates.

Maxime

Hi all,

On 15.04.20 15:04, H. Nikolaus Schaller wrote:

...

Am 15.04.2020 um 15:02 schrieb Maxime Ripard maxime@cerno.tech:

On Wed, Apr 15, 2020 at 02:41:52PM +0200, H. Nikolaus Schaller wrote:

...

...

...

The kernel modules built from this project have successfully demonstrated to work with the DTS definitions from this patch set on AM335x BeagleBone Black, DM3730 and OMAP5 Pyra and Droid 4. They partially work on OMAP3530 and PandaBoard ES but that is likely a problem in the kernel driver or the (non-free) user-space libraries and binaries.

Wotk for JZ4780 (CI20 board) is in progress and there is potential to extend this work to e.g. BananaPi-M3 (A83) and some Intel Poulsbo and CedarView devices.

If it's not been tested on any Allwinner board yet, I'll leave it aside until it's been properly shown to work.

Phillip has tested something on a83.

Yes I'm currently working on the a83t demo. The kernel module is loading correctly and the clocks, interrupts and resets seems to be working correctly.

I'm currently working on getting the users space driver working with the kernel driver. This is hopefully done soon.

...

I'm a bit skeptical on that one since it doesn't even list the interrupts connected to the GPU that the binding mandates.

I think he left it out for a future update. But best he comments himself.

I'm currently working on those bindings. They are now 90% done, but they are not finished till now. Currently there is some mainline support missing to add the full binding. The A83T and also the A31/A31s have a GPU Power Off Gating Register in the R_PRCM module, that is not supported right now in Mainline. The Register need to be written when the GPU is powered on and off.

@Maxime: I totally agree on your point that a demo needs to be provided before the related DTS patches should be provided. That's the reason why I added the gpu placeholder patches. Do you have an idea how a driver for the R_PRCM stuff can look like? I'm not that experienced with the clock driver framework.

The big question is right now how to proceed with the A83T and A31s patches. I see there three options, which one do you prefer?:

1. Provide now placeholder patches and send new patches, if everything is clear and other things are mainlined 2. Provide now patches as complete as possible and provide later patches to complete them when the R_PRCM things are mainlined 3. Leave them out, till the related work is mainlined and the bindings are final.

Since this GPU IP core is very flexible and the SOC manufactures can configure it on their needs, I think the binding will extend in the future. For example the SGX544 GPU is available in different configurations: there is a SGX544 core and SGX544MPx core. The x stands for the count of the USSE (Universal Scalable Shader Engine) cores. For example the GPU in the A83T is a MP1 and the A31/A31s a MP2. In addition to that some of the GPU's have also a 2D engine. There might be even more differences in the GPU's that we don't know right now and should be described in the Devicetree, but that's a different topic that we should keep in mind.

Cheers Philipp

Hi,

On 20.04.20 09:38, Maxime Ripard wrote:

Hi,

On Fri, Apr 17, 2020 at 02:09:06PM +0200, Philipp Rossak wrote:

...

...

...

I'm a bit skeptical on that one since it doesn't even list the interrupts connected to the GPU that the binding mandates.

I think he left it out for a future update. But best he comments himself.

I'm currently working on those bindings. They are now 90% done, but they are not finished till now. Currently there is some mainline support missing to add the full binding. The A83T and also the A31/A31s have a GPU Power Off Gating Register in the R_PRCM module, that is not supported right now in Mainline. The Register need to be written when the GPU is powered on and off.

@Maxime: I totally agree on your point that a demo needs to be provided before the related DTS patches should be provided. That's the reason why I added the gpu placeholder patches. Do you have an idea how a driver for the R_PRCM stuff can look like? I'm not that experienced with the clock driver framework.

It looks like a power-domain to me, so you'd rather plug that into the genpd framework.

I had a look on genpd and I'm not really sure if that fits.

It is basically some bit that verify that the clocks should be enabled or disabled. I think this is better placed somewhere in the clocking framework. I see there more similarities to the gating stuff. Do you think it is suitable to implement it like the clock gating?

...

The big question is right now how to proceed with the A83T and A31s patches. I see there three options, which one do you prefer?:

1. Provide now placeholder patches and send new patches, if everything is

clear and other things are mainlined 2. Provide now patches as complete as possible and provide later patches to complete them when the R_PRCM things are mainlined 3. Leave them out, till the related work is mainlined and the bindings are final.

Like I said, the DT *has* to be backward-compatible, so for any DT patch that you are asking to be merged, you should be prepared to support it indefinitely and be able to run from it, and you won't be able to change the bindings later on.

I agree on your points. But is this also suitable to drivers that are currently off tree and might be merged in one or two years?

...

Since this GPU IP core is very flexible and the SOC manufactures can configure it on their needs, I think the binding will extend in the future. For example the SGX544 GPU is available in different configurations: there is a SGX544 core and SGX544MPx core. The x stands for the count of the USSE (Universal Scalable Shader Engine) cores. For example the GPU in the A83T is a MP1 and the A31/A31s a MP2.

Mali is in the same situation and it didn't cause much trouble.

Good to know.

...

In addition to that some of the GPU's have also a 2D engine.

In the same memory region, running from the same interrupts, or is it a completely separate IP that happens to be sold by the same vendor?

What I know till now this is part of the PowerVR IP and not separated. So it should use the same memory region, clocks and interrupts.

Cheers Philipp

* Maxime Ripard maxime@cerno.tech [200421 11:22]:

On Tue, Apr 21, 2020 at 11:57:33AM +0200, Philipp Rossak wrote:

...

I had a look on genpd and I'm not really sure if that fits.

It is basically some bit that verify that the clocks should be enabled or disabled.

No, it can do much more than that. It's a framework to control the SoCs power domains, so clocks might be a part of it, but most of the time it's going to be about powering up a particular device.

Note that on omaps there are actually SoC module specific registers. And there can be multiple devices within a single target module on omaps. So the extra dts node and device is justified there.

For other SoCs, the SGX clocks are probably best handled directly in pvr-drv.c PM runtime functions unless a custom hardware wrapper with SoC specific registers exists.

Regards,

Tony

* H. Nikolaus Schaller hns@goldelico.com [200421 17:31]:

...

Am 21.04.2020 um 16:15 schrieb Tony Lindgren tony@atomide.com: Note that on omaps there are actually SoC module specific registers.

Ah, I see. This is of course a difference that the TI glue logic has its own registers in the same address range as the sgx and this can't be easily handled by a common sgx driver.

This indeed seems to be unique with omap.

...

And there can be multiple devices within a single target module on omaps. So the extra dts node and device is justified there.

For other SoCs, the SGX clocks are probably best handled directly in pvr-drv.c PM runtime functions unless a custom hardware wrapper with SoC specific registers exists.

That is why we need to evaluate what the better strategy is.

So we have a) omap which has a custom wrapper around the sgx b) others without, i.e. an empty (or pass-through) wrapper

Which one do we make the "standard" and which one the "exception"? What are good reasons for either one?

The wrapper is already handled by the ti-sysc binding, the sgx binding should be standard with optional clocks.

See for example the standard 8250 uart for am335x with:

$ git grep -B20 -A10 uart0 arch/arm/boot/dts/am33xx-l4.dtsi

The 8250 device configuration is described in the standard 8250 dts binding, and the am335x module in the ti-sysc binding. The are separate devices :)

So for the sgx binding, you can just leave out TI specific module wrapper completely from the example.

It also allows to handle different number of clocks (A31 seems to need 4, Samsung, A83 and JZ4780 one) without changing the sgx bindings or making big lists of conditionals. This variance would be handled outside the sgx core bindings and driver.

Well if other SoCs implement genpd domains etc, that's then again part of a separate binding and not part of the sgx binding.

Regards,

Tony

On Tue, Apr 21, 2020 at 07:29:32PM +0200, H. Nikolaus Schaller wrote:

...

Am 21.04.2020 um 16:15 schrieb Tony Lindgren tony@atomide.com:

• Maxime Ripard maxime@cerno.tech [200421 11:22]:

...

On Tue, Apr 21, 2020 at 11:57:33AM +0200, Philipp Rossak wrote:

...

I had a look on genpd and I'm not really sure if that fits.

It is basically some bit that verify that the clocks should be enabled or disabled.

No, it can do much more than that. It's a framework to control the SoCs power domains, so clocks might be a part of it, but most of the time it's going to be about powering up a particular device.

Note that on omaps there are actually SoC module specific registers.

Ah, I see. This is of course a difference that the TI glue logic has its own registers in the same address range as the sgx and this can't be easily handled by a common sgx driver.

This indeed seems to be unique with omap.

...

And there can be multiple devices within a single target module on omaps. So the extra dts node and device is justified there.

For other SoCs, the SGX clocks are probably best handled directly in pvr-drv.c PM runtime functions unless a custom hardware wrapper with SoC specific registers exists.

That is why we need to evaluate what the better strategy is.

So we have a) omap which has a custom wrapper around the sgx b) others without, i.e. an empty (or pass-through) wrapper

Which one do we make the "standard" and which one the "exception"? What are good reasons for either one?

I am currently in strong favour of a) being standard because it makes the pvr-drv.c simpler and really generic (independent of wrapping into any SoC).

This will likely avoid problems if we find more SoC with yet another scheme how the SGX clocks are wrapped.

It also allows to handle different number of clocks (A31 seems to need 4, Samsung, A83 and JZ4780 one) without changing the sgx bindings or making big lists of conditionals. This variance would be handled outside the sgx core bindings and driver.

I disagree. Every other GPU binding and driver is handling that just fine, and the SGX is not special in any case here.

Maxime

On Wed, Apr 22, 2020 at 09:10:57AM +0200, H. Nikolaus Schaller wrote:

...

Am 22.04.2020 um 08:58 schrieb Maxime Ripard maxime@cerno.tech:

On Tue, Apr 21, 2020 at 07:29:32PM +0200, H. Nikolaus Schaller wrote:

...

...

Am 21.04.2020 um 16:15 schrieb Tony Lindgren tony@atomide.com:

• Maxime Ripard maxime@cerno.tech [200421 11:22]:

...

On Tue, Apr 21, 2020 at 11:57:33AM +0200, Philipp Rossak wrote:

...

I had a look on genpd and I'm not really sure if that fits.

It is basically some bit that verify that the clocks should be enabled or disabled.

No, it can do much more than that. It's a framework to control the SoCs power domains, so clocks might be a part of it, but most of the time it's going to be about powering up a particular device.

Note that on omaps there are actually SoC module specific registers.

Ah, I see. This is of course a difference that the TI glue logic has its own registers in the same address range as the sgx and this can't be easily handled by a common sgx driver.

This indeed seems to be unique with omap.

...

And there can be multiple devices within a single target module on omaps. So the extra dts node and device is justified there.

For other SoCs, the SGX clocks are probably best handled directly in pvr-drv.c PM runtime functions unless a custom hardware wrapper with SoC specific registers exists.

That is why we need to evaluate what the better strategy is.

So we have a) omap which has a custom wrapper around the sgx b) others without, i.e. an empty (or pass-through) wrapper

Which one do we make the "standard" and which one the "exception"? What are good reasons for either one?

I am currently in strong favour of a) being standard because it makes the pvr-drv.c simpler and really generic (independent of wrapping into any SoC).

This will likely avoid problems if we find more SoC with yet another scheme how the SGX clocks are wrapped.

It also allows to handle different number of clocks (A31 seems to need 4, Samsung, A83 and JZ4780 one) without changing the sgx bindings or making big lists of conditionals. This variance would be handled outside the sgx core bindings and driver.

I disagree. Every other GPU binding and driver is handling that just fine, and the SGX is not special in any case here.

Can you please better explain this? With example or a description or a proposal?

I can't, I don't have any knowledge about this GPU.

I simply do not have your experience with "every other GPU" as you have. And I admit that I can't read from your statement what we should do to bring this topic forward.

So please make a proposal how it should be in your view.

If you need some inspiration, I guess you could look at the mali and vivante bindings once you have an idea of what the GPU needs across the SoCs it's integrated in.

Maxime

On 22/04/2020 18:09, H. Nikolaus Schaller wrote:

Hi Maxime,

...

Am 22.04.2020 um 17:13 schrieb Maxime Ripard maxime@cerno.tech:

On Wed, Apr 22, 2020 at 09:10:57AM +0200, H. Nikolaus Schaller wrote:

...

...

Am 22.04.2020 um 08:58 schrieb Maxime Ripard maxime@cerno.tech:

...

It also allows to handle different number of clocks (A31 seems to need 4, Samsung, A83 and JZ4780 one) without changing the sgx bindings or making big lists of conditionals. This variance would be handled outside the sgx core bindings and driver.

I disagree. Every other GPU binding and driver is handling that just fine, and the SGX is not special in any case here.

Can you please better explain this? With example or a description or a proposal?

I can't, I don't have any knowledge about this GPU.

Hm. Now I am fully puzzled. You have no knowledge about this GPU but disagree with our proposal? Is it just gut feeling?

Anyways, we need to find a solution. Together.

...

...

I simply do not have your experience with "every other GPU" as you have. And I admit that I can't read from your statement what we should do to bring this topic forward.

So please make a proposal how it should be in your view.

If you need some inspiration, I guess you could look at the mali and vivante bindings once you have an idea of what the GPU needs across the SoCs it's integrated in.

Well, I do not need inspiration, we need to come to an agreement about img,pvrsgx.yaml and we need some maintainer to finally pick it up.

I wonder how we can come to this stage.

If I look at vivante,gc.yaml or arm,mali-utgard.yaml I don't see big differences to what we propose and those I see seem to come from technical differences between sgx, vivante, mali etc. So there is no single scheme that fits all different gpu types.

One thing we can learn is that "core" seems to be a de facto standard for the core clock-name. An alternative "gpu" is used by nvidia,gk20a.txt.

Usually IPs needs a few clocks: - pclk or apb or reg: the clock clocking the "slave" bus to serve the registers - axi or bus or ahb: the bus clocking the the "master" bus to get data from system memory - core: the actual clock feeding the GPU logic

Sometimes you have a single clock for slave and master bus.

But you can also have separate clocks for shader cores, .. this depends on the IP and it's architecture. The IP can also have memories with separate clocks, etc...

But all these clocks can be source by an unique clock on a SoC, but different on another SoC, this is why it's important to list them all, even optional.

You'll certainly have at least a reset signal, and a power domain, these should exist and be optional.

Neil

BR and thanks, Nikolaus

---

linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Apr 23, 2020 at 05:45:55PM +0200, H. Nikolaus Schaller wrote:

...

Am 23.04.2020 um 17:00 schrieb Neil Armstrong narmstrong@baylibre.com:

...

One thing we can learn is that "core" seems to be a de facto standard for the core clock-name. An alternative "gpu" is used by nvidia,gk20a.txt.

Usually IPs needs a few clocks:

• pclk or apb or reg: the clock clocking the "slave" bus to serve the registers
• axi or bus or ahb: the bus clocking the the "master" bus to get data from system memory
• core: the actual clock feeding the GPU logic

And the sgx544 seems to have two such clocks.

...

Sometimes you have a single clock for slave and master bus.

But you can also have separate clocks for shader cores, .. this depends on the IP and it's architecture. The IP can also have memories with separate clocks, etc...

Indeed.

...

But all these clocks can be source by an unique clock on a SoC, but different on another SoC, this is why it's important to list them all, even optional.

You'll certainly have at least a reset signal, and a power domain, these should exist and be optional.

Well, they exist only as hints in block diagrams of some SoC data sheets (so we do not know if they represent the imagination definitions) and the current driver code doesn't make use of it. Still the gpu core works.

So I do not see any urgent need to add a complete list to the bindings now.

Unless some special SoC integration makes use of them. Then it is IMHO easier to extend the bindings by a follow-up patch than now thinking about all potential options and bloating the bindings with things we (the open source community) doesn't and can't know.

My goal is to keep the bindings as minimalistic as possible. And reset lines and power domains are (at least for those we have in the works) not needed to make working systems.

Therefore, for clocks I also would start with a minimalistic approach for a single optional GPU core clock and leave out reset and power completely.

Like I said above, the DT is considered an ABI and you'll have to maintain backward compatibility (ie, newer kernel running with older DT). Therefore, you won't be able to require a new clock, reset or power-domain later on for example.

I guess the question I'm really asking is: since you don't really know how the hardware is integrated at the moment, why should we have that discussion *now*. It's really not suprising that you don't know yet, so I'm not sure why we need to rush in the bindings.

Maxime

Hi,

On 21.04.20 13:21, Maxime Ripard wrote:

Hi,

On Tue, Apr 21, 2020 at 11:57:33AM +0200, Philipp Rossak wrote:

...

On 20.04.20 09:38, Maxime Ripard wrote:

...

Hi,

On Fri, Apr 17, 2020 at 02:09:06PM +0200, Philipp Rossak wrote:

...

...

...

I'm a bit skeptical on that one since it doesn't even list the interrupts connected to the GPU that the binding mandates.

I think he left it out for a future update. But best he comments himself.

I'm currently working on those bindings. They are now 90% done, but they are not finished till now. Currently there is some mainline support missing to add the full binding. The A83T and also the A31/A31s have a GPU Power Off Gating Register in the R_PRCM module, that is not supported right now in Mainline. The Register need to be written when the GPU is powered on and off.

@Maxime: I totally agree on your point that a demo needs to be provided before the related DTS patches should be provided. That's the reason why I added the gpu placeholder patches. Do you have an idea how a driver for the R_PRCM stuff can look like? I'm not that experienced with the clock driver framework.

It looks like a power-domain to me, so you'd rather plug that into the genpd framework.

I had a look on genpd and I'm not really sure if that fits.

It is basically some bit that verify that the clocks should be enabled or disabled.

No, it can do much more than that. It's a framework to control the SoCs power domains, so clocks might be a part of it, but most of the time it's going to be about powering up a particular device.

So I think I've found now the right piece of documentation and a driver that implements something similar [1].

So I will write a similar driver like linked above that only sets the right bits for A83T and A31/A31s. Do you think this is the right approach?

...

I think this is better placed somewhere in the clocking framework. I see there more similarities to the gating stuff. Do you think it is suitable to implement it like the clock gating?

I'm really not sure what makes you think that this should be modelled as a clock?

Looks like I looked in the wrong place and got some information that are not suitable for this.

...

...

...

The big question is right now how to proceed with the A83T and A31s patches. I see there three options, which one do you prefer?:

1. Provide now placeholder patches and send new patches, if everything is

clear and other things are mainlined 2. Provide now patches as complete as possible and provide later patches to complete them when the R_PRCM things are mainlined 3. Leave them out, till the related work is mainlined and the bindings are final.

Like I said, the DT *has* to be backward-compatible, so for any DT patch that you are asking to be merged, you should be prepared to support it indefinitely and be able to run from it, and you won't be able to change the bindings later on.

I agree on your points. But is this also suitable to drivers that are currently off tree and might be merged in one or two years?

This is what we done for the Mali. The devicetree binding was first done for the out-of-tree driver, and then lima/panfrost reused it.

The key thing here is to have enough confidence about how the hardware works so that you can accurately describe it.

Ok thanks! So I will resend my patches when the work got a more mature state and we know enough about the Hardware.

Cheers, Philipp

[1]: https://elixir.bootlin.com/linux/latest/source/drivers/soc/amlogic/meson-gx-...