Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org>
Cc: olteanv@gmail.com, linux-clk@vger.kernel.org, robh@kernel.org,
	conor+dt@kernel.org, devicetree@vger.kernel.org,
	linux-phy@lists.infradead.org, neil.armstrong@linaro.org,
	vkoul@kernel.org, media-ci@linuxtv.org
Subject: Re: [PATCH v19 2/5] phy: core: Add phy_get_by_of_node()
Date: Thu, 17 Sep 2026 15:36:29 +0000	[thread overview]
Message-ID: <20260917153630.02E591F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260917-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v19-2-21ad8a76e941@linaro.org>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] phy: core: missing #phy-cells validation in phy_get_by_of_node() causes silent misbehavior
- [Medium] phy: core: missing usb-nop-xceiv compatibility check in phy_get_by_of_node()
--

commit da2f62e45c36c5bbb5ae2340d33b32c34093c095
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

phy: core: Add phy_get_by_of_node()

Add new function phy_get_by_of_node() allowing lookup of a phy by
device_node. Separates existing logic in _of_phy_get() into an internal
helper method _of_phy_get_with_args() to allow for reuse in new method.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 89addd732bff3..4a8e1d3cdfe75 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c

[ ... ]

> @@ -1001,6 +1017,41 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
>  
> +/**
> + * phy_get_by_of_node() - lookup and obtain a reference to a phy by device_node
> + * @np: node containing the phy
> + *
> + * Returns the phy associated with the device node or ERR_PTR.
> + */
> +struct phy *phy_get_by_of_node(struct device_node *np)
> +{
> +	struct of_phandle_args args = { .np = np, .args_count = 0 };

[Severity: High]
Does this hardcoded args_count = 0 bypass the #phy-cells validation that
normally happens in of_parse_phandle_with_args()?

If called on a PHY provider node with #phy-cells > 0 (e.g. resolved via an OF
graph endpoint), the provider's of_xlate callback will receive args_count = 0.

Since many callbacks don't check args_count and directly access args->args[0]
(which is initialized to 0), could they silently return PHY 0 instead of
returning an error, causing the caller to operate on the wrong PHY instance?

> +	struct phy *phy;
> +
> +	if (!np)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&phy_provider_mutex);
> +
> +	phy = _of_phy_get_with_args(&args);

[Severity: Medium]
Is this missing a compatibility check for "usb-nop-xceiv"?

In _of_phy_get(), legacy USB PHYs are explicitly checked and return -ENODEV
so they can gracefully fall back to the older usb-phy subsystem.

Because phy_get_by_of_node() directly calls _of_phy_get_with_args() without
this check, could a driver using this API on a legacy USB PHY node fail to
find a generic PHY provider and receive -EPROBE_DEFER, causing an infinite
probe deferral loop?

> +
> +	if (IS_ERR(phy))
> +		goto out_unlock;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v19-0-21ad8a76e941@linaro.org?part=2

  reply	other threads:[~2026-09-17 15:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 15:25 [PATCH v19 0/5] Add phy_get_by_of_node and devm helper Bryan O'Donoghue
2026-09-17 15:25 ` [PATCH v19 1/5] phy: core: Fix race-condition between _of_phy_get() and try_module_get() Bryan O'Donoghue
2026-09-17 15:25 ` [PATCH v19 2/5] phy: core: Add phy_get_by_of_node() Bryan O'Donoghue
2026-09-17 15:36   ` sashiko-bot [this message]
2026-09-17 15:25 ` [PATCH v19 3/5] phy: core: Add devm_phy_get_by_of_node() Bryan O'Donoghue
2026-09-17 15:25 ` [PATCH v19 4/5] media: qcom: camss: Add support for PHY API devices Bryan O'Donoghue
2026-09-17 15:37   ` sashiko-bot
2026-09-17 15:25 ` [PATCH v19 5/5] media: qcom: camss: Use data-lanes starting at 1 for new CSIPHY mode Bryan O'Donoghue
2026-09-17 18:12 ` [PATCH v19 0/5] Add phy_get_by_of_node and devm helper Oleg Keri
2026-09-18  1:36 ` Manivannan Sadhasivam
2026-09-18  8:37   ` Bryan O'Donoghue

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=20260917153630.02E591F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=media-ci@linuxtv.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /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