Re: [PATCH v9 3/3] usb: gadget: tegra-xudc: avoid direct dereference of phy->dev.of_node

[email protected]
Newsgroups dev.linux.lists.imx
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing error code assignment when `of_property_match_string()` fails, causing a silent initialization failure.
- [Low] Malformed error message format string splits log across two lines.
--

--- Patch [3]: [PATCH v9 3/3] usb: gadget: tegra-xudc: avoid direct dereference of phy->dev.of_node ---
commit 5dbf80ca5d539781ed1dcc83abb9ad4e29628462
Author: Vladimir Oltean <[email protected]>

usb: gadget: tegra-xudc: avoid direct dereference of phy->dev.of_node

This patch reworks the PHY lookup in the Tegra XUDC driver to avoid directly
dereferencing phy->dev.of_node. It introduces a parallel OF node lookup path
based on the "phys" phandle and the #phy-cells of the padctl provider.

> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
[ ... ]
> @@ -3525,22 +3529,39 @@ static int tegra_xudc_phy_get(struct tegra_xudc *xudc)
[ ... ]
>  		} else if (!xudc->utmi_phy[i]) {
>  			/* if utmi phy is not available, ignore USB3 phy get */
>  			continue;
>  		}
>  
> +		index = of_property_match_string(np, "phy-names", phy_name);
> +		if (index < 0) {
> +			dev_err(xudc->dev,
> +				"no \"phy-names\" entry for \"%s\": %pe\n",
> +				phy_name, ERR_PTR(index));
> +			goto clean_up;
> +		}

[Severity: High]
If this fails, we jump to clean_up without updating the err variable. Since err
was initialized to 0, would this cause the driver probe to falsely report
success without having acquired the PHYs?

Could we assign err = index before the goto?

> +
> +		err = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> +						 index, &args);
> +		if (err) {
> +			dev_err(xudc->dev,
> +				"failed to follow \"%s\" reference\n: %pe\n",
> +				phy_name, ERR_PTR(err));

[Severity: Low]
This isn't a bug, but there is an extraneous newline before the colon in the
format string which will split the log message across two lines.

> +			goto clean_up;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.