From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: mfd: lm3533: Convert to use OF bindings
Date: Mon, 18 May 2026 16:19:46 +1000
Message-ID: <review-patch2-20260517074306.30937-3-clamor95@gmail.com>
In-Reply-To: <20260517074306.30937-3-clamor95@gmail.com>
References: <20260517074306.30937-1-clamor95@gmail.com> <20260517074306.30937-3-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 and has several bugs:

**Critical =E2=80=94 MFD cell ID mismatch**: The LED MFD cells are defined =
with `cell->id` =3D 0,1,2,3:
```c
MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 0, "ti,lm3533-leds", 2),
MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 1, "ti,lm3533-leds", 3),
MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 2, "ti,lm3533-leds", 4),
MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 3, "ti,lm3533-leds", 5),
```
Since `devm_mfd_add_devices` is called with `id=3D0`, `pdev->id =3D 0 + cel=
l->id`, giving 0,1,2,3. But in `lm3533_led_probe`, the check is:
```c
if (pdev->id < LM3533_LVCTRLBANK_MIN || pdev->id > LM3533_LVCTRLBANK_MAX)
```
where `LM3533_LVCTRLBANK_MIN=3D2`. So LED cells with `pdev->id` 0 and 1 wil=
l **always fail probe with `-EINVAL`**. And `led->id =3D pdev->id - LM3533_=
LVCTRLBANK_MIN` will produce wrong/negative values for cells 0 and 1.

Fix: The cell IDs should be 2,3,4,5 to match the DT reg values.

**DT property name mismatch**: The binding defines `ti,resistor-ohm`, but t=
he driver reads a different name:
```c
device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm",
                         &als->r_select);
```
This will silently fail to read the property on all boards that use the doc=
umented binding name.

**Dead code**: The `have_als`, `have_backlights`, `have_leds` flags are com=
puted by scanning child nodes but never read anywhere in the new code =E2=
=80=94 they're dead stores. Since `devm_mfd_add_devices` always registers a=
ll 7 cells regardless of these flags, the iteration and flag-setting should=
 either be removed or used to conditionally register cells.

**Removed `iio_device_set_parent`**: The line `iio_device_set_parent(indio_=
dev, pdev->dev.parent)` is deleted without comment. This changes the IIO de=
vice's parent from the i2c device to the platform device. If this is intent=
ional (for proper OF node association), it should be mentioned in the commi=
t message.

**Minor =E2=80=94 `linux,default-trigger` property**: The LEDs driver reads=
 `linux,default-trigger` via `device_property_read_string`, but the DT bind=
ing in Patch 1 doesn't document this property (though it inherits from `com=
mon.yaml` which may cover it).

---
Generated by Claude Code Patch Reviewer