Re: [PATCH phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Uninitialized memory read in `ds125df111_xlate` due to missing `args_count` validation.
- [Low] Missing explicit header inclusions for macros and memory allocation functions.
--

commit a5dcf247e2e2a6b752d667ac46227925aa476def
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

This commit adds a generic PHY driver for the TI DS125DF111 Multi-Protocol
Dual-Channel Retimer over I2C. It currently supports 10G and 1G link speeds,
and registers a separate PHY device for each of the two channels to allow
independent configuration.

> diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> new file mode 100644
> index 0000000000000..8ac10c603bf70
> --- /dev/null
> +++ b/drivers/phy/ti/phy-ds125df111.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2026 NXP */
> +
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>

This isn't a bug, but should this include linux/bitfield.h, linux/bits.h,
and linux/slab.h?

The driver uses FIELD_PREP(), GENMASK(), and devm_kzalloc(), but relies on
implicit inclusions for these headers. Explicitly including them can help
prevent build breakages if the include chains are refactored in the future.

[ ... ]

> +static struct phy *ds125df111_xlate(struct device *dev,
> +				    const struct of_phandle_args *args)
> +{
> +	struct ds125df111_priv *priv = dev_get_drvdata(dev);
> +	u32 idx = args->args[0];

Is it possible to read uninitialized memory here?

If a consumer's Device Tree reference incorrectly configures #phy-cells = <0>,
of_parse_phandle_with_args() sets args->args_count to 0 and leaves the
args array unmodified.

Since the of_phandle_args structure is often allocated on the stack, for
example in _of_phy_get(), evaluating args->args[0] without verifying that
args->args_count >= 1 could result in evaluating stack garbage against the
maximum channel count.

> +
> +	if (idx >= DS125DF111_NUM_CH) {
> +		dev_err(dev, "Maximum number of channels is %d\n",
> +			DS125DF111_NUM_CH);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return priv->ch[idx].phy;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513185103.1371809-1-ioana.ciornei@nxp.com?part=2