devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Colin Huang" <colin.huang2@amd.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org,
	linux-hwmon@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v3 2/2] hwmon: (pmbus/tda38740) Add driver for Infineon TDA38740/TDA38725
Date: Thu, 10 Sep 2026 06:31:04 +0000	[thread overview]
Message-ID: <20260910063105.0ADEB1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260910-add-tda38740-and-tda38725-v3-2-3e87637da3d6@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver intercepts PMBUS_READ_VOUT to apply `vout_scale_micro` but fails to scale VOUT limits during reads and completely omits a write hook, leading to dangerous unscaled hardware writes.
--

commit 1abfb85e1a82a16c8ba7229d3c9657b7d232835d
Author: Colin Huang <u8813345@gmail.com>

hwmon: (pmbus/tda38740) Add driver for Infineon TDA38740/TDA38725
    
Add a PMBus driver for Infineon TDA38740 and TDA38725
single-voltage synchronous buck regulators.

> diff --git a/drivers/hwmon/pmbus/tda38740.c b/drivers/hwmon/pmbus/tda38740.c
> new file mode 100644
> index 0000000000000..df5173d8da0c1
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/tda38740.c
[ ... ]
> +static int tda38740_read_word_data(struct i2c_client *client, int page,
> +				   int phase, int reg)
> +{
> +	const struct tda38740_data *data;
> +	int ret;
> +	u64 scaled;
> +
> +	if (reg != PMBUS_READ_VOUT)
> +		return -ENODATA;

[Severity: High]
Does this bypass software scaling for all VOUT-related limit commands?

By returning -ENODATA for anything other than PMBUS_READ_VOUT, the PMBus
core falls back to unscaled hardware reads for VOUT limits (e.g., in1_crit
or in1_input limits).

Should this function also scale the limit registers when they are read?

> +
> +	ret = pmbus_read_word_data(client, page, phase, reg);
> +	if (ret < 0)
> +		return ret;
[ ... ]
> +static int tda38740_probe(struct i2c_client *client)
> +{
[ ... ]
> +	memcpy(&data->info, &tda38740_info, sizeof(tda38740_info));
> +	data->vout_scale_micro = vout_scale_micro;
> +	data->info.read_word_data = tda38740_read_word_data;
> +
> +	return pmbus_do_probe(client, &data->info);

[Severity: High]
Is there a missing write_word_data hook here?

Since the driver intercepts reads to apply vout_scale_micro, writes to limit
registers currently fall back to unscaled hardware writes.

If a user writes a 1.5V limit for a board with a 2x voltage divider, the PMBus
core writes 1.5V directly to hardware without scaling. The hardware then
regulates or triggers a fault only when the pin reaches 1.5V, which corresponds
to 3.0V on the board, potentially causing physical damage.

Could we add a write_word_data hook to ensure VOUT limit writes are scaled
correctly?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-add-tda38740-and-tda38725-v3-0-3e87637da3d6@gmail.com?part=2

  reply	other threads:[~2026-09-10  6:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10  6:18 [PATCH v3 0/2] Add driver for Infineon TDA38740/TDA38725 Colin Huang
2026-09-10  6:18 ` [PATCH v3 1/2] dt-bindings: hwmon: pmbus: Add Infineon tda38740 and tda38725 Colin Huang
2026-09-10  6:25   ` sashiko-bot
2026-09-10 11:24   ` Conor Dooley
2026-09-14  9:12     ` Colin Huang
2026-09-10  6:18 ` [PATCH v3 2/2] hwmon: (pmbus/tda38740) Add driver for Infineon TDA38740/TDA38725 Colin Huang
2026-09-10  6:31   ` sashiko-bot [this message]
2026-09-10  8:55   ` Guenter Roeck
2026-09-10 12:35     ` Colin Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260910063105.0ADEB1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=colin.huang2@amd.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).