Claude review: drm: Introduce drmm_connector_dp_init() with link training state properties

Claude Code Review Bot
2026-04-12T00:51:25Z

Patch Review

This is the core of the series and has several issues:

**Bug -- kerneldoc mismatch:**
```c
/**
* drm_connector_dp_link_reset_properties() - Reset DisplayPort link configuration
* @connector: DRM connector
* @dp_link: Link training informations
*/
int
drm_connector_dp_set_link_train_properties(...)
```
The kerneldoc name says `reset` but the function is `set`. The `@dp_link` parameter name doesn't match the actual parameter `dp_link_train`. "informations" should be "information".

**Bug -- copyright header:**
```c
* Copyright (C) 2026 Google
* Author: Kory Maincent <kory.maincent@bootlin.com>
```
Author is from Bootlin, copyright says Google. This is likely wrong.

**Design issue -- no uevent on property change:** `drm_object_property_set_value()` updates the stored value but does not send a uevent to userspace. There is no mechanism for userspace to learn that link training has completed or that values have changed without polling. For a monitoring/diagnostic feature, this severely limits usefulness. Consider calling `drm_sysfs_connector_property_event()` or similar after updating properties.

**Design issue -- bitmask for single current values:** The `nlanes` property uses bitmask semantics. When reporting current state (e.g., "using 2 lanes"), the driver sets `nlanes = DRM_DP_2LANE = BIT(1)`. A bitmask with a single bit set is semantically an enum value, not a bitmask. The same bitmask property type is used for both "supported lane counts" (multiple bits) and "current lane count" (single bit), which is confusing. Consider using enum properties for runtime state.

**Performance concern -- link rate lookup via string parsing:**
```c
if (!kstrtou32(prop_enum->name, 10, &parsed_rate) &&
dp_link_train->rate * 10 == parsed_rate) {
```
This iterates over enum entries and parses string names to find a matching rate. This is called on every link training completion. Storing the rates as integers and comparing directly would be more efficient and less fragile.

**Minor -- `{0}` initialization:**
```c
struct drm_connector_dp_link_train dp_link_train = {0};
```
Preferred kernel style is `= { }` or `= { 0 }` (with space). The `{0}` form works but is not idiomatic in kernel code.

---
Generated by Claude Code Patch Reviewer