[PATCH v4 3/3] usb: ohci-at91: Add USB PHY functionality

[Marek Vasut]

On 6/22/23 15:07, Eugen Hristev wrote:
> From: Sergiu Moga <sergiu.moga at microchip.com>
> 
> Add the ability to enable/disable whatever USB PHY's are
> passed to the AT91 OHCI driver through DT.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga at microchip.com>
> Tested-by: Mihai Sain <mihai.sain at microchip.com>
> Reviewed-by: Marek Vasut <marex at denx.de>
> [eugen.hristev at collabora.com: disable PHYs in at91_start_clock if
> clk_enable_bulk fails]
> Signed-off-by: Eugen Hristev <eugen.hristev at collabora.com>
> ---
> 
> 
> v1 -> v2:
> - use *_bulk API's
> 
> 
> v2 -> v3:
> - use if (CONFIG_IS_ENABLED(...))
> 
> v3 -> v4
> [eugen.hristev at collabora.com: disable PHYs in at91_start_clock if
> clk_enable_bulk fails]
> 
>   drivers/usb/host/ohci-at91.c | 34 +++++++++++++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 92d0ab7184c4..3d4f31d240aa 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -73,6 +73,7 @@ int usb_cpu_init_fail(void)
>   #include <asm/gpio.h>
>   #include <usb.h>
>   #include "ohci.h"
> +#include <generic-phy.h>
>   
>   #define AT91_MAX_USBH_PORTS        3
>   
> @@ -90,15 +91,36 @@ struct at91_usbh_data {
>   struct ohci_at91_priv {
>   	ohci_t ohci;
>   	struct clk_bulk clks;
> +	struct phy_bulk phys;
>   };
>   
>   static int at91_start_clock(struct ohci_at91_priv *ohci_at91)
>   {
> -	return clk_enable_bulk(&ohci_at91->clks);
> +	int ret;
> +
> +	if (CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)) {
> +		ret = generic_phy_power_on_bulk(&ohci_at91->phys);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = clk_enable_bulk(&ohci_at91->clks);
> +	if (ret)
> +		generic_phy_power_off_bulk(&ohci_at91->phys);

A fail path (goto err;) would be nicer here.

> +	return ret;
>   }
>   
>   static int at91_stop_clock(struct ohci_at91_priv *ohci_at91)
>   {
> +	if (CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)) {
> +		int ret;
> +
> +		ret = generic_phy_power_off_bulk(&ohci_at91->phys);
> +		if (ret)
> +			return ret;

Shouldn't this one ignore the return value and instead proceed to 
disable clock below ?

> +	}
> +
>   	return clk_disable_bulk(&ohci_at91->clks);
>   }