Re: [PATCH v2 3/9] usb: misc: qcom_eud: add per-path High-Speed PHY control

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 1/27/26 12:38 AM, Elson Serrao wrote:
> EUD hardware can support multiple High-Speed USB paths, each routed
> through its own PHY. The active path is selected in hardware via the
> EUD_PORT_SEL register. As a High-Speed hub, EUD requires access to the
> High-Speed PHY associated with the active UTMI path. To support this
> multi-path capability, the driver must manage PHY resources on a per-path
> basis, ensuring that the PHY for the currently selected path is properly
> initialized and powered.
> 
> This patch restructures the driver to implement per-path PHY management.
> The driver now powers the appropriate PHY based on the selected and
> enabled UTMI path, ensuring correct operation when EUD is enabled.
> 
> Supporting this requires describing the available UTMI paths and their
> corresponding PHYs in Device Tree. This updates DT requirements and is
> not backward compatible with older DTs that lacked this description.
> Historically, EUD appeared to work on single-path systems because the
> USB controller kept the PHY initialized. However, EUD is designed to
> operate independently of the USB controller and therefore requires
> explicit PHY control.
> 
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> ---

[...]

> +static int eud_phy_enable(struct eud_chip *chip)
> +{
> +	struct eud_path *path;
> +	struct phy *phy;
> +	int ret;
> +
> +	if (chip->phy_enabled)
> +		return 0;
> +
> +	path = chip->paths[chip->port_idx];
> +	if (!path || !path->phy) {

I think neither are possible - path is != NULL since we can't enter into
this function without failing the check in _store and !path->phy would error
out in probe()->eud_init_path()

[...]

> +static void eud_phy_disable(struct eud_chip *chip)
> +{
> +	struct eud_path *path;
> +	struct phy *phy;
> +
> +	if (!chip->phy_enabled)
> +		return;
> +
> +	path = chip->paths[chip->port_idx];
> +	if (!path || !path->phy)

Likewise

[...]

> +static int eud_init_path(struct eud_chip *chip, struct device_node *np)
> +{
> +	struct eud_path *path;
> +	u32 path_num;
> +	int ret;
> +
> +	ret = of_property_read_u32(np, "reg", &path_num);
> +	if (ret) {
> +		dev_err(chip->dev, "Missing 'reg' property in path node\n");
> +		return ret;

You can use return dev_err_probe like you did a little below

> +	}
> +
> +	if (path_num >= EUD_MAX_PORTS) {
> +		dev_err(chip->dev, "Invalid path number: %u (max %d)\n",
> +			path_num, EUD_MAX_PORTS - 1);
> +		return -EINVAL;
> +	}
> +
> +	path = devm_kzalloc(chip->dev, sizeof(*path), GFP_KERNEL);
> +	if (!path)
> +		return -ENOMEM;
> +
> +	path->chip = chip;
> +	path->num = path_num;
> +
> +	path->phy = devm_of_phy_get(chip->dev, np, NULL);
> +	if (IS_ERR(path->phy))
> +		return dev_err_probe(chip->dev, PTR_ERR(path->phy),
> +				     "Failed to get PHY for path %d\n", path_num);
> +
> +	chip->paths[path_num] = path;
> +
> +	return 0;
> +}
> +
>  static int eud_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child;
>  	struct eud_chip *chip;
>  	struct resource *res;
>  	int ret;
> @@ -252,6 +368,18 @@ static int eud_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	for_each_child_of_node(np, child) {

With for_each_child_of_node_scoped(), you can dispose of the manual
_put()

> +		ret = eud_init_path(chip, child);
> +		if (ret) {
> +			of_node_put(child);
> +			return ret;
> +		}
> +	}
> +
> +	/* Primary path is mandatory. Secondary is optional */
> +	if (!chip->paths[0])
> +		return -ENODEV;

I'm going to assume we don't have any funny chips that violate this :)

Konrad