From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: mfd: lm3533: Convert to use OF bindings Date: Thu, 04 Jun 2026 13:59:54 +1000 Message-ID: In-Reply-To: <20260601151831.76350-6-clamor95@gmail.com> References: <20260601151831.76350-1-clamor95@gmail.com> <20260601151831.76350-6-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 This is the largest patch. The overall conversion from pdata to DT is well = done =E2=80=94 dynamic `mfd_cell` allocation from child nodes, parsing DT p= roperties, removing all pdata structs. **Bug: DT property name mismatch for ALS resistor.** The binding schema (patch 1) defines the property as `ti,resistor-ohms`, bu= t the driver reads: ```c ret =3D device_property_read_u32(dev, "ti,resistor-value-ohms", &als->r_select); ``` This should be `"ti,resistor-ohms"` to match the binding. **Typo in error message:** ```c return dev_err_probe(dev, ret, "failed to ger resistor value\n"); ``` "ger" should be "get". **Minor observations:** - The resistor ohms-to-register conversion `DIV_ROUND_UP(2 * MICRO, 10 * al= s->r_select)` is correct: for 1575 ohms =E2=86=92 register 127, for 200000 = ohms =E2=86=92 register 1. - `led->cdev.default_trigger =3D "none"` is unconventional; the standard ap= proach is to leave it NULL when no trigger is specified. The subsequent `de= vice_property_read_string` will overwrite it only if the DT property exists. - Moving `LM3533_MAX_CURRENT_*` defines from `lm3533-ctrlbank.c` to the sha= red header is necessary since both LED and backlight drivers now clamp curr= ent values before calling `lm3533_ctrlbank_set_max_current`. - The `boost_ovp` and `boost_freq` values are not initialized before the `c= lamp()` call. If the DT property is missing, `device_property_read_u32` lea= ves the variable uninitialized. Since `boost_ovp` and `boost_freq` are stru= ct members (zero-initialized via devm_kzalloc), the default value 0 will be= clamped to the minimum. This works but could be more explicit. --- Generated by Claude Code Patch Reviewer