Re: [PATCH v5 0/3] hwmon: Add support for the LTC4283 Hot Swap Controller

["Nuno Sá" <noname.nuno@gmail.com>]

Hi Guenter,

On Tue, 2026-01-27 at 09:39 -0800, Guenter Roeck wrote:
> Hi Nuno,
> 
> On 12/23/25 04:21, Nuno Sá via B4 Relay wrote:
> > This is v3 for the LTC4283 how swap controller. Main change is that I'm
> > now using the auxiliary bus for adding the GPIO device (done depending
> > on FW properties).
> > 
> > Similar to the LTC4282 device, we're clearing some fault logs in the
> > reset_history attributes.
> > 
> 
> I ran the patch series through an AI review. Results are below.
> Please take a look.
> 
> Thanks,
> Guenter
> 
> ---
> # Deep Dive Regression Analysis Log
> Date: 2026-01-27
> 
> ## Commit 5998c620fbd8: gpio: gpio-ltc4283: Add support for the LTC4283 Swap Controller
> 
> **Context**:
> - Adds a new GPIO driver `drivers/gpio/gpio-ltc4283.c`.
> - Depends on `drivers/hwmon/ltc4283.c` for regmap and MFD instantiation.
> 
> **Findings**:
> 1.  **Typo/Naming Issue**:
>      - File `drivers/gpio/gpio-ltc4283.c` uses `LTC42823_ADIO_CONFIG` definition.
>      - `LTC42823` appears to be a typo for `LTC4283`.
>      - `drivers/gpio/gpio-ltc4283.c:26`: `#define LTC42823_ADIO_CONFIG 0x12`
> 
> 2.  **Logic Verification**:
>      - GPIO direction and value setting logic appears consistent with bitmasks provided, assuming
> register `0x12` and `0x11` layout is correct as inferred.
> 
> 3.  **Regressions**:
>      - No critical functional regressions found in this commit alone.
>      - Code quality: Typo in macro name.
> 
> ## Commit 82433332ce16: hwmon: ltc4283: Add support for the LTC4283 Swap Controller
> 
> **Context**:
> - Adds core HWMON driver `drivers/hwmon/ltc4283.c`.
> - Handles I2C communication, regmap initialization, and MFD child creation.
> 
> **Findings**:
> 1.  **Critical Regression: DebugFS Use-After-Free**:
>      - `ltc4283_probe` calls `ltc4283_debugfs_init`.
>      - `ltc4283_debugfs_init` calls `debugfs_create_file_unsafe` creating files associated with
> `st`.
>      - `st` is allocated with `devm_kzalloc`.
>      - **Issue**: There is no explicit cleanup of debugfs entries (e.g.,
> `debugfs_remove_recursive`) and no `devm_add_action` to handle it.
>      - **Consequence**: When the driver is unbound/unloaded, `st` is freed by devm. The debugfs
> entries remain. Accessing them triggers a Use-After-Free on `st`.
>      - **Fix Required**: Use `devm_add_action_or_reset` to remove debugfs directory/entries on
> driver detach.

So actually a nice deduction from the agent but not accurate given that all the entries should be
removed in [1]. If you want it to be explicit, I can add the action but given that the dentry is
given by i2c, I would argue it's fair to assume i2c will take care of the removal. Thoughts?

I'm also planning to pick this up again but was caught by something else. So hopefully next week. 

On another topic, I also (you questioned it in another thread) looked around and I do have a ltc4282
board with me. Given that I just need to power up the digital parts to access the i2c bus, it should
be pretty straight to get a register dump. So if you're still interested on that, I should be able
to do it.

[1]: https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/i2c/i2c-core-base.c#L634

- Nuno Sá
>