Claude review: drm/bridge: analogix_dp: Add support for optional data-lanes mapping

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

Patch Review

**Status: Needs revision — potential mapping inversion bug and minor issues**

#### 1. Potential lane mapping semantic bug (important)

The `data-lanes` DT standard (from `video-interfaces.yaml`) defines: **position = logical lane index, value = physical lane index**. So `data-lanes = <1 2 3 0>` means logical lane 0 is on physical lane 1, etc.

The register `ANALOGIX_DP_LANE_MAP` maps in the opposite direction: `LANE_N_MAP` specifies which **logical** lane **physical** lane N carries.

The code writes the `data-lanes` array directly into the register:
```c
reg = lane_map[0] << LANE0_MAP_SHIFT;
reg |= lane_map[1] << LANE1_MAP_SHIFT;
reg |= lane_map[2] << LANE2_MAP_SHIFT;
reg |= lane_map[3] << LANE3_MAP_SHIFT;
```

This treats `data-lanes[physical] = logical`, but the standard says `data-lanes[logical] = physical`. These interpretations yield the same result only for self-inverse permutations (involutions) like `<1 0 3 2>` or `<3 2 1 0>`. For a permutation like `<1 2 3 0>`, the mapping would be wrong.

For example, `data-lanes = <1 2 3 0>` means:
- Logical 0 → Physical 1, Logical 1 → Physical 2, Logical 2 → Physical 3, Logical 3 → Physical 0

The correct register values should be the **inverse** permutation:
- Physical 0 carries Logical 3 → LANE0_MAP = 3
- Physical 1 carries Logical 0 → LANE1_MAP = 0

But the code writes LANE0_MAP = 1, LANE1_MAP = 2, which is incorrect.

**Recommendation**: Either compute the inverse permutation when programming the register, or document clearly in the binding that this driver interprets `data-lanes` as physical-to-logical (non-standard). The former is preferred for DT convention compliance.

#### 2. Redundant endpoint lookup

The endpoint node is fetched twice — once inside `drm_of_get_data_lanes_count_ep()` and once explicitly:
```c
num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
                                           video_info->max_lane_count);
...
endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
```

Consider using `of_graph_get_endpoint_by_regs()` once, then calling `drm_of_get_data_lanes_count()` (the non-`_ep` variant) on the resulting endpoint node directly, reading the property from the same node.

#### 3. Inconsistent endpoint reg parameter

The first call uses `reg=0`:
```c
drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, ...)
```
The second uses `reg=-1` (wildcard):
```c
of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1)
```

If there were multiple endpoints under port@1, these could find different endpoints. Use the same value in both calls to ensure consistency.

#### 4. Early default initialization is partially redundant

```c
u32 map[LANE_COUNT4] = {0, 1, 2, 3};
...
memcpy(video_info->lane_map, map, sizeof(map));
```

The `memcpy` at line 230 sets the default immediately, which is good for the early-return paths. But the `map` array itself is initialized at declaration with the same values, so this is fine functionally — just slightly redundant since the two copies serve the same purpose.

#### 5. Minor style: unnecessary parentheses on shift defines

```c
#define LANE3_MAP_SHIFT    (6)
#define LANE2_MAP_SHIFT    (4)
```

Kernel coding style doesn't require parentheses around simple integer constants. Consider `#define LANE3_MAP_SHIFT 6` for consistency with the existing defines in the same file (e.g., `(0x0 << 6)` uses parentheses around expressions, not bare constants).

#### 6. The rename from `analogix_dp_lane_swap` to `analogix_dp_lane_mapping` is clean

The old boolean `enable` parameter is replaced by reading from `dp->video_info.lane_map`, and the single call site in `analogix_dp_reset()` is updated. No external callers exist. This is a clean API evolution.

---
Generated by Claude Code Patch Reviewer