From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: video: leds: backlight: lm3533: Support getting LED sources from DT Date: Thu, 04 Jun 2026 13:59:56 +1000 Message-ID: In-Reply-To: <20260601151831.76350-12-clamor95@gmail.com> References: <20260601151831.76350-1-clamor95@gmail.com> <20260601151831.76350-12-clamor95@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Bug: Wide mask clobbers other banks' LED source configurations.** In `lm3533_bl_setup`: ```c ret =3D regmap_update_bits(bl->lm3533->regmap, LM3533_REG_OUTPUT_CONF1, OUTPUT_CONF1_MASK, output_cfg_val); ``` `OUTPUT_CONF1_MASK =3D GENMASK(1, 0) =3D 0x03` covers BOTH HVLED1 and HVLED= 2 fields. If backlight bank B only configures HVLED1 (led-sources =3D <0>),= the mask still writes bit 1 as 0, resetting HVLED2's assignment to bank A = =E2=80=94 even if bank A had previously configured HVLED2 differently. The = same issue exists on the LED side: ```c ret =3D regmap_update_bits(led->lm3533->regmap, LM3533_REG_OUTPUT_CONF1, OUTPUT_CONF1_MASK, output_cfg_val << OUTPUT_CONF1_SHIFT); ``` `OUTPUT_CONF1_MASK =3D GENMASK(7, 2)` covers all three LVLED1/2/3 fields. C= onfiguring one LVLED clobbers the other two. The original `lm3533_set_hvled= _config` and `lm3533_set_lvled_config` correctly computed per-LED masks to = avoid this. The fix is to build the mask dynamically alongside the value: ```c for (i =3D 0; i < bl->num_leds; i++) { if (bl->led_strings[i] >=3D LM3533_HVCTRLBANK_COUNT) continue; output_cfg_mask |=3D LM3533_BL_ID_MASK << bl->led_strings[i]; output_cfg_val |=3D id << bl->led_strings[i]; } ret =3D regmap_update_bits(..., output_cfg_mask, output_cfg_val); ``` **Possible undefined symbol:** `LM3533_LVCTRLBANK_MAX` is used in `leds-lm3= 533.c` for the `leds[]` array size and bounds check, but I don't see its de= finition in any patch diff. Verify this compiles =E2=80=94 it may need to b= e added to the header or as a local define. --- Generated by Claude Code Patch Reviewer