[U-Boot] [RESEND PATCH 1/5] usb: host: dwc2: add phy support

[Marek Vasut]

On 10/14/19 10:00 AM, Patrick Delaunay wrote:
> Use generic phy to initialize the PHY associated to the

PHY and USB are abbreviations, should be in capitals.

> DWC2 device and available in the device tree.

[...]

General question -- is the PHY subsystem a mandatory dependency of this
driver now or will it work without the PHY subsystem still ?

> +static int dwc2_setup_phy(struct udevice *dev)
> +{
> +	struct dwc2_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = generic_phy_get_by_index(dev, 0, &priv->phy);
> +	if (ret) {
> +		if (ret != -ENOENT) {
> +			dev_err(dev, "failed to get usb phy\n");

Sentence starts with capital letter, USB and PHY are in capitals. Fix
globally please.

It would be useful to print the $ret value too.

> +			return ret;
> +		}
> +		return 0;
> +	}
> +
> +	ret = generic_phy_init(&priv->phy);
> +	if (ret) {
> +		dev_err(dev, "failed to init usb phy\n");
> +		return ret;
> +	}
> +
> +	ret = generic_phy_power_on(&priv->phy);
> +	if (ret) {
> +		dev_err(dev, "failed to power on usb phy\n");
> +		return generic_phy_exit(&priv->phy);
> +	}
> +
> +	return 0;
> +}
> +
> +static int dwc2_shutdown_phy(struct udevice *dev)
> +{
> +	struct dwc2_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	if (!generic_phy_valid(&priv->phy))
> +		return 0;
> +
> +	ret = generic_phy_power_off(&priv->phy);
> +	if (ret) {
> +		dev_err(dev, "failed to power off usb phy\n");
> +		return ret;
> +	}
> +
> +	ret = generic_phy_exit(&priv->phy);
> +	if (ret) {
> +		dev_err(dev, "failed to power off usb phy\n");

Shouldn't all those error prints be produced by the PHY subsystem ?

> +		return ret;

[...]

> @@ -1339,6 +1398,8 @@ static int dwc2_usb_remove(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> +	dwc2_shutdown_phy(dev);

This function returns a return value, but it's ignored here ?

>  	dwc2_uninit_common(priv->regs);
>  
>  	reset_release_bulk(&priv->resets);
> 

[...]