Received-SPF: pass (google.com: domain of linux-kernel+bounces-72620-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <e0c44c03-8c66-4a44-9c6d-6b8c83e9d16f@collabora.com>
Date: Tue, 20 Feb 2024 09:54:08 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
To: Michael Walle <mwalle@kernel.org>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Matthias Brugger <matthias.bgg@gmail.com>
Cc: devicetree@vger.kernel.org, Sean Wang <sean.wang@mediatek.com>,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-mediatek@lists.infradead.org
References: <20240219084456.1075445-1-mwalle@kernel.org>
 <20240219084456.1075445-2-mwalle@kernel.org>
 <2ad6bda8-a457-421b-b35d-dc005fb00ae9@collabora.com>
 <CZ92W3VSYV1A.1693O76GY1XDP@kernel.org>
 <b50d49fd-2976-46fc-9f35-354fb39720ad@collabora.com>
 <CZ93XYV3MB10.YRCFWLLHIICY@kernel.org>
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Content-Language: en-US
In-Reply-To: <CZ93XYV3MB10.YRCFWLLHIICY@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Il 19/02/24 14:59, Michael Walle ha scritto:
> On Mon Feb 19, 2024 at 2:35 PM CET, AngeloGioacchino Del Regno wrote:
>>>> vbus is always supplied by something, as otherwise USB won't work - whether this
>>>> is an always-on regulator or a passthrough from external supply this doesn't really
>>>> matter - you should model a regulator-fixed that provides the 5V VBUS line.
>>>
>>> I don't think this is correct, though. Think of an on-board USB
>>> hub. There only D+/D- are connected (and maybe the USB3.2 SerDes
>>> lanes). Or have a look at the M.2 pinout. There is no Vbus.
>>>
>>
>> Yes but the MediaTek MTU3 and/or controllers do have it ;-)
> 
> .. and ..
> 
>>>> For example:
>>>> 	vbus_fixed: regulator-vbus {
>>>> 		compatible = "regulator-fixed";
>>>> 		regulator-name = "usb-vbus";
>>>> 		regulator-always-on;
>>>> 		regulator-boot-on;
>>>> 		regulator-min-microvolt = <5000000>;
>>>> 		regulator-max-microvolt = <5000000>;
>>>> 	};
>>>
>>> As mentioned above, I don't think this will make sense in my case.
>>>   >> P.S.: If the rail has a different name, please use that different name. Obviously
>>>> that requires you to have schematics at hand, and I don't know if you do: if you
>>>> don't, then that regulator-vbus name is just fine.
>>>
>>> I do have the schematics.
>>
>> In that case, you should model the power tree with the fixed power lines,
>> check mt8195-cherry (and/or cherry-tomato) and radxa-nio-12l; even though
>> those are technically "doing nothing", this is device tree, so it should
>> provide a description of the hardware ... and the board does have fixed
>> power lines.
>> It has at least one: DC-IN (typec, barrel jack or whatever, the board needs
>> power, doesn't it?!).
> 
> Mh, maybe I don't get it. But within the hardware there is simply no
> Vbus. Thus I'd argue it doesn't make sense to have a vbus-supply
> property. Besides, the mediatek,mtu3.yaml binding lists it as
> deprecated anyway and it should rather be on the connector. There,
> it makes perfectly sense (at least if it's a USB connector).
> 
> Thus in my case, the xhci for the front port has a vbus-supply
> property (but it should rather have a connector node, as I've just
> learned). But the internal port which connects to the USB hub
> shouldn't have one.
> 
> 
>    +-----+           +-----+                  +------+
>    |     |<--Dp/Dn-->|     |<------Dp/Dn----->| USB  |
>    | SoC |           |     |                  | Conn |
>    |     |           |     |   +-----+        |      |
>    +-----+           | USB |   | PWR |--Vbus->|      |
>                      | Hub |   | SW  |        +------+
>                      |     |   +-----+
>                      |     |     ^
>                      |     |     | PRTPWR
>                      |     |-----'
>                      +-----+
> 
> "PWR SW" is a power switch, the input (+5V) isn't shown here. The
> power will be enabled by the USB Hub.
> 

Ok then, put the vbus on the connector, where applicable.

If PWR_SW is a physical switch (not detectable), then you can still model it as an
always-on regulator-fixed, as there's no other clean way of doing that.

As for modeling the power tree with the fixed power lines, I mean that you should
add the power rails as described in your schematics.
You will surely have one DC-IN supply, and others. That should be modeled as a
fixed regulator, which outputs to (probably) the PMIC, using it as an input for
its buck regulators.

Again, your PMIC must be getting a voltage from somewhere in order to regulate
that to a different voltage for a different power rail, right? :-)

Regards,
Angelo