Claude review: drm/tyr: add parser for firmware binary

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Author:** Daniel Almeida / Beata Michalska / Boris Brezillon / Deborah Brouwer

The firmware parser is well-structured with a Cursor abstraction for sequential reading.

1. `EntryHeader::size()` extracts bits 8-15:

```rust
+    fn size(&self) -> u32 {
+        self.0 >> 8 & 0xff
+    }
```

This means the maximum entry size is 255 bytes. This matches the C panthor driver's `CSF_FW_BINARY_ENTRY_SIZE` extraction. Worth a comment documenting this limit.

2. The `Cursor::view()` method checks `range.start < self.pos`:

```rust
+    fn view(&self, range: Range<usize>) -> Result<Cursor<'_>> {
+        if range.start < self.pos || range.end > self.data.len() {
```

This prevents viewing data before the current position. The intent is to prevent re-reading already-consumed data. However, the comparison should probably be `range.start < self.pos` should be `range.start < 0` or removed — since `view()` is used with `self.cursor.pos()..self.cursor.pos() + section_hdr_size`, `range.start` always equals `self.pos`, so the check `range.start < self.pos` will never trigger. This is dead validation code. Not a bug, just unnecessary.

3. The section name is parsed and converted but stored in `_name` (unused):

```rust
+        let _name = CStr::from_bytes_with_nul(&name)
+            .ok()
+            .and_then(|name| CString::try_from(name).ok());
```

This is presumably for future use (logging, debugging). Fine to keep.

4. In `parse_entry()`, the error handling for unknown entry types has slightly convoluted logic:

```rust
+                entry_type => {
+                    if entry_type.is_err() || !entry_section.entry_hdr.optional() {
+                        if !entry_section.entry_hdr.optional() {
+                            ...
+                            Err(EINVAL)
+                        } else {
+                            Ok(entry_section)
+                        }
+                    } else {
+                        Ok(entry_section)
+                    }
+                }
```

The outer `if` checks `is_err() || !optional()`, then the inner `if` checks `!optional()`. If `is_err()` is true AND `optional()` is true, the outer if is true but the inner if is false, falling through to `Ok(entry_section)`. This is logically correct (unknown-but-optional entries are silently skipped), but the nesting is harder to follow than it needs to be. Could be simplified to:

```rust
if !entry_section.entry_hdr.optional() {
    pr_err!(...);
    Err(EINVAL)
} else {
    Ok(entry_section)
}
```

Since if it's optional, we skip it regardless of whether the type is recognized.

---
Generated by Claude Code Patch Reviewer