Claude review: drm/tegra: dsi: add support for Tegra20/Tegra30

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

Patch Review

**Generally well done.** The `tegra_dsi_config` approach with boolean flags is appropriate and keeps the code readable.

**Minor issues:**

1. **Comment style (kernel convention violation):** The multi-line comment block for `tegra124_dsi_config` uses `/* TODO:` on a single line. This is fine, but worth noting the convention. More importantly, the `XXX` comment that was moved into the `has_multiple_pad_controls` branch at `dsi.c:676-678` — while this is just code motion and preserves the existing comment, it might be worth updating or removing it since this is a good opportunity.

2. **`devm_clk_get_optional` change for `clk_lp`:** The change from `devm_clk_get()` to `devm_clk_get_optional()` at line 1635 is correct for Tegra20/30 which lack this clock, but the subsequent error check:
   ```c
   dsi->clk_lp = devm_clk_get_optional(&pdev->dev, "lp");
   if (IS_ERR(dsi->clk_lp)) {
   ```
   This is still correct — `devm_clk_get_optional` returns NULL if the clock doesn't exist, and returns an error pointer only for real errors, so the `IS_ERR` check is valid.

3. **`tegra_dsi_setup_clocks` comment:** The added comment at `dsi.c:1191-1195`:
   ```c
   /*
    * Tegra124+ uses a clock gate, not a mux, so this step
    * should be redundant for configuration; yet, DSI refuses
    * to work without it.
    */
   ```
   This is informative but the comment is placed *before* the `if (dsi->config->has_mux_parent_clk)` guard, so on Tegra124+ this function *is* still called (since `has_mux_parent_clk = true`). The comment seems to be explaining why `has_mux_parent_clk` is set to `true` on Tegra124+, but its placement inside `tegra_dsi_setup_clocks()` rather than next to the config definition is slightly confusing. The `TODO` comment on `tegra124_dsi_config` at line 1741 is clearer.

4. **Tegra30 config identical to Tegra20:** Both `tegra20_dsi_config` and `tegra30_dsi_config` have the same values. The explanatory comment about Tegra30 PLL_D/PLL_D2 muxing justifies having a separate struct for future use, so this is acceptable.

5. **No functional concern with `device_get_match_data`:** The NULL check after `device_get_match_data()` returning `-ENODEV` is the right pattern for a required match data.

**No blocking issues.** Already has Reviewed-by from Mikko Perttunen.

---
Generated by Claude Code Patch Reviewer