lkml.org 
[lkml]   [2021]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] of: base: Skip CPU nodes with non-"okay"/"disabled" status
From
Date
On Sun, 2021-11-14 at 14:41 -0500, Frank Rowand wrote:
> On 11/8/21 3:48 AM, Matthias Schiffer wrote:
> > Allow fully disabling CPU nodes using status = "fail". Having no status
> > property at all is still interpreted as "okay" as usual.
> >
> > This allows a bootloader to change the number of available CPUs (for
> > example when a common DTS is used for SoC variants with different numbers
> > of cores) without deleting the nodes altogether, which could require
> > additional fixups to avoid dangling phandle references.
> >
> > References:
> > - https://www.lkml.org/lkml/2020/8/26/1237
> > - https://www.spinics.net/lists/devicetree-spec/msg01007.html
> > - https://github.com/devicetree-org/dt-schema/pull/61
> >
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > drivers/of/base.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 61de453b885c..4e9973627c8d 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -650,6 +650,32 @@ bool of_device_is_available(const struct device_node *device)
> > }
> > EXPORT_SYMBOL(of_device_is_available);
> >
> > +/**
> > + * __of_device_is_disabled - check if a device has status "disabled"
> > + *
> > + * @device: Node to check status for, with locks already held
> > + *
> > + * Return: True if the status property is set to "disabled",
> > + * false otherwise
> > + *
> > + * Most callers should use __of_device_is_available() instead, this function
> > + * only exists due to the special interpretation of the "disabled" status for
> > + * CPU nodes.
> > + */
> > +static bool __of_device_is_disabled(const struct device_node *device)
> > +{
> > + const char *status;
> > +
> > + if (!device)
> > + return false;
> > +
> > + status = __of_get_property(device, "status", NULL);
> > + if (status == NULL)
> > + return false;
> > +
> > + return !strcmp(status, "disabled");
> > +}
> > +
> > /**
> > * of_device_is_big_endian - check if a device has BE registers
> > *
> > @@ -817,6 +843,9 @@ struct device_node *of_get_next_cpu_node(struct device_node *prev)
> > of_node_put(node);
> > }
> > for (; next; next = next->sibling) {
> > + if (!__of_device_is_available(next) &&
> > + !__of_device_is_disabled(next))
>
> Shouldn't that just be a check to continue if the device is disabled?
>
> If adding a check for available, then all of the callers of for_each_of_cpu_node()
> need to be checked. There is at least one that is suspicious - arch/arm/mach-imx/platsmp.c
> has a comment:
>
> * Initialise the CPU possible map early - this describes the CPUs
> * which may be present or become present in the system.

Previously, there were two option for the (effective) value of the
status of a device_node:

- "okay", "ok" or unset
- anything else (which includes "disabled" and "fail")

__of_device_is_available() checks which of these two is the case.

With the new code, we have 3 cases for the status of CPU nodes:

- "okay", "ok" or unset
- "disabled"
- anything else ("fail", ...)

My patch will only change the behaviour in one case: When a CPU node
has a status that is not "okay", "ok", "disabled" or unset - which is
exactly the point of my patch.

See also the change [1], which removed the !available check a while
ago, and the discussion in [2], which led us to the conclusion that
of_get_next_cpu_node() must not distinguish "okay" and "disabled" CPU
nodes and we instead need a third status to disable a CPU for real.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/of/base.c?id=c961cb3be9064d1097ccc019390f8b5739daafc6
[2] https://www.lkml.org/lkml/2020/8/26/1237


>
> -Frank
>
> > + continue;
> > if (!(of_node_name_eq(next, "cpu") ||
> > __of_node_is_type(next, "cpu")))
> > continue;
> >
>
>

\
 
 \ /
  Last update: 2021-11-15 09:14    [W:3.185 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site