* [PATCH 0/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
@ 2026-05-12 16:46 Boerge Struempfel
2026-05-12 16:46 ` [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property Boerge Struempfel
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Boerge Struempfel @ 2026-05-12 16:46 UTC (permalink / raw)
To: Marek Vasut, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-kernel, Boerge Struempfel
The LT9211 LVDS TX output driver current is currently hardcoded to
0x8 (~25 uA). Some board layouts require a different drive strength
to meet signal integrity requirements.
This series adds support for the standard 'drive-strength-microamp'
DT property, allowing board DTs to select one of sixteen discrete
current levels between 12 uA and 36 uA. The default preserves the
existing behaviour.
Boerge Struempfel (2):
dt-bindings: display/bridge: lt9211: Add drive-strength-microamp
property
drm/bridge: lt9211: Add drive-strength-microamp DT property
.../display/bridge/lontium,lt9211.yaml | 7 +++++
drivers/gpu/drm/bridge/lontium-lt9211.c | 28 ++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
--
2.54.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property
2026-05-12 16:46 [PATCH 0/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
@ 2026-05-12 16:46 ` Boerge Struempfel
2026-05-12 17:05 ` Conor Dooley
2026-05-12 16:46 ` [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
2 siblings, 1 reply; 11+ messages in thread
From: Boerge Struempfel @ 2026-05-12 16:46 UTC (permalink / raw)
To: Marek Vasut, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-kernel, Boerge Struempfel
Add the 'drive-strength-microamp' property to allow board DTs to
configure the LT9211 LVDS TX output driver current. Sixteen discrete
levels are supported, ranging from 12 uA to 36 uA. Defaults to 25 uA.
Signed-off-by: Boerge Struempfel <bstruempfel@data-modul.com>
---
.../devicetree/bindings/display/bridge/lontium,lt9211.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt9211.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt9211.yaml
index 9a6e9b25d14a..381b69c761b8 100644
--- a/Documentation/devicetree/bindings/display/bridge/lontium,lt9211.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9211.yaml
@@ -31,6 +31,13 @@ properties:
vccio-supply:
description: Regulator for 1.8V IO power.
+ drive-strength-microamp:
+ description:
+ LVDS TX output driver current. Sixteen discrete levels are supported,
+ corresponding to the following nominal values in microamps.
+ enum: [12, 14, 16, 17, 19, 20, 22, 23, 25, 27, 28, 30, 31, 33, 34, 36]
+ default: 25
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-12 16:46 [PATCH 0/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
2026-05-12 16:46 ` [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property Boerge Struempfel
@ 2026-05-12 16:46 ` Boerge Struempfel
2026-05-12 18:29 ` Marek Vasut
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
2 siblings, 1 reply; 11+ messages in thread
From: Boerge Struempfel @ 2026-05-12 16:46 UTC (permalink / raw)
To: Marek Vasut, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-kernel, Boerge Struempfel
The LT9211 LVDS TX output driver current (RG_MLTX_HSDRV_ISEL) was
previously hardcoded to 0x8 (~25 uA), which may not be optimal for all
board layouts.
The hardware supports 16 discrete current levels starting at 12.5 uA
with a step of 1.5625 uA. These are exposed as rounded integer microamp
values in the lookup table.
Add support for the 'drive-strength-microamp' DT property. A lookup
table maps the sixteen supported microamp values (12..36 uA) to the
corresponding register field. Defaults to 25 uA when the property is
absent, preserving the existing behaviour.
Signed-off-by: Boerge Struempfel <bstruempfel@data-modul.com>
---
drivers/gpu/drm/bridge/lontium-lt9211.c | 28 ++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt9211.c b/drivers/gpu/drm/bridge/lontium-lt9211.c
index 03fc8fd10f20..402c639ac515 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9211.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9211.c
@@ -40,6 +40,11 @@
/* DSI lane count - 0 means 4 lanes ; 1, 2, 3 means 1, 2, 3 lanes. */
#define REG_DSI_LANE_COUNT(n) ((n) & 3)
+/* Maps register value (index) to drive-strength-microamp DT property value */
+static const u32 lt9211_hsdrv_microamp[] = {
+ 12, 14, 16, 17, 19, 20, 22, 23, 25, 27, 28, 30, 31, 33, 34, 36
+};
+
struct lt9211 {
struct drm_bridge bridge;
struct device *dev;
@@ -50,6 +55,7 @@ struct lt9211 {
struct regulator *vccio;
bool lvds_dual_link;
bool lvds_dual_link_even_odd_swap;
+ u8 lvds_hsdrv_isel;
};
static const struct regmap_range lt9211_rw_ranges[] = {
@@ -374,7 +380,8 @@ static int lt9211_configure_tx(struct lt9211 *ctx, bool jeida,
/* BIT(7) is LVDS dual-port */
{ 0x823b, 0x38 | (ctx->lvds_dual_link ? BIT(7) : 0) },
{ 0x823e, 0x92 },
- { 0x823f, 0x48 },
+ /* bits 3:0: RG_MLTX_HSDRV_ISEL, LVDS TX driver current */
+ { 0x823f, 0x40 | ctx->lvds_hsdrv_isel },
{ 0x8240, 0x31 },
{ 0x8243, 0x80 },
{ 0x8244, 0x00 },
@@ -629,7 +636,9 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
struct device *dev = ctx->dev;
struct drm_panel *panel;
int dual_link;
+ u32 microamp;
int ret;
+ int i;
ctx->vccio = devm_regulator_get(dev, "vccio");
if (IS_ERR(ctx->vccio))
@@ -666,6 +675,23 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
ctx->panel_bridge = panel_bridge;
+ ctx->lvds_hsdrv_isel = 8; /* default: 25 uA */
+ ret = of_property_read_u32(dev->of_node, "drive-strength-microamp",
+ µamp);
+ if (!ret) {
+ for (i = 0; i < ARRAY_SIZE(lt9211_hsdrv_microamp); i++) {
+ if (lt9211_hsdrv_microamp[i] == microamp) {
+ ctx->lvds_hsdrv_isel = i;
+ break;
+ }
+ }
+ if (i == ARRAY_SIZE(lt9211_hsdrv_microamp)) {
+ dev_err(dev, "Invalid drive-strength-microamp value %u\n",
+ microamp);
+ return -EINVAL;
+ }
+ }
+
return 0;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property
2026-05-12 16:46 ` [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property Boerge Struempfel
@ 2026-05-12 17:05 ` Conor Dooley
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
0 siblings, 1 reply; 11+ messages in thread
From: Conor Dooley @ 2026-05-12 17:05 UTC (permalink / raw)
To: Boerge Struempfel
Cc: Marek Vasut, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
dri-devel, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 429 bytes --]
On Tue, May 12, 2026 at 06:46:08PM +0200, Boerge Struempfel wrote:
> Add the 'drive-strength-microamp' property to allow board DTs to
> configure the LT9211 LVDS TX output driver current. Sixteen discrete
> levels are supported, ranging from 12 uA to 36 uA. Defaults to 25 uA.
>
> Signed-off-by: Boerge Struempfel <bstruempfel@data-modul.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-12 16:46 ` [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
@ 2026-05-12 18:29 ` Marek Vasut
2026-05-13 9:02 ` Börge Strümpfel
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
0 siblings, 2 replies; 11+ messages in thread
From: Marek Vasut @ 2026-05-12 18:29 UTC (permalink / raw)
To: Boerge Struempfel, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-kernel
On 5/12/26 6:46 PM, Boerge Struempfel wrote:
> +/* Maps register value (index) to drive-strength-microamp DT property value */
> +static const u32 lt9211_hsdrv_microamp[] = {
This can be u8 .
> + 12, 14, 16, 17, 19, 20, 22, 23, 25, 27, 28, 30, 31, 33, 34, 36
> +};
> +
> struct lt9211 {
> struct drm_bridge bridge;
> struct device *dev;
> @@ -50,6 +55,7 @@ struct lt9211 {
> struct regulator *vccio;
> bool lvds_dual_link;
> bool lvds_dual_link_even_odd_swap;
> + u8 lvds_hsdrv_isel;
> };
>
> static const struct regmap_range lt9211_rw_ranges[] = {
> @@ -374,7 +380,8 @@ static int lt9211_configure_tx(struct lt9211 *ctx, bool jeida,
> /* BIT(7) is LVDS dual-port */
> { 0x823b, 0x38 | (ctx->lvds_dual_link ? BIT(7) : 0) },
> { 0x823e, 0x92 },
> - { 0x823f, 0x48 },
> + /* bits 3:0: RG_MLTX_HSDRV_ISEL, LVDS TX driver current */
> + { 0x823f, 0x40 | ctx->lvds_hsdrv_isel },
> { 0x8240, 0x31 },
> { 0x8243, 0x80 },
> { 0x8244, 0x00 },
> @@ -629,7 +636,9 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
> struct device *dev = ctx->dev;
> struct drm_panel *panel;
> int dual_link;
> + u32 microamp;
> int ret;
> + int i;
>
> ctx->vccio = devm_regulator_get(dev, "vccio");
> if (IS_ERR(ctx->vccio))
> @@ -666,6 +675,23 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
>
> ctx->panel_bridge = panel_bridge;
>
> + ctx->lvds_hsdrv_isel = 8; /* default: 25 uA */
> + ret = of_property_read_u32(dev->of_node, "drive-strength-microamp",
> + µamp);
if ret != 0 , then what happens here ?
> + if (!ret) {
> + for (i = 0; i < ARRAY_SIZE(lt9211_hsdrv_microamp); i++) {
> + if (lt9211_hsdrv_microamp[i] == microamp) {
> + ctx->lvds_hsdrv_isel = i;
> + break;
> + }
> + }
> + if (i == ARRAY_SIZE(lt9211_hsdrv_microamp)) {
> + dev_err(dev, "Invalid drive-strength-microamp value %u\n",
> + microamp);
> + return -EINVAL;
> + }
> + }
> +
> return 0;
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-12 18:29 ` Marek Vasut
@ 2026-05-13 9:02 ` Börge Strümpfel
2026-05-13 11:33 ` Marek Vasut
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 11+ messages in thread
From: Börge Strümpfel @ 2026-05-13 9:02 UTC (permalink / raw)
To: Marek Vasut
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
linux-kernel
Hello Marek,
Thank you for your feedback,
On Tue, May 12, 2026 at 08:29:55PM +0200, Marek Vasut wrote:
> On 5/12/26 6:46 PM, Boerge Struempfel wrote:
>
> > +/* Maps register value (index) to drive-strength-microamp DT property value */
> > +static const u32 lt9211_hsdrv_microamp[] = {
>
> This can be u8 .
>
you are right, I'll change that in the v2.
> > + 12, 14, 16, 17, 19, 20, 22, 23, 25, 27, 28, 30, 31, 33, 34, 36
> > +};
> > +
> > struct lt9211 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > @@ -50,6 +55,7 @@ struct lt9211 {
> > struct regulator *vccio;
> > bool lvds_dual_link;
> > bool lvds_dual_link_even_odd_swap;
> > + u8 lvds_hsdrv_isel;
> > };
> >
> > static const struct regmap_range lt9211_rw_ranges[] = {
> > @@ -374,7 +380,8 @@ static int lt9211_configure_tx(struct lt9211 *ctx, bool jeida,
> > /* BIT(7) is LVDS dual-port */
> > { 0x823b, 0x38 | (ctx->lvds_dual_link ? BIT(7) : 0) },
> > { 0x823e, 0x92 },
> > - { 0x823f, 0x48 },
> > + /* bits 3:0: RG_MLTX_HSDRV_ISEL, LVDS TX driver current */
> > + { 0x823f, 0x40 | ctx->lvds_hsdrv_isel },
> > { 0x8240, 0x31 },
> > { 0x8243, 0x80 },
> > { 0x8244, 0x00 },
> > @@ -629,7 +636,9 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
> > struct device *dev = ctx->dev;
> > struct drm_panel *panel;
> > int dual_link;
> > + u32 microamp;
> > int ret;
> > + int i;
> >
> > ctx->vccio = devm_regulator_get(dev, "vccio");
> > if (IS_ERR(ctx->vccio))
> > @@ -666,6 +675,23 @@ static int lt9211_parse_dt(struct lt9211 *ctx)
> >
> > ctx->panel_bridge = panel_bridge;
> >
> > + ctx->lvds_hsdrv_isel = 8; /* default: 25 uA */
> > + ret = of_property_read_u32(dev->of_node, "drive-strength-microamp",
> > + µamp);
>
> if ret != 0 , then what happens here ?
>
if ret != 0, we will keep the default value of 8 (corresponding to 25 uA
as written above), which is the behavior that the driver had previously.
According to the documentation, of_property_read_u32() will return 0 on
success, -EINVAL if the property does not exist, -ENODATA if property
does not have a value, and -EOVERFLOW if the property data isn't large
enough. I think we only would need to give a warning or similar if the
-ENODATA or -EOVERFLOW cases. However, I personally do not think that is
necessary, as the usage is specified unambiguously in the devicetree
bindings.
If you feel like we should add a warning in those cases, I can add them
however.
> > + if (!ret) {
> > + for (i = 0; i < ARRAY_SIZE(lt9211_hsdrv_microamp); i++) {
> > + if (lt9211_hsdrv_microamp[i] == microamp) {
> > + ctx->lvds_hsdrv_isel = i;
> > + break;
> > + }
> > + }
> > + if (i == ARRAY_SIZE(lt9211_hsdrv_microamp)) {
> > + dev_err(dev, "Invalid drive-strength-microamp value %u\n",
> > + microamp);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > return 0;
> [...]
>
--
Best regards,
Börge Strümpfel
DATA MODUL AG
Landsberger Str. 322
80687 München
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-13 9:02 ` Börge Strümpfel
@ 2026-05-13 11:33 ` Marek Vasut
0 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2026-05-13 11:33 UTC (permalink / raw)
To: Börge Strümpfel
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
linux-kernel
On 5/13/26 11:02 AM, Börge Strümpfel wrote:
[...]
>>> + ctx->lvds_hsdrv_isel = 8; /* default: 25 uA */
>>> + ret = of_property_read_u32(dev->of_node, "drive-strength-microamp",
>>> + µamp);
>>
>> if ret != 0 , then what happens here ?
>>
>
> if ret != 0, we will keep the default value of 8 (corresponding to 25 uA
> as written above), which is the behavior that the driver had previously.
>
> According to the documentation, of_property_read_u32() will return 0 on
> success, -EINVAL if the property does not exist, -ENODATA if property
> does not have a value, and -EOVERFLOW if the property data isn't large
> enough. I think we only would need to give a warning or similar if the
> -ENODATA or -EOVERFLOW cases. However, I personally do not think that is
> necessary, as the usage is specified unambiguously in the devicetree
> bindings.
Users can pass in malformed DT, so a warning is a good idea. Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Claude review: drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-13 13:31 [PATCH v2 0/2] " Boerge Struempfel
@ 2026-05-16 1:56 ` Claude Code Review Bot
0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:56 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/bridge: lt9211: Add drive-strength-microamp DT property
Author: Boerge Struempfel <bstruempfel@data-modul.com>
Patches: 8
Reviewed: 2026-05-16T11:56:03.253260
---
This is a clean, straightforward 2-patch series that adds a configurable LVDS TX drive strength to the LT9211 bridge driver via the standard `drive-strength-microamp` DT property. The series is well-structured: patch 1 adds the DT binding, patch 2 adds the driver implementation. The default value (25 uA, register index 8) preserves backward compatibility with the previously hardcoded `0x48`. The register-level change from `0x48` to `0x40 | ctx->lvds_hsdrv_isel` is correct — the default index 8 produces `0x40 | 0x08 = 0x48`, matching the original. The lookup table values are internally consistent with the described formula (12.5 uA base + 1.5625 uA steps, rounded to integer).
No functional issues found. A couple of minor style observations below.
**Verdict: Series looks good.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 11+ messages in thread
* Claude review: drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-12 16:46 [PATCH 0/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
2026-05-12 16:46 ` [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property Boerge Struempfel
2026-05-12 16:46 ` [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
@ 2026-05-16 3:11 ` Claude Code Review Bot
2 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:11 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/bridge: lt9211: Add drive-strength-microamp DT property
Author: Boerge Struempfel <bstruempfel@data-modul.com>
Patches: 7
Reviewed: 2026-05-16T13:11:28.036918
---
This is a clean, well-structured 2-patch series adding a configurable LVDS TX drive-strength property to the LT9211 bridge driver. The binding and driver changes are split correctly across the two patches. The default value (`0x8` = 25 uA) preserves backward compatibility. The code is correct and consistent with existing patterns in the driver.
One concern worth flagging: the values 12-36 in `drive-strength-microamp` seem implausibly low for LVDS output drive current (standard LVDS specifies ~3.5 mA nominal differential current). These may represent an internal bias/reference current rather than the actual output current, or there may be a units error (milliamps vs. microamps). This should be verified against the LT9211 datasheet.
Overall: **Looks good with minor comments.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 11+ messages in thread
* Claude review: Re: [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property
2026-05-12 17:05 ` Conor Dooley
@ 2026-05-16 3:11 ` Claude Code Review Bot
0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:11 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Looks good.** The binding addition is well-formed.
The enum lists 16 values matching the 16 hardware levels, and the default of 25 matches the existing hardcoded behavior. Placement within the `properties:` block (between `vccio-supply` and `ports`) is reasonable.
Minor points:
1. **Property name context**: `drive-strength-microamp` originates from the pinctrl generic bindings (`pincfg-node.yaml`). Using it directly on a bridge device node is somewhat unusual. DT binding maintainers may prefer a vendor-prefixed property like `lontium,drive-strength-microamp`, or may be fine with it since the semantics are a natural fit. Worth confirming with Rob Herring / Krzysztof.
2. **Units**: As noted above, 12-36 uA is extremely low for LVDS drive current. If these values actually represent milliamps, the property name and all values would need correction. The commit message says "12 uA to 36 uA" — please verify against the datasheet.
3. **Example DT**: The binding example does not show usage of the new property. While optional, adding `drive-strength-microamp = <25>;` to the existing example would serve as documentation.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 11+ messages in thread
* Claude review: Re: [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
2026-05-12 18:29 ` Marek Vasut
2026-05-13 9:02 ` Börge Strümpfel
@ 2026-05-16 3:11 ` Claude Code Review Bot
1 sibling, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:11 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Looks good.** The implementation is correct and backward compatible.
**Register value correctness**: The original hardcoded value was `0x48`:
```c
{ 0x823f, 0x48 },
```
The patch changes this to:
```c
{ 0x823f, 0x40 | ctx->lvds_hsdrv_isel },
```
Since `0x48 = 0x40 | 0x08` and the default `lvds_hsdrv_isel = 8`, this correctly preserves the original value when the DT property is absent. The upper nibble (`0x40`) is preserved while bits `[3:0]` carry the drive-strength select. This is correct.
**DT parsing**: The linear search through `lt9211_hsdrv_microamp[]` (16 entries) is fine, and the error path for invalid values is handled correctly with `dev_err` + `-EINVAL`.
Minor points:
1. **`of_property_read_u32` vs `device_property_read_u32`**: The patch uses `of_property_read_u32()`, which is consistent with the rest of this driver (it's heavily OF-dependent with `of_graph_*` calls). Modern kernel convention prefers the firmware-agnostic `device_property_read_u32(dev, ...)`, but consistency with the existing driver is a reasonable choice here.
2. **Lookup table could use `match_string`-style helper**: The kernel has `match_u32_property_value()` or similar patterns, but the explicit loop is perfectly clear for 16 elements. No change needed.
3. **The `const` array becomes runtime-initialized**: `system_lt9211_tx_phy_seq[]` was previously fully compile-time constant (eligible for `.rodata`). With `ctx->lvds_hsdrv_isel`, it's now stack-initialized at each call. This is fine — the same pattern already exists in the array with `ctx->lvds_dual_link` on the line just above:
```c
{ 0x823b, 0x38 | (ctx->lvds_dual_link ? BIT(7) : 0) },
```
So this is consistent with the existing code.
4. **Validation redundancy**: The DT schema enum already constrains values, but the driver validates too. This is good practice since not all DTs are schema-validated at runtime.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-16 3:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 16:46 [PATCH 0/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
2026-05-12 16:46 ` [PATCH 1/2] dt-bindings: display/bridge: lt9211: Add drive-strength-microamp property Boerge Struempfel
2026-05-12 17:05 ` Conor Dooley
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
2026-05-12 16:46 ` [PATCH 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property Boerge Struempfel
2026-05-12 18:29 ` Marek Vasut
2026-05-13 9:02 ` Börge Strümpfel
2026-05-13 11:33 ` Marek Vasut
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
2026-05-16 3:11 ` Claude review: " Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-05-13 13:31 [PATCH v2 0/2] " Boerge Struempfel
2026-05-16 1:56 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox