On Sat, Jul 18, 2026 at 12:39:19AM +0200, Leander Kieweg wrote: > 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: >
All of this sounds good, thanks.
> #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
signature.asc
Description: PGP signature
