Sascha, Robin,
On 9/5/23 11:03, Robin Murphy wrote:
[You don't often get email from robin.murphy@xxxxxxx. Learn why this is
important at https://aka.ms/LearnAboutSenderIdentification ]
On 2023-09-04 12:58, Sascha Hauer wrote:
Add rockchip,io-domains property to the Rockchip pinctrl driver. This
list of phandles points to the IO domain device(s) the pins of the
pinctrl driver are supplied from.
Also a rockchip,io-domain-boot-on property is added to pin groups
which can be used for pin groups which themselves are needed to access
the regulators an IO domain is driven from.
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
.../bindings/pinctrl/rockchip,pinctrl.yaml | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
index 10c335efe619e..92075419d29cf 100644
--- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
@@ -62,6 +62,11 @@ properties:
Required for at least rk3188 and rk3288. On the rk3368 this
should
point to the PMUGRF syscon.
+ rockchip,io-domains:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ Phandles to io domains
+
"#address-cells":
enum: [1, 2]
@@ -137,7 +142,13 @@ additionalProperties:
- description:
The phandle of a node contains the generic pinconfig
options
to use as described in pinctrl-bindings.txt.
-
+ rockchip,io-domain-boot-on:
I don't think "on" is a particularly descriptive or useful property name
for something that has no "off" state. Furthermore it's no help at all
if the DT consumer *is* the bootloader that's expected to configure this
in the first place. IMO it would seem a lot more sensible to have an
integer (or enum) property which describes the actual value for the
initial I/O domain setting. Then Linux can choose to assume the presence
of the property at all implies that the bootloader should have set it up
already, but also has the option of actively enforcing it as well if we
want to.
This is actually highly misleading. Whether the bootloader handles IO
domains for pinctrl or not absolutely doesn't matter to the kernel since
the kernel is required to handle IO domain for pinctrl as well. They're
not exclusive, they are not complementary.
The point is that the voltage of the IO domain **can** change at
runtime, at any point in time. We could theoretically have the
bootloader require the regulator to be running at 1.8V and then the
kernel at 3.3V, both should work (can't think of anything that would
work like that but why not, the kernel IO domain driver is supposed to
handle that within the kernel runtime even).
The issue here is that we want to avoid a cyclic dependency, where the
pinctrl is needed for the IO domain regulator that we're trying to add
as a dependency of the same pinctrl. There needs to be either some smart
detection or a property to specify that the IO domain dependency needs
to be ignored. This seems unfortunately to be for working around how
Linux handles dependencies between devices and doesn't allow cyclic
dependencies. At the same time, I do not know if there's anyway to not
work around it?
+ type: boolean
+ description:
+ If true assume that the io domain needed for this pin group
has been
+ configured correctly by the bootloader. This is needed to
break cyclic
+ dependencies introduced when a io domain needs a regulator
that can be
+ accessed through pins configured here.
This is describing a Linux implementation detail, not the binding
itself. There's no technical reason a DT consumer couldn't already
figure this much out from the existing topology (by observing that the
pinctrl consumer is a grandparent of the I/O domain's supply).
I am guessing you're suggesting to have some complex handling in the
driver to detect those cyclic dependencies and ignore the IO domain
dependency for the pinctrl pins where this happens?
This can actually be quite difficult to detect reliably:
We need to go through the phandle in pinctrl to the IO domain DT node,
then check all phandles there to other DT node (likely regulators), then
we need to look into the pinctrl-0 (actually, the one for "default"
maybe, but what about the other states of pinctrl?) phandles and then
parse the pinctrt DT nodes to see if they're pointing to the same DT
node as the one we're trying to use. Here, we also do not know if the
regulator DT node has other dependencies that needs to be accounted for.
I haven't put too much thoughts into it so maybe it's easier/harder than
what I'm saying here (or maybe I'm completely off too...).
One of the issues we're having here too is that we lose granularity.
There are multiple domains inside an IO domain device and here we make
the whole pinctrl device depend on all domains from one IO domain device
(there can be multiple ones) while it is factually (on the HW level)
only dependent on one domain. Considering (if I remember correctly)
Heiko highly suggested we think about adding child nodes to the IO
domain devices to have a DT node per domain in the IO domain device, how
would this work with the suggested DT binding?
Cheers,
Quentin
