Hi Emil,
On Fri, Dec 01, 2023 at 03:28:27PM +0100, Emil Renner Berthing wrote:
Nam Cao wrote:
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c index 530fe340a9a1..561fd0c6b9b0 100644 --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c @@ -492,7 +492,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
nmaps = 0; ngroups = 0;
- for_each_child_of_node(np, child) {
- for_each_available_child_of_node(np, child) {
Is this safe to do? I mean will the children considered "available" not change as drivers are loaded during boot so this is racy?
I think if node removal like this causes race condition, we would already have race condition with node addition too: "what if the nodes are added while the drivers are being loaded?"
At least with U-Boot, the device tree overlay is "merged" into the base device tree, before the kernel even runs, so no race there. I don't know if there are any cases where the device tree overlay is not guaranteed to be applied before driver loading, but those cases do not sound sane to me: they would cause race condition, regardless of whether nodes are added or removed.
Also arguably this is not a bugfix, but a new feature.
I'm not sure myself, I haven't seen official documentation/rules about this. But many people do consider this to be a bug:
"Though you can add/override 'status' with 'status = "disabled";' which should be treated very similar to a node not being present. I say similar because it's a source of bugs for the OS to fail to pay attention to status property." - Rob Herring [1].
"Linux has widespread use of the "status" property to indicate that a node does not exist. (...). Expect efforts to fix the kernel code to respect the "status" property." - elinux.org [2].
And I do agree with them. When someone write a device tree with some nodes with "status = disabled" for whatever reasons, clearly they intend to exclude these nodes.
Though I must admit that I am still quite new, so please correct me if my reasoning/understanding is flawed.
Best regards, Nam
[1] https://lore.kernel.org/lkml/CAL_JsqLV5d5cL3o3Dx=--zGD37c5O09rL9AXyRFmceTfBH... [2] https://elinux.org/Device_Tree_Linux#status_property