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

[Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>]

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@quicinc.com> wrote:
>>>
>>>
>>>
>>> On 2/6/2024 5:43 PM, Dmitry Baryshkov wrote:
>>>> On Tue, 6 Feb 2024 at 14:03, Krishna Kurapati <quic_kriskura@quicinc.com> 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.

Regards,
Krishna,