Re: [PATCH] regulator: Fix current limit for QCOM PMIC VBUS

From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
To: Mark Brown <broonie@kernel.org>
Cc: agross@kernel.org, bjorn.andersson@linaro.org,
	lgirdwood@gmail.com, wcheng@codeaurora.org,
	linux-arm-msm@vger.kernel.org, dmitry.baryshkov@linaro.org
Subject: Re: [PATCH] regulator: Fix current limit for QCOM PMIC VBUS
Date: Fri, 23 Apr 2021 21:38:51 +0100	[thread overview]
Message-ID: <894579a9-8ce0-affb-b6cb-4ce4b3d797ec@linaro.org> (raw)
In-Reply-To: <a2152494-4e8f-27ef-07e7-e575ce3ee653@linaro.org>

On 23/04/2021 21:24, Bryan O'Donoghue wrote:
> On 23/04/2021 17:32, Mark Brown wrote:
>> On Fri, Apr 23, 2021 at 05:06:58PM +0100, Bryan O'Donoghue wrote:
>>
>>> +    /* Set OTG current limit to 3000mA up from bootloader set 2000mA */
>>> +    regmap_update_bits(regmap, base + OTG_CURRENT_LIMIT_CFG,
>>> +               OTG_CURRENT_LIMIT_MASK, OTG_CURRENT_LIMIT_3000MA);
>>
>> This seems like something that needs to be configured per system, the
>> system may not be physically capable of delivering an additional amp
>> (150% of the current defaults) safely.  It's going to be better to be
>> out of spec for high current USB devices than to exceed safe physical
>> limits, there's a good solid reason why the regulator API never touches
>> the hardware without explicit constraints allowing it to do so.
> 
> You're right that its per system.
> 
> - smb5_pmi632_params.otg_cl.max_u == 1000000
> - smb5_pm8150b_params.otg_cl.max_u == 3000000
> 
> https://github.com/BeastRoms-Devices/kernel_realme_RMX1927/blame/master/drivers/power/supply/qcom/qpnp-smb5.c 
> 
> 
> See page 3 here
> 
> https://www.st.com/content/ccc/resource/sales_and_marketing/presentation/product_presentation/group0/5a/b1/8e/6c/2b/0d/46/3c/Apec/files/APEC_2016_USB_Power.pdf/_jcr_content/translations/en.APEC_2016_USB_Power.pdf 
> 
> 
> and section 1.2.2 here
> 
> https://cdn.sparkfun.com/assets/e/b/4/f/7/USB-C_Datasheet.pdf
> 
> For type-c 3 Amps at 5 Volts is correct or 1.5 Amps at 5 Volts, either 
> way the default coming out of the bootloader at 2 Amps is wrong.
> 
>> I also thought there was support in the USB specs for negotiating how
>> much power is drawn (not that everything will DTRT there but still)?
> 
> Right the Power-delivery stuff up to 100 watts is associated with Vconn 
> which is driven on CC1 or CC2 depending on the orientation of the cable.
> 
> https://www.allaboutcircuits.com/uploads/articles/Fig1m11292018.png
> 
> This patch deals with the higher spec VBUS current that the pm8150b can 
> drive, not the power-delivery protocol juice over VCONN.
> 
> ---
> bod

Oh and I should mention the pm8150b is a type-c controller and battery 
charger block, so you wouldn't connect it to a USB 3.x type-a port.

The Qualcomm reference schematics drive VBUS for those ports from a 
current limiter - a SY6288F1ABC - which steps down 5v/4amp rail to 
5v/1.2amp.

So the pm8150b is exclusively for the type-c not type-a ports.

---
bod