Re: [PATCH v4 4/9] usb: misc: qcom_eud: add per-path role switch support

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

On 7/5/2026 8:32 PM, Peter Chen wrote:
> On 26-05-01 10:06:30, Elson Serrao wrote:
>> The EUD hardware can support multiple High-Speed USB paths, each connected
>> to different USB controllers. The current implementation uses a single

[...]

>> +static void eud_path_role_switch_release(void *data)
>> +{
>> +	struct eud_path *path = data;
>> +
>> +	usb_role_switch_unregister(path->eud_sw);
>> +	usb_role_switch_put(path->controller_sw);
>>  }
>>  
>>  static int eud_init_path(struct eud_chip *chip, struct device_node *np)
>>  {
>> +	struct usb_role_switch_desc role_sw_desc;
> 
> Should role_sw_desc be zero-initialized here? Otherwise, some entries
> of path->eud_sw are uncertain after calling usb_role_switch_register?
> 

Thank you for your review, Peter.

I'll zero-initialize role_sw_desc in the next revision to avoid any
uninitialized fields.

Elson

>> +	struct usb_role_switch *sw;
>>  	struct device_node *controller_node;
>>  	struct eud_path *path;
>>  	u32 path_num;
>> @@ -331,11 +358,44 @@ static int eud_init_path(struct eud_chip *chip, struct device_node *np)
>>  				     "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))
>> +	if (IS_ERR(path->phy)) {
>> +		of_node_put(controller_node);
>>  		return dev_err_probe(chip->dev, PTR_ERR(path->phy),
>>  				     "failed to get PHY for path %d\n", path_num);
>> +	}
>> +
>> +	path->curr_role = USB_ROLE_NONE;
>> +
>> +	/* Fetch controller role switch if it is role switch capable */
>> +	if (of_property_read_bool(controller_node, "usb-role-switch")) {
>> +		sw = usb_role_switch_find_by_fwnode(of_fwnode_handle(controller_node));
>> +		if (!sw) {
>> +			of_node_put(controller_node);
>> +			return dev_err_probe(chip->dev, -EPROBE_DEFER,
>> +					     "Failed to get controller role switch for path %d\n",
>> +					     path_num);
>> +		}
>> +		path->controller_sw = sw;
>> +	}
>> +
>> +	of_node_put(controller_node);
>> +
>> +	role_sw_desc.fwnode = of_fwnode_handle(np);
>> +	role_sw_desc.set = eud_role_switch_set;
>> +	role_sw_desc.driver_data = path;
>> +	snprintf(path->name, sizeof(path->name), "eud-path%u", path_num);
>> +	role_sw_desc.name = path->name;
>> +
>> +	path->eud_sw = usb_role_switch_register(chip->dev, &role_sw_desc);
>> +	if (IS_ERR(path->eud_sw)) {
>> +		usb_role_switch_put(path->controller_sw);
>> +		return dev_err_probe(chip->dev, PTR_ERR(path->eud_sw),
>> +				     "Failed to register EUD role switch for path %d\n", path_num);
>> +	}
>> +
>> +	ret = devm_add_action_or_reset(chip->dev, eud_path_role_switch_release, path);
>> +	if (ret)
>> +		return ret;
>>  
>>  	chip->paths[path_num] = path;
>>  
>> @@ -365,15 +425,6 @@ static int eud_probe(struct platform_device *pdev)
>>  	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),
>> -					"failed to get role switch\n");
>> -
>> -	ret = devm_add_action_or_reset(chip->dev, eud_role_switch_release, chip);
>> -	if (ret)
>> -		return ret;
>> -
>>  	chip->base = devm_platform_ioremap_resource(pdev, 0);
>>  	if (IS_ERR(chip->base))
>>  		return PTR_ERR(chip->base);
>> -- 
>> 2.34.1
>>
>>
>