Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The commit message falsely claims that the old `st7920_crtc_reset` 
freed the previous state.
--

--- Patch [63]: [PATCH 63/70] drm/sitronix: st7920: Convert to 
atomic_create_state ---
commit 51126b7e2261e0176cf52ba6e5a5273dcbb0592e
Author: Maxime Ripard <[email protected]>
drm/sitronix: st7920: Convert to atomic_create_state

This commit converts the st7920 CRTC implementation to use the
atomic_create_state hook rather than a custom reset hook.

> The st7920 crtc implementation provides a custom reset hook. However,
> this hook only allocates the state, initializes it with
> __drm_atomic_helper_crtc_reset(), and frees the previous state. It
> does not perform any hardware reset.

[Severity: Medium]
Is this description accurate regarding the previous state being freed?

Looking at the original st7920_crtc_reset() implementation before this
patch, it warns if crtc->state is non-NULL but does not appear to free
the old state memory:

        drm_WARN_ON_ONCE(crtc->dev, crtc->state);

        st7920_state = kzalloc_obj(*st7920_state);
        if (!st7920_state)
                return;

        __drm_atomic_helper_crtc_reset(crtc, &st7920_state->base);

Could the commit message be updated to clarify that the old hook was not
actually freeing the previous state?

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=63

Reply via email to