Re: [PATCH 2/2] of: property: fw_devlink: Set 'optional_con_dev' for parse_power_domains
flat view
From: Saravana Kannan <hidden>
Date: 2021-08-31 17:51:14
Also in:
linux-arm-kernel, linux-pm, lkml
On Tue, Aug 31, 2021 at 3:21 AM Ulf Hansson [off-list ref] wrote:
The power-domain DT bindings [1] doesn't enforce a compatible string for a provider node, even if this is common to use. In particular, when describing a hierarchy with parent/child power-domains, as the psci DT bindings [2] for example, it's sometimes not applicable to use a compatible string.
Ok, and fw_devlink handles that -- provider not having a compatible string is pretty common. In these cases, the parent node is the actual device that gets probed and registers the provider. So fw_devlink will create a link from the consumer to the parent device node.
Therefore, let's set the 'optional_con_dev' to true to avoid creating incorrect fw_devlinks for power-domains.
This part doesn't make sense or is incomplete. What is being done incorrectly?
[1] Documentation/devicetree/bindings/power/power-domain.yaml [2] Documentation/devicetree/bindings/arm/psci.yaml Signed-off-by: Ulf Hansson <redacted> --- Some more details of what goes on here. I have added a debug print in of_link_to_phandle() to see the fw_devlinks that gets created. This is what happens on Dragonboard 410c when 'optional_con_dev' isn't set: ... [ 0.041274] device: 'psci': device_add [ 0.041366] OF: Linking power-domain-cpu0 (consumer) to psci (supplier) [ 0.041395] OF: Linking power-domain-cpu1 (consumer) to psci (supplier) [ 0.041423] OF: Linking power-domain-cpu2 (consumer) to psci (supplier) [ 0.041451] OF: Linking power-domain-cpu3 (consumer) to psci (supplier) [ 0.041494] device: 'platform:psci--platform:psci': device_add [ 0.041556] platform psci: Linked as a sync state only consumer to psci ... This is what happens on Dragonboard 410c when 'optional_con_dev' is set: ... [ 0.041179] device: 'psci': device_add [ 0.041265] OF: Not linking psci to psci - is descendant [ 0.041293] OF: Not linking psci to psci - is descendant [ 0.041319] OF: Not linking psci to psci - is descendant [ 0.041346] OF: Not linking psci to psci - is descendant ...
Can you please explain what exactly is going on that's wrong here? I notice that psci is not probed as a device at all. And when you aren't setting this flag the only difference I see is the creating of a sync state only link -- which shouldn't matter here because you don't even have a driver implemented.
quoted hunk ↗ jump to hunk
The relevant dtsi file: arch/arm64/boot/dts/qcom/msm8916.dtsi Kind regards Ulf Hansson --- drivers/of/property.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/of/property.c b/drivers/of/property.c index 2babb1807228..4d607fdbea24 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c@@ -1356,7 +1356,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_io_channels, }, { .parse_prop = parse_interrupt_parent, }, { .parse_prop = parse_dmas, .optional = true, }, - { .parse_prop = parse_power_domains, }, + { .parse_prop = parse_power_domains, .optional_con_dev = true, },
This change is just shooting in dark/completely unrelated to the commit text. This is just saying the actual consumer is a level up from where the property is listed (eg: remote-endpoint). It just happens to fix your case for unrelated reasons. Definite Nak as this *will* break other cases. -Saravana
{ .parse_prop = parse_hwlocks, },
{ .parse_prop = parse_extcon, },
{ .parse_prop = parse_nvmem_cells, },
--
2.25.1