Re: [PATCH 7/8] mfd: omap-usb-host: Add pbias regulator support

[Lee Jones <lee@kernel.org>]

On Mon, 23 Mar 2026, Thomas Richard wrote:

> Add pbias regulator support to enable SIM_VDDS supply and unlock USB I/O
> cell. Previously, this was handled by the bootloader, now the kernel can
> take responsibility for managing the PBIAS regulator, ensuring correct
> operation regardless of the bootloader.
> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>  drivers/mfd/omap-usb-host.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index ac974285be341fa579ef198d1893b77af428b5f8..9e254e00183e940b775d5bde6e891f0d26af27b0 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -15,6 +15,9 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/string_choices.h>
> +

?

>  
>  #include "omap-usb.h"
>  
> @@ -95,6 +98,8 @@ struct usbhs_hcd_omap {
>  	struct usbhs_omap_platform_data	*pdata;
>  
>  	u32				usbhs_rev;
> +
> +	struct regulator		*pbias;
>  };
>  /*-------------------------------------------------------------------------*/
>  
> @@ -270,6 +275,25 @@ static bool is_ohci_port(enum usbhs_omap_port_mode pmode)
>  	}
>  }
>  
> +static int omap_usbhs_set_pbias(struct device *dev, bool power_on)
> +{
> +	struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!omap->pbias)
> +		return 0;
> +
> +	if (power_on)
> +		ret = regulator_enable(omap->pbias);
> +	else
> +		ret = regulator_disable(omap->pbias);
> +
> +	if (ret)
> +		dev_err(dev, "pbias reg %s failed\n", str_enable_disable(power_on));
> +
> +	return ret;
> +}
> +
>  static int usbhs_runtime_resume(struct device *dev)
>  {
>  	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
> @@ -278,6 +302,10 @@ static int usbhs_runtime_resume(struct device *dev)
>  
>  	dev_dbg(dev, "usbhs_runtime_resume\n");
>  
> +	r = omap_usbhs_set_pbias(dev, true);
> +	if (r)
> +		return r;
> +
>  	omap_tll_enable(pdata);
>  
>  	if (!IS_ERR(omap->ehci_logic_fck))
> @@ -355,7 +383,7 @@ static int usbhs_runtime_suspend(struct device *dev)
>  
>  	omap_tll_disable(pdata);
>  
> -	return 0;
> +	return omap_usbhs_set_pbias(dev, false);
>  }
>  
>  static unsigned omap_usbhs_rev1_hostconfig(struct usbhs_hcd_omap *omap,
> @@ -564,6 +592,11 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>  
>  	omap->pdata = pdata;
>  
> +	omap->pbias = devm_regulator_get_optional(dev, "pbias");
> +	if (IS_ERR(omap->pbias))
> +		return dev_err_probe(dev, PTR_ERR(omap->pbias),
> +				     "unable to get pbias regulator\n");

You need to check for '-ENODEV' here or you are ignoring the optional part.

> +
>  	/* Initialize the TLL subsystem */
>  	omap_tll_init(pdata);
>  
> @@ -759,6 +792,10 @@ static int usbhs_omap_probe(struct platform_device *pdev)
>  	}
>  
>  initialize:
> +	ret = omap_usbhs_set_pbias(dev, true);
> +	if (ret)
> +		goto err_mem;

Since this regulator is also managed by 'usbhs_runtime_resume' and
'usbhs_runtime_suspend', could manually enabling it here in probe and
disabling it in remove interfere with the reference counting during runtime
PM transitions? Should we consider relying entirely on runtime PM to manage
its state?

> +
>  	omap_usbhs_init(dev);
>  
>  	if (dev->of_node) {
> @@ -806,6 +843,8 @@ static void usbhs_omap_remove(struct platform_device *pdev)
>  		of_platform_depopulate(&pdev->dev);
>  	else
>  		device_for_each_child(&pdev->dev, NULL, usbhs_omap_remove_child);
> +
> +	omap_usbhs_set_pbias(&pdev->dev, false);
>  }
>  
>  static const struct dev_pm_ops usbhsomap_dev_pm_ops = {
> 
> -- 
> 2.53.0
> 

-- 
Lee Jones [李琼斯]