Received-SPF: pass (google.com: domain of linux-kernel+bounces-207431-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <9150cfa3-ab22-4b1e-a392-2cb9469f3821@mail.de>
Date: Sun, 9 Jun 2024 18:46:09 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v2 2/2] arm64: dts: rockchip: Add FriendlyElec CM3588 NAS
 board
To: =?UTF-8?Q?Heiko_St=C3=BCbner?= <heiko@sntech.de>,
 linux-rockchip@lists.infradead.org,
 Sebastian Reichel <sebastian.reichel@collabora.com>,
 Space Meyer <me@the-space.agency>
Cc: Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,
 Conor Dooley <conor+dt@kernel.org>, Jonas Karlman <jonas@kwiboo.se>,
 Dragan Simic <dsimic@manjaro.org>, devicetree@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20240602211901.237769-1-seb-dev@mail.de>
 <11747652.CDJkKcVGEf@phil> <f5cfcf3e-27e5-464a-9adf-261753ad6de7@mail.de>
 <4035271.e99z0qppnp@diego>
From: Sebastian Kropatsch <seb-dev@mail.de>
In-Reply-To: <4035271.e99z0qppnp@diego>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Am 09.06.2024 um 18:05 schrieb Heiko Stübner:
> Am Samstag, 8. Juni 2024, 19:22:01 CEST schrieb Sebastian Kropatsch:
>> Am 08.06.2024 um 16:38 schrieb Heiko Stuebner:
>>> Am Donnerstag, 6. Juni 2024, 15:13:20 CEST schrieb Space Meyer:
>>>> On 02.06.2024 22:20, Sebastian Kropatsch wrote:
>>>>> +	vcc_3v3_pcie30: regulator-vcc-3v3-pcie30 {
>>>>> +		compatible = "regulator-fixed";
>>>>> +		regulator-name = "vcc_3v3_pcie30";
>>>>> +		regulator-always-on;
>>>>> +		regulator-boot-on;
>>>>> +		regulator-min-microvolt = <3300000>;
>>>>> +		regulator-max-microvolt = <3300000>;
>>>>> +		vin-supply = <&vcc_5v0_sys>;
>>>>> +	};
>>>>
>>>> These are 4 seperate regulators according to the schematic. However, as
>>>> they are all fixed, idk if they should be split or kept like this.
>>>
>>> personally, I really like the power-diagram to match schematics.
>>> I.e. $debugfs/regulator/regulator_summary will produce a really nice
>>> graph of all the system's regulators, so it's definitly nice if the
>>> hirarchy matches. Also prevents head-scratching later on ;-)
>>
>> These are indeed 4 different regulators according to the schematic.[1]
>> But they don't have any pin to control them separately. I can
>> duplicate them 4 times if that's the preferred practice.
>>
>> But matching the schematics won't be possible either way, since
>> e.g. there is only one single 5v regulator acc. to the schematic
>> (vcc_5v0_sys), but vcc_5v0_host_20, vcc_5v0_host_30, vbus_5v0_typec
>> and so on are needed since each device has a different control pin
>> to enable its power. Or is there a better way to solve this while
>> having only one 5v regulator node but still being able to set the
>> control pins separately for the different USB ports?
> 
> The other option we often use is to define multiple phandles
> for a regulator. For exactly that case where one gpio controls
> a set of regulators.
> 
> So you have one regulator
> 
> vcc_5v0_host_20: vcc_5v0_host_30: vbus_5v0_typec: regulator-vcc-whatever {
> 	foo;
> }
> 
> So in short there is not set rule, but more like a best-effort to get as
> close to the schematics as possible. I.e. someone going from dt
> to schematics should be able to just search for an identifier
> (of course same for the other direction).

I see. And then later in the file have something like this?

&vcc_5v0_host_20{
	gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_HIGH>;
};

&vcc_5v0_host_30{
	gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
};

&vbus_5v0_typec{
	gpios = <&gpio1 RK_PD2 GPIO_ACTIVE_HIGH>;
};

Will this work? I thought these gpios would then all just go into
the above mentioned "regulator-vcc-whatever" node, overwriting
each other. If so, I think the solution of keeping them as separate
regulators would still be fine, since as Space Meyer mentioned that,
although all USBs are connected to the same regulator, they have one
SY6280AAC power switch each and these switches have enable pins (which
are seen above in the gpios).


Cheers,
Sebastian