Claude review: drm/panel: Add driver for Synaptics R63455 DSI panel

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

Patch Review

**Concept**: Panel driver managing two R63455-based panels as a single logical display for VR.

**Issues:**

1. **Build breakage — `pulse_offset_rows` does not exist**: The code uses:
   ```c
   ctx->backlight->props.pulse_offset_rows = GPO1_TES1;
   ```
   and in `r63455_bl_update_status`:
   ```c
   bl->props.pulse_offset_rows
   ```
   The field `pulse_offset_rows` does not exist in `struct backlight_properties` in the kernel. This will not compile. It appears to be a custom extension that was never submitted upstream, or a leftover from an out-of-tree tree. The brightness-related pulse timing should likely be handled within the panel driver directly rather than via backlight properties.

2. **Deprecated API — `of_device_get_match_data`**: Line:
   ```c
   ctx->dsi_init_seq = of_device_get_match_data(dev);
   ```
   Should use `device_get_match_data(dev)` instead. `of_device_get_match_data` is deprecated in favor of the bus-agnostic version.

3. **Missing `IS_ERR` check after `devm_drm_panel_alloc`**: The probe function does:
   ```c
   ctx = devm_drm_panel_alloc(&dsi->dev, __typeof(*ctx), panel,
                               &r63455_drm_funcs, DRM_MODE_CONNECTOR_DSI);
   ctx->dsi_init_seq = of_device_get_match_data(dev);
   ```
   There's no `IS_ERR(ctx)` check. `devm_drm_panel_alloc` can return `ERR_PTR(-ENOMEM)`, which would cause a NULL-pointer dereference on the next line.

4. **Unused header**: `#include <linux/of_device.h>` — with the switch to `device_get_match_data()`, this would be unnecessary. Even with `of_device_get_match_data`, this header is largely deprecated in favor of `<linux/property.h>`.

5. **Unused header**: `#include <linux/pinctrl/consumer.h>` — the driver doesn't use any pinctrl APIs.

6. **`LE16_BYTE0`/`LE16_BYTE1` and `BE16_BYTE0`/`BE16_BYTE1` macros are confusing**: These macros take a `val` that is a plain integer constant (e.g., `VBP = 100`), then call `le16_to_cpu()` or `be16_to_cpu()` on it. Since the input is a host-endian constant (not actually little- or big-endian data), this is incorrect usage of endian conversion functions. On little-endian ARM, `le16_to_cpu(100)` happens to be a no-op and `be16_to_cpu(100)` byte-swaps, but the semantic is wrong. These should just use plain shift/mask operations: `(val & 0xff)` and `((val >> 8) & 0xff)`.

7. **Error handling in `r63455_unprepare`**: The function continues disabling regulators even after errors, which is reasonable for cleanup, but it only returns the *last* error code. If `reg_lcd_bias_neg` disable fails but `reg_bl` disable succeeds, the error from `reg_lcd_bias_neg` is lost.

8. **No `remove` or cleanup for `dsi1_device`**: In probe, `mipi_dsi_device_register_full()` is called to create `dsi1_device`, but there's no corresponding `mipi_dsi_device_unregister()` in an error path or remove callback. Since only `devm_mipi_dsi_attach` is used (which handles detach), the device registration itself leaks on driver unbind. Consider using `devm_mipi_dsi_device_register_full()` if available, or adding cleanup.

9. **Function name typo**: `r63455_panel_on_boe_vs026c4m_n52_26000` — the compatible string is `boe,vs026c4m-n52-6000`, but the function name has `26000` (an extra `2`).

10. **`of_graph_get_remote_node` returns a reference**: Line:
    ```c
    dsi1 = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
    ```
    The `of_node_put(dsi1)` is correctly called a few lines later, but if `of_find_mipi_dsi_host_by_node` fails and returns `-EPROBE_DEFER`, the function returns without any issue. This is fine. However, `dsi1` is used as the node to find the host, but the host's node is the *parent* of the endpoint — `of_graph_get_remote_node` returns the remote port's parent device node, which should be correct for `of_find_mipi_dsi_host_by_node`. This looks correct.

11. **Mode `.clock` computation**: The clock calculation:
    ```c
    .clock = (2160 + 24 + 20 + 20) * (2160 + 404 + 1 + 20) * 120 * 2 / 1000,
    ```
    The `* 2` is for the doubled horizontal resolution. The result is `(2224) * (2585) * 120 * 2 / 1000 = 1,380,787.2` → truncated to `1380787` kHz (~1.38 GHz pixel clock). This seems extremely high for a DSI link but may be correct for a compressed/DSC mode. Worth double-checking.

---
Generated by Claude Code Patch Reviewer