On Thu, Oct 21, 2021 at 11:43:23PM PDT, Greg Kroah-Hartman wrote:
On Thu, Oct 21, 2021 at 07:00:28PM -0700, Zev Weiss wrote:
Per v0.3 of the Devicetree Specification [0]:
Indicates that the device is operational, but should not be used.
Typically this is used for devices that are controlled by another
software component, such as platform firmware.
One use-case for this is in OpenBMC, where certain devices (such as a
BIOS flash chip) may be shared by the host and the BMC, but cannot be
accessed by the BMC during its usual boot-time device probing, because
they require additional (potentially elaborate) coordination with the
host to arbitrate which processor is controlling the device.
Devices marked with this status should thus be instantiated, but not
have a driver bound to them or be otherwise touched.
[0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf
Signed-off-by: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
---
drivers/of/base.c | 56 +++++++++++++++++++++++++++++++++++++++-------
include/linux/of.h | 6 +++++
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 0ac17256258d..3bd7c5b8a2cc 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -580,14 +580,16 @@ int of_machine_is_compatible(const char *compat)
EXPORT_SYMBOL(of_machine_is_compatible);
/**
- * __of_device_is_available - check if a device is available for use
+ * __of_device_check_status - check if a device's status matches a particular string
*
- * @device: Node to check for availability, with locks already held
+ * @device: Node to check status of, with locks already held
+ * @val: Status string to check for, or NULL for "okay"/"ok"
*
- * Return: True if the status property is absent or set to "okay" or "ok",
- * false otherwise
+ * Return: True if status property exists and matches @val, or either "okay"
+ * or "ok" if @val is NULL, or if status property is absent and @val is
+ * "okay", "ok", or NULL. False otherwise.
*/
-static bool __of_device_is_available(const struct device_node *device)
+static bool __of_device_check_status(const struct device_node *device, const char *val)
{
const char *status;
int statlen;
@@ -596,17 +598,35 @@ static bool __of_device_is_available(const struct device_node *device)
return false;
status = __of_get_property(device, "status", &statlen);
- if (status == NULL)
- return true;
+ if (!status) {
+ /* a missing status property is treated as "okay" */
+ status = "okay";
+ statlen = strlen(status) + 1; /* property lengths include the NUL terminator */
+ }
if (statlen > 0) {
- if (!strcmp(status, "okay") || !strcmp(status, "ok"))
+ if (!val && (!strcmp(status, "okay") || !strcmp(status, "ok")))
+ return true;
+ else if (val && !strcmp(status, val))
Ick, where is this string coming from? The kernel or userspace or a
device tree? This feels very wrong, why is the kernel doing parsing
like this of different options that all mean the same thing?
Which string do you mean by "this string"? 'status' comes from a
property of the device tree node; 'val' will be one of a small set of
string constants passed by the caller. Accepting either spelling of
"okay"/"ok" has been in place since 2008 (commit 834d97d45220,
"[POWERPC] Add of_device_is_available function"); using NULL as a
shorthand for those two strings was a suggestion that came up in review
feedback on a previous incarnation of these patches
(https://lore.kernel.org/openbmc/CAL_Jsq+rKGv39zHTxNx0A7=X4K48nXRLqWonecG5SobdJq3yKw@xxxxxxxxxxxxxx/T/#u).
return true;
}
return false;
}
+/**
+ * __of_device_is_available - check if a device is available for use
+ *
+ * @device: Node to check for availability, with locks already held
+ *
+ * Return: True if the status property is absent or set to "okay" or "ok",
+ * false otherwise
+ */
+static bool __of_device_is_available(const struct device_node *device)
+{
+ return __of_device_check_status(device, NULL);
+}
+
/**
* of_device_is_available - check if a device is available for use
*
@@ -628,6 +648,26 @@ bool of_device_is_available(const struct device_node *device)
}
EXPORT_SYMBOL(of_device_is_available);
+/**
+ * of_device_is_reserved - check if a device is marked as reserved
+ *
+ * @device: Node to check for reservation
+ *
+ * Return: True if the status property is set to "reserved", false otherwise
+ */
+bool of_device_is_reserved(const struct device_node *device)
+{
+ unsigned long flags;
+ bool res;
+
+ raw_spin_lock_irqsave(&devtree_lock, flags);
+ res = __of_device_check_status(device, "reserved");
+ raw_spin_unlock_irqrestore(&devtree_lock, flags);
Why is this a "raw" spinlock?
devtree_lock being a raw_spinlock_t appears to date from commit
d6d3c4e65651 ("OF: convert devtree lock from rw_lock to raw spinlock");
"required for preempt-rt", according to Thomas Gleixner's commit
message.
Where is this status coming from?
This would be specified in a DT node, e.g. via something like:
&somedev {
compatible = "foobar";
status = "reserved";
/* ... */
};
+
+ return res;
+}
+EXPORT_SYMBOL(of_device_is_reserved);
EXPORT_SYMBOL_GPL()?
Its closest existing sibling, of_device_is_available(), is a plain
EXPORT_SYMBOL(); if we want to convert things more broadly that'd be
fine with me, but having one be GPL-only and the other not seems like
it'd be a bit confusing and inconsistent?
Thanks,
Zev
