Re: [RFC PATCH 2/2] platform: arm64: qcom-hamoa-ec: test sensor and variant handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unresolvable devres reference loop causes a permanent hang during KUnit test cleanup.
- [Medium] Hardware monitoring device registered from a platform driver outside of drivers/hwmon/.
--

commit 10084c21cb97238d4ea1fb5148e02186d2b75bb5
Author: Birk Skyum <birk.skyum@pm.me>

platform: arm64: qcom-hamoa-ec: test sensor and variant handling

This commit introduces KUnit tests for the Qualcomm EC driver's sensor
transactions and read-only callbacks. The tests simulate an I2C adapter to
verify RPM decoding, short transfers, and error propagation without
accessing a physical EC.

> diff --git a/drivers/platform/arm64/qcom-hamoa-ec-test.c b/drivers/platform/arm64/qcom-hamoa-ec-test.c
> new file mode 100644
> index 0000000000000..68868ef7625ae
> --- /dev/null
> +++ b/drivers/platform/arm64/qcom-hamoa-ec-test.c
[ ... ]
> @@ -0,0 +1,294 @@
[ ... ]
> +static void qcom_ec_probe_sensor_test(struct kunit *test)
> +{
> +	struct qcom_ec_test_context *ctx = test->priv;
> +
> +	KUNIT_EXPECT_EQ(test, qcom_ec_hwmon_probe(&ctx->ec), 0);

[Severity: High]
Does manually invoking qcom_ec_hwmon_probe() on a dummy I2C client cause a
permanent kernel hang during test cleanup?

The KUnit test creates a dummy I2C client using i2c_new_dummy_device(),
which lacks a bound driver. When qcom_ec_hwmon_probe() is manually invoked,
it allocates a child hwmon device via devm_hwmon_device_register_with_info():

drivers/platform/arm64/qcom-hamoa-ec.c:qcom_ec_hwmon_probe() {
    ...
    hwmon = devm_hwmon_device_register_with_info(dev, "qcom_ec", ec,
                                                 &qcom_ec_hwmon_chip_info,
                                                 NULL);
    ...
}

Because the dummy client has no bound driver, devres cleanup is deferred
until its kobject refcount reaches zero. However, the registered child hwmon
device takes a kobject reference on the dummy client. This creates an
unresolvable cycle where the parent refcount cannot reach zero until the
child hwmon device is unregistered, which is supposed to happen in the
devres cleanup.

This prevents the dummy client from being freed, causing the subsequent
i2c_del_adapter() in test cleanup to block forever in
wait_for_completion(&adap->dev_released).

[Severity: Medium]
Does this code register a hardware monitoring device from a driver located
outside of the drivers/hwmon/ directory?

The hwmon subsystem architecture requires that hardware monitoring devices
be registered from drivers residing in drivers/hwmon/. If the main
functionality of the chip is not hardware monitoring, the hardware
monitoring functionality should be implemented as an auxiliary device driver
residing in drivers/hwmon/.

Here, devm_hwmon_device_register_with_info() is called directly by a
platform driver located in drivers/platform/arm64/:

drivers/platform/arm64/qcom-hamoa-ec.c:qcom_ec_hwmon_probe() {
    ...
    hwmon = devm_hwmon_device_register_with_info(dev, "qcom_ec", ec,
                                                 &qcom_ec_hwmon_chip_info,
                                                 NULL);
    ...
}

> +	KUNIT_EXPECT_EQ(test, ctx->transactions, 1);
> +	KUNIT_EXPECT_EQ(test, ctx->writes, 0);
> +	ctx->reply[0] = 0;
> +	KUNIT_EXPECT_EQ(test, qcom_ec_hwmon_probe(&ctx->ec), -EPROTO);
> +	KUNIT_EXPECT_EQ(test, ctx->writes, 0);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908015650.84401-1-birk.skyum@pm.me?part=2