Hi Conor,
Thanks for the review.
> Sadly googling this device only shows up Phoronix and other copy-paste
> reporting sites. I think it'd probably be good to include a link to your
> project either here or in the commit message.
Good point either way. The link was in the cover letter, but I agree
it makes sense to have it directly in the description too, so it's
not something you have to go dig up separately when looking at just
this patch.
> This clock sounds like it is actually required? Doubt the device works
> without it!
> pw-bot: changes-requested
You're right, I'll move clocks into the required list.
> Usually I would say do not include things if you don't have to, but I
> would much rather you did if the alternative is an interrupts property
> like this that doesn't resemble anything used in devicetrees.
The example was actually taken from my DE10-Standard DTS, just
shortened, since I understood examples are generally expected to be
minimal. For v2 I'll add interrupt-parent back in and use the
GIC_SPI/IRQ_TYPE_LEVEL_HIGH macros from
dt-bindings/interrupt-controller/arm-gic.h instead of raw numbers:
#include <dt-bindings/interrupt-controller/arm-gic.h>
gpu@c0000000 {
compatible = "glanda,gpu-1.0";
reg = <0xc0000000 0x1000000>;
interrupt-parent = <&intc>;
interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&osc1>;
};
Thanks again,
Leander
