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

[Elson Serrao <elson.serrao@oss.qualcomm.com>]

On 7/5/2026 11:26 PM, Peter Chen wrote:
> On 26-05-01 10:06:29, 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 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.
>>
>> 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 for proper operation.

[...]

>> +};
>> +
>>  struct eud_chip {
>>  	struct device			*dev;
>>  	struct usb_role_switch		*role_sw;
>>  	void __iomem			*base;
>> +	struct eud_path			*paths[EUD_MAX_PORTS];
>>  	phys_addr_t			mode_mgr;
>>  	unsigned int			int_status;
>>  	int				irq;
>>  	bool				enabled;
>>  	bool				usb_attached;
>> +	bool				phy_enabled;
> 
> There is already a 'enabled' flag, could be leveraged or still need
> a new 'phy_enabled' flag?
> 
enabled reflects the user's sysfs intent, while phy_enabled tracks the
actual PHY power state. These need to stay independent because a later
patch in this series ("add host mode coordination") disables/re-enables
the PHY on host-mode role transitions without touching chip->enabled, so
as to preserve the user's original sysfs setting across the transition.

Thanks,
Elson

u8				port_idx;
>>  };
>>  
>> +static int eud_phy_enable(struct eud_chip *chip)
>> +{
>> +	struct phy *phy;
>> +	int ret;
>> +
>> +	if (chip->phy_enabled)
>> +		return 0;
>> +
>> +	phy = chip->paths[chip->port_idx]->phy;
>> +
>> +	ret = phy_init(phy);
>> +	if (ret) {
>> +		dev_err(chip->dev, "Failed to initialize USB2 PHY for port %u: %d\n",
>> +			chip->port_idx, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = phy_power_on(phy);
>> +	if (ret) {
>> +		dev_err(chip->dev, "Failed to power on USB2 PHY for port %u: %d\n",
>> +			chip->port_idx, ret);
>> +		phy_exit(phy);
>> +		return ret;
>> +	}
>> +
>> +	chip->phy_enabled = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static void eud_phy_disable(struct eud_chip *chip)
>> +{
>> +	struct phy *phy;
>> +
>> +	if (!chip->phy_enabled)
>> +		return;
>> +
>> +	phy = chip->paths[chip->port_idx]->phy;
>> +
>> +	phy_power_off(phy);
>> +	phy_exit(phy);
>> +	chip->phy_enabled = false;
>> +}
>> +
>>  static int enable_eud(struct eud_chip *priv)
>>  {
>>  	int ret;
>>  
>> -	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
>> +	ret = eud_phy_enable(priv);
>>  	if (ret)
>>  		return ret;
>>  
>> +	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
>> +	if (ret) {
>> +		eud_phy_disable(priv);
>> +		return ret;
>> +	}
>> +
>>  	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
>>  	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>>  			priv->base + EUD_REG_INT1_EN_MASK);
>> @@ -75,6 +135,8 @@ static int disable_eud(struct eud_chip *priv)
>>  		return ret;
>>  
>>  	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
>> +	eud_phy_disable(priv);
>> +
>>  	return 0;
>>  }
>>  
>> @@ -130,6 +192,12 @@ static ssize_t port_store(struct device *dev, struct device_attribute *attr,
>>  	if (port < 0)
>>  		return port;
>>  
>> +	/* Check if the corresponding path is available */
>> +	if (!chip->paths[port]) {
>> +		dev_err(chip->dev, "EUD not supported on selected port\n");
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>>  	/* Port selection must be done before enabling EUD */
>>  	if (chip->enabled) {
>>  		dev_err(chip->dev, "Cannot change port while EUD is enabled\n");
>> @@ -229,8 +297,54 @@ static void eud_role_switch_release(void *data)
>>  	usb_role_switch_put(chip->role_sw);
>>  }
>>  
>> +static int eud_init_path(struct eud_chip *chip, struct device_node *np)
>> +{
>> +	struct device_node *controller_node;
>> +	struct eud_path *path;
>> +	u32 path_num;
>> +	int ret;
>> +
>> +	ret = of_property_read_u32(np, "reg", &path_num);
>> +	if (ret) {
>> +		/* Legacy DT uses 'ports' node without 'reg' property; treat as path 0 */
>> +		if (of_node_name_eq(np, "ports"))
>> +			path_num = 0;
>> +		else
>> +			return dev_err_probe(chip->dev, ret, "unexpected child node '%s'\n",
>> +					     np->name);
>> +	}
>> +
>> +	if (path_num >= EUD_MAX_PORTS)
>> +		return dev_err_probe(chip->dev, -EINVAL, "invalid path number: %u (max %d)\n",
>> +				     path_num, EUD_MAX_PORTS - 1);
>> +
>> +	path = devm_kzalloc(chip->dev, sizeof(*path), GFP_KERNEL);
>> +	if (!path)
>> +		return -ENOMEM;
>> +
>> +	path->chip = chip;
>> +	path->num = path_num;
>> +
>> +	controller_node = of_graph_get_remote_node(np, 0, -1);
>> +	if (!controller_node)
>> +		return dev_err_probe(chip->dev, -ENODEV,
>> +				     "failed to get controller node for path %u\n", path_num);
>> +
>> +	path->phy = devm_of_phy_get_by_index(chip->dev, controller_node, 0);
>> +	of_node_put(controller_node);
>> +
>> +	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 eud_chip *chip;
>>  	struct resource *res;
>>  	int ret;
>> @@ -241,6 +355,16 @@ static int eud_probe(struct platform_device *pdev)
>>  
>>  	chip->dev = &pdev->dev;
>>  
>> +	for_each_child_of_node_scoped(np, child) {
>> +		ret = eud_init_path(chip, child);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	/* Primary path is mandatory. Secondary is optional */
>> +	if (!chip->paths[0])
>> +		return dev_err_probe(chip->dev, -ENODEV, "primary path not found\n");
>> +
>>  	chip->role_sw = usb_role_switch_get(&pdev->dev);
>>  	if (IS_ERR(chip->role_sw))
>>  		return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
>> -- 
>> 2.34.1
>>
>>
>