Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Medium] The commit message claims all drivers have been converted to `atomic_create_state`, but two Cadence bridge drivers were missed. - [Low] Kernel-doc references to `drm_atomic_helper_bridge_reset()` were left intact despite the function being removed. Pre-existing issues: - [High] cdns_dsi_bridge_atomic_reset() fails to initialize the private object pointer in the state structure, which can lead to a NULL pointer dereference. -- --- Patch [74]: [PATCH v2 74/78] drm/atomic-state-helper: Remove drm_atomic_helper_bridge_reset() --- commit 5bf80d9ba302438fec05db1d7bd3c9f1680d8cd3 Author: Maxime Ripard <[email protected]> drm/atomic-state-helper: Remove drm_atomic_helper_bridge_reset() All drivers have been converted to the atomic_create_state callback and its drm_atomic_helper_bridge_create_state() helper. Remove the deprecated drm_atomic_helper_bridge_reset(). > All drivers have been converted to the atomic_create_state callback > and its drm_atomic_helper_bridge_create_state() helper. [Severity: Medium] Are all drivers fully converted? Code inspection shows two Cadence bridge drivers (cdns-dsi-core.c and cdns-mhdp8546-core.c) still assign .atomic_reset instead of being converted to the new callback. > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > b/drivers/gpu/drm/drm_atomic_state_helper.c > index 268cff1faad66..db42c3823b12a 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c [ ... ] > @@ -905,22 +905,6 @@ void __drm_atomic_helper_bridge_state_init(struct > drm_bridge_state *state, > } > EXPORT_SYMBOL(__drm_atomic_helper_bridge_state_init); > > -/** > - * drm_atomic_helper_bridge_reset() - Allocate and initialize a bridge state > - * to its default > - * @bridge: the bridge this state refers to > - * > - * Allocates the bridge state and initializes it to default values. This > helper > - * is meant to be used as a bridge &drm_bridge_funcs.atomic_reset hook for > - * bridges that don't subclass the bridge state. > - */ > -struct drm_bridge_state * > -drm_atomic_helper_bridge_reset(struct drm_bridge *bridge) > -{ > - return drm_atomic_helper_bridge_create_state(bridge); > -} > -EXPORT_SYMBOL(drm_atomic_helper_bridge_reset); > - [Severity: Low] Are there still kernel-doc references to drm_atomic_helper_bridge_reset() that need to be updated? For example, struct drm_bridge_funcs in include/drm/drm_bridge.h and drm_atomic_helper_bridge_destroy_state() in drivers/gpu/drm/drm_atomic_state_helper.c still refer to this removed function. While reviewing this refactoring, I noticed a pre-existing issue in one of the Cadence bridge drivers that hasn't been converted yet: drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c:cdns_dsi_bridge_atomic_reset() { struct cdns_dsi_bridge_state *dsi_state; dsi_state = kzalloc_obj(*dsi_state); if (!dsi_state) return ERR_PTR(-ENOMEM); memset(dsi_state, 0, sizeof(*dsi_state)); dsi_state->base.bridge = bridge; return &dsi_state->base; } [Severity: High] This isn't a bug introduced by this patch, but could this fail to initialize the private object pointer (state->base.base.obj) and leave it uninitialized (null)? Since it does not call __drm_atomic_helper_bridge_state_init() like the core helpers do, could this lead to a null pointer dereference when attempting to print the private object state later (for example, on a mode commit failure or debugfs read)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=74
