Re: [PATCH 2/3] arm64: dts: qcom: sa8295p: Enable tertiary controller and its 4 USB ports

From: Krishna Kurapati PSSNV
Date: Fri Feb 09 2024 - 00:16:01 EST

On 2/9/2024 8:11 AM, Bjorn Andersson wrote:
On Tue, Feb 06, 2024 at 03:24:45PM +0200, Dmitry Baryshkov wrote:
On Tue, 6 Feb 2024 at 14:28, Krishna Kurapati PSSNV
<quic_kriskura@xxxxxxxxxxx> wrote:

On 2/6/2024 5:43 PM, Dmitry Baryshkov wrote:
On Tue, 6 Feb 2024 at 14:03, Krishna Kurapati <quic_kriskura@xxxxxxxxxxx> wrote:

Enable tertiary controller for SA8295P (based on SC8280XP).
Add pinctrl support for usb ports to provide VBUS to connected peripherals.

These are not just pinctrl entries. They hide VBUS regulators. Please
implement them properly as corresponding vbus regulators.

Hi Dmitry. Apologies, can you elaborate on your comment. I thought this
implementation was fine as Konrad reviewed it in v13 [1]. I removed his
RB tag as I made one change of dropping "_state" in labels.

My comment is pretty simple: if I'm not mistaken, your DT doesn't
reflect your hardware design.
You have actual VBUS regulators driven by these GPIO pins. Is this correct?
If so, you should describe them properly in the device tree rather
than describing them just as USB host's pinctrl state.

This is correct, these gpios are wired to the enable-pin of a set of
regulators providing the VBUS voltage. Please, Krishna, represent them
as always-on fixed-regulators instead.

Hi Dmitry, Krzysztof, Bjorn,

Thanks for the review on this patch. Will convert the pinctrl DT's to regulator entries.