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

Thomas Richard
2026-03-31T18:19:52Z

Hello Lee,

On 3/31/26 6:55 PM, Lee Jones wrote:
> 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?
>

Thanks a lot for the review. I already sent a v2, do not spend your time
on this version. This comment is also valid for v2 (and the headers
one), but I fixed -ENODEV in v2.

Best Regards,
Thomas