From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/bridge: tc358768: Separate indirect register writes Date: Thu, 12 Mar 2026 07:14:50 +1000 Message-ID: In-Reply-To: <20260311-tc358768-v2-3-e75a99131bd5@ideasonboard.com> References: <20260311-tc358768-v2-0-e75a99131bd5@ideasonboard.com> <20260311-tc358768-v2-3-e75a99131bd5@ideasonboard.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Introduces `tc358768_confw_update_bits()` to abstract the indirect DSI_CONFW register write pattern. The function properly handles set/clear operations. One minor observation on operator precedence in the mask calculation at the call site: ```c mask = TC358768_DSI_CONTROL_TXMD | TC358768_DSI_CONTROL_HSCKMD | 0x3 << 1 | TC358768_DSI_CONTROL_EOTDIS; ``` The `<<` operator has higher precedence than `|`, so `0x3 << 1` evaluates to `0x6` before the OR, which is the intended behavior (lanes mask bits [2:1]). This expression was carried over from the original code, so it's correct but could benefit from parentheses for clarity: `(0x3 << 1)`. Very minor style nit. The `tc358768_confw_update_bits` logic itself is sound: if `mask != val` it clears the full mask first, then if `val & mask` is non-zero it sets the desired bits. This is an efficient approach that avoids unnecessary CLR when setting all bits, and avoids unnecessary SET when clearing all bits. --- Generated by Claude Code Patch Reviewer