On 26/10/2025 08:27, Boris Brezillon wrote: > On Fri, 24 Oct 2025 21:21:15 +0100 > Karunika Choo <[email protected]> wrote: > >> +static bool panthor_fw_has_glb_state(struct panthor_device *ptdev) >> +{ >> + struct panthor_fw_global_iface *glb_iface = >> panthor_fw_get_glb_iface(ptdev); >> + >> + return glb_iface->control->version >= CSF_IFACE_VERSION(4, 1, 0); >> +} > > It's okay for now, but if we start having more of these, we probably > want to automate the generation of those has_<featurex>() helpers with > something like: > > #define FW_FEATURE(feat_name, major, minor) > \ > static bool panthor_fw_has_ ## feat_name(struct panthor_device *ptdev) > \ > { > \ > struct panthor_fw_global_iface *glb_iface = > panthor_fw_get_glb_iface(ptdev); \ > > \ > return glb_iface->control->version >= CSF_IFACE_VERSION(major, minor, > 0); \ > } > > Same goes for the HW features BTW. >
I wonder if at that point, would a bitmask as previously proposed be a cleaner solution? I have a minor bone to pick with MACROs that generate functions as they make finding its definition unnecessarily complicated and obtuse. Not to mention if the conditions for a feature changes and is disjoint from all the others, updating the macro to handle these would cause additional churn that I would really hope to avoid. >> + >> /** >> * panthor_fw_conv_timeout() - Convert a timeout into a cycle-count >> * @ptdev: Device. >> @@ -995,6 +1003,9 @@ static void panthor_fw_init_global_iface(struct >> panthor_device *ptdev) >> GLB_IDLE_EN | >> GLB_IDLE; >> >> + if (glb_iface->control->version >= CSF_IFACE_VERSION(4, 1, 0)) > > if (panthor_fw_has_glb_state(ptdev)) > Thanks for catching this. Will fix in v3. Kind regards, Karunika Choo >> + glb_iface->input->ack_irq_mask |= GLB_STATE_MASK; >> + >> panthor_fw_update_reqs(glb_iface, req, GLB_IDLE_EN, GLB_IDLE_EN); >> panthor_fw_toggle_reqs(glb_iface, req, ack, >> GLB_CFG_ALLOC_EN | >> @@ -1068,6 +1079,54 @@ static void panthor_fw_stop(struct panthor_device >> *ptdev) >> drm_err(&ptdev->base, "Failed to stop MCU"); >> } >
