Message-ID: <acb47522-920d-46aa-8c65-440f7c0b31e2@linaro.org>
Date: Wed, 12 Feb 2025 17:12:29 +0000
From: Caleb Connolly <caleb.connolly@...aro.org>
To: "James A. MacInnes" <james.a.macinnes@...il.com>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, andersson@...nel.org, konradybcio@...nel.org,
 quic_wcheng@...cinc.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, lgirdwood@...il.com, broonie@...nel.org
Subject: Re: [PATCH 2/3] regulator: qcom_usb_vbus: Add support for PMI8998
 VBUS

Hi James,

On 2/12/25 16:56, James A. MacInnes wrote:
> On Wed, 12 Feb 2025 15:29:54 +0000
> Caleb Connolly <caleb.connolly@...aro.org> wrote:
> 
> 
> Hi Caleb,
> 
>> Hi James,
>>
>> On 2/12/25 01:07, James A. MacInnes wrote:
>>> This patch extends the Qualcomm USB VBUS regulator driver to support
>>> PMI8998 PMIC alongside the existing support for PM8150B.
>>
>> Thanks for the patch!
> 
> Happy to try and contribute. I know that the working Type-C port is
> going to be a misery after the relative simplicity of pushing the VBUS
> upstream.

Yeah, it's hard to get used to the process... I'm happy to hear you're 
trying to get type-c working on this platform though! This will make a 
bunch of folks very happy if it finally lands, folks have been doing all 
sorts of workarounds for this over on 
https://wiki.postmarketos.org/wiki/OnePlus_6_(oneplus-enchilada)#OTG_doesn't_work


>>>
>>> Key changes:
>>> - Added current limit tables specific to PMI8998.
>>
>> I also played around with vbus on PMI8998 before for type-c support
>> (unfortunately didn't make it's way to the lists yet...) and I needed
>> some additional changes for this to work correctly. I found that it
>> was possible for the overcurrent protection to be hit if the type-c
>> port manager allowed the peripheral to pull current too early, and
>> it's necessary to allow 2.5ms enable time.
>>
>> PM8150b doesn't have these limitations (and supports the instant
>> power role switch feature that's part of the type-c PD spec, allowing
>> the power role to be switched without either side losing power e.g.
>> when you unplug the power supply from a dock), hence it's only
>> necessary for PMI8998.
>>
>> I would suggest implementing a proper .is_enabled op to poll the
>> status register for OTG_STATE_ENABLED and configuring
>> qcom_usb_vbus_rdesc.enable_time = 250000;
>>
>> Kind regards,
>>
> 
> Technical question for you in regards to the VBUS overcurrent and
> timing for the PMI8998. I would like to try and reproduce what you have
> seen as my system hasn't had switching issues, but then again the TCPM
> system may be covering the exact bug you are mentioning. I also
> searched for some definite bit in the 4.9 Android driver and was left
> wanting. As of yet, I have not had issues with the overcurrent
> protecction.

I guess there could be differences in our implementations, there is a 
delay after enabling the regulator for pm8150b type-c iirc

I never got around to chasing some of the remaining issues and sending 
this upstream, but maybe my patches will be useful for you:

https://git.codelinaro.org/caleb_connolly/kernel/-/commits/b4/pmi8998-typec/?ref_type=heads

I spent many many hours pouring over the smb2 driver downstream, so feel 
free to reach out if you have some specific questions.
> 
> I will be all too happy to migrate to the PM8150B and its associated
> SoCs and leave the 845 platform to history.

Well I for one am always happy to see SDM845 getting attention, it has a 
special place in my heart :>

And still gets love over here, even if not nearly enough of these 
patches have made it upstream: 
https://gitlab.com/sdm845-mainline/linux/-/commits/sdm845/6.13-release?ref_type=heads

Kind regards,
> 
> Thank you for your feedback and I look forward to narrowing down this
> issue.
> 
> Best wishes,

-- 
Caleb (they/them)

Powered by blists - more mailing lists