On 2023-07-30 02:18:10, Dmitry Baryshkov wrote: > On 29/07/2023 21:31, Marijn Suijten wrote: > > On 2023-07-29 02:59:32, Dmitry Baryshkov wrote: > >> On 27/07/2023 23:03, Marijn Suijten wrote: > >>> On 2023-07-27 19:20:58, Dmitry Baryshkov wrote: > >>>> The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0. > >>>> Rather than checking for the flag, check for the presense of the > >>>> corresponding interrupt line. > >>>> > >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>> > >>> That's a smart use of the interrupt field. I both like it, and I do > >>> not. While we didn't do any validation for consistency previously, this > >>> means we now have multiple ways of controlling available "features": > >>> > >>> - Feature flags on hardware blocks; > >>> - Presence of certain IRQs; > >>> - DPU core revision. > >> > >> I hesitated here too. For INTF it is clear that there is no other good > >> way to check for the TE feature. For PP we can just check for the DPU > >> revision. > > > > For both we could additionally check the DPU revision, and for INTF we > > could check for TYPE_DSI. Both would aid in extra validation, if we > > require the IRQ to be present or absent under these conditions. > > Yep, maybe that's better. > > > > > It might also help to document this, either here and on the respective > > struct fields so that catalog implementers know when they should supply > > or leave out an IRQ? > > Probably a WARN_ON would be enough. SGTM, it is after all only for bring-up as after that (additions have been validated, reviewed and merged) we "trust the kernel" including our catalog, so errors like this should pretty much be unreachable. - Marijn
