On 2023-04-25 14:32:51, Abhinav Kumar wrote: <snip> > >>>>> We can return NULL from dpu_hw_foo_init(), which would mean that the > >>>>> block was skipped or is not present. > >>>> > >>>> An then replace the `if INTF_NONE continue` logic in dpu_rm_init with a > >>>> check for NULL that skips, and a check for IS_ERR` that goes to `fail`? > >>> > >>> You can just drop the INTF_NONE in dpu_rm. If dpu_hw_intf_init() > >>> returns NULL, the rest of the code in dpu_rm will work correctly. Sure, I'll keep the check exclusively in dpu_hw_intf_init(). Should I also move the pingpong==PINGPONG_MAX check into dpu_hw_lm_init()? > >> The only thing lost will be that the loop in the RM will break at the > >> first instance of NULL so if the loop has valid intf blocks later, those > >> will also not get initialized. > > > > No, it won't. There is the IS_ERR check, not the IS_ERR_OR_NULL() Only DSC currently uses IS_ERR_OR_NULL... We should fix that as rc=PTR_ERR(hw) on the next should be 0 (actually, the intent is for it to be undefined I think) for that... > Ack, but isnt that an issue since rm->hw_intf[intf->id - INTF_0] can be > assigned to a NULL hw. Yes, that is exactly the intent here. > >> That wont happen today because catalog doesnt have such entries but just > >> wanted to note what gets lost with this change. It does, we have a few SoCs with type=INTF_NONE. Quoting myself from above: As per the first patch of this series SM6115/QCM2290 only have a DSI interface which always sits at ID 1, and ID 0 has its TYPE set to INTF_NONE and is skipped. - Marijn