Re: [PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-17 21:21:40
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Bypassing the regulator framework on ACPI platforms breaks PRP0001-based configurations. - [Low] Missing inclusion of `<linux/acpi.h>` for `ACPI_COMPANION`. --
--- Patch [5]: [PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths ---commit 6ac93c8870295bc478f5b446654ca1771bdebc78 Author: Taha Ed-Dafili [off-list ref] iio: dac: ad5504: strictly separate ACPI and DT probe paths Refactor the ad5504_probe() function to explicitly separate the ACPI and Device Tree execution paths.
quoted hunk ↗ jump to hunk
diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c index 7b52ec95150de..e124e7804f3fb 100644 --- a/drivers/iio/dac/ad5504.c +++ b/drivers/iio/dac/ad5504.c@@ -19,6 +19,7 @@ #include <linux/spi/spi.h> #include <linux/sysfs.h> #include <linux/types.h> +#include <linux/units.h>
[Severity: Low] This isn't a bug, but should <linux/acpi.h> be explicitly included here to satisfy the 'Include What You Use' (IWYU) principle for the ACPI_COMPANION macro used below? Building under configurations where it is not transitively included could lead to a build failure.
#include <linux/iio/events.h> #include <linux/iio/iio.h>
[ ... ]
quoted hunk ↗ jump to hunk
@@ -300,11 +305,16 @@ static int ad5504_probe(struct spi_device *spi) if (ret) return ret; - ret = devm_regulator_get_enable_read_voltage(dev, "vcc"); - if (ret < 0) - return ret; + if (ACPI_COMPANION(dev)) {
[Severity: Medium] Does bypassing the regulator framework on ACPI platforms break PRP0001-based configurations? On ACPI systems that use standard PRP0001 (Device Tree bindings wrapped in ACPI) to provide a vcc regulator, this unconditional branch skips enabling the regulator and reading its voltage. The device might remain unpowered or use an incorrect hardcoded reference voltage. Before this patch, ad5504_probe() attempted to fetch the regulator and fell back to defaults only if it returned -ENODEV. Should the regulator fetch be preserved for ACPI platforms to avoid defeating the unified device property frameworks?
quoted hunk ↗ jump to hunk
+ st->vref_mv = AD5504_VA_MV_ACPI_DEFAULT; + } else { + ret = devm_regulator_get_enable_read_voltage(dev, "vcc"); + if (ret < 0) + return dev_err_probe(dev, ret, + "Failed to get vcc regulator\n"); - st->vref_mv = ret / 1000; + st->vref_mv = ret / MILLI; + }
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260817211118.21833-1-0rayn.dev@gmail.com?part=5