Re: [PATCH 2/2] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg

[Konrad Dybcio]

On 5/20/26 1:21 PM, Rakesh Kota wrote:
> On Wed, May 20, 2026 at 11:51:49AM +0200, Konrad Dybcio wrote:
>> On 5/20/26 11:07 AM, Rakesh Kota wrote:
>>> The PM4125 PMIC uses a different register layout for USB VBUS control
>>> compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
>>> at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
>>> a current-limit selector.
>>>
>>> Introduce per-compatible regulator descriptor data to accommodate these
>>> differences. This keeps the existing PM8150B current-limit logic intact
>>> while adding a dedicated voltage-selector path for PM4125.
>>>
>>> Signed-off-by: Rakesh Kota <rakesh.kota@xxxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/regulator/qcom_usb_vbus-regulator.c | 102 ++++++++++++++++++++++++----
>>>  1 file changed, 88 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
>>> index cd94ed67621fee9f6d7a0327054db0ebab6cc7ee..3d425452a0b35b35c4b454f84eb28e87cc8ba4f8 100644
>>> --- a/drivers/regulator/qcom_usb_vbus-regulator.c
>>> +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
>>> @@ -20,10 +20,35 @@
>>>  #define OTG_CFG				0x53
>>>  #define OTG_EN_SRC_CFG			BIT(1)
>>>  
>>> +#define PM4125_CMD_OTG			0x50
>>
>> This register is named differently
> Ok, i will update name to PM4125_VBOOST_EN in next version.
>>
>>> +#define PM4125_VBOOST_CFG		0x52
>>> +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
>>> +#define PM4125_OTG_CFG			0x56
>>
>> And so is this one
> update name to PM4125_VBOOST_CFG in next version
>>
>>
>>> +#define PM4125_OTG_EN_SRC_CFG          BIT(0)
>>> +
>>> +struct qcom_usb_vbus_reg_data {
>>> +	u16 cmd_otg;
>>> +	u16 otg_cfg;
>>> +	u8  otg_en_src_cfg;
>>> +	u16 csel_reg;
>>> +	u8 csel_mask;
>>> +	const unsigned int *curr_table;
>>> +	unsigned int n_current_limits;
>>> +	u16 vsel_reg;
>>> +	u8 vsel_mask;
>>> +	const unsigned int *volt_table;
>>> +	unsigned int n_voltages;
>>> +	const struct regulator_ops *ops;
>>> +};
>>
>> And because they're so different, keeping them in a single driver starts to
>> look a little odd
> 
> You are right, they differ in control mechanism — one uses current
> control and the other uses voltage control for VBUS in host mode.
> Since the functional purpose is the same (VBUS regulation in host
> mode), I felt a single driver made sense to avoid code duplication.
> 
> But open to suggestions — should I split this into separate drivers
> or is there a better way to handle this cleanly?
>>
>>> +
>>>  static const unsigned int curr_table[] = {
>>>  	500000, 1000000, 1500000, 2000000, 2500000, 3000000,
>>>  };
>>
>> And I think there's current-limiting on this PMIC too, except in a differnt
>> peripheral (@1300)
>>
> The @1300 peripheral is for ICL (Input Current Limit) in device mode
> charging — not for host mode VBUS regulation.
> 
> Could you share the register you're referring to for further clarity?

I think I looked at ICL_OPTIONS (+0x50) and ICL_CFG (+0x52), but yeah,
it seems like I mistook the two kinds of currents at play.

Konrad