From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Zev Weiss <zev@bewilderbeest.net>
Cc: Frank Rowand <frowand.list@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
openbmc@lists.ozlabs.org, Jeremy Kerr <jk@codeconstruct.com.au>,
Joel Stanley <joel@jms.id.au>, Andrew Jeffery <andrew@aj.id.au>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] of: base: add function to check for status = "reserved"
Date: Fri, 22 Oct 2021 08:43:23 +0200 [thread overview]
Message-ID: <YXJdi3IBzaqmSZ9b@kroah.com> (raw)
In-Reply-To: <20211022020032.26980-2-zev@bewilderbeest.net>
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@bewilderbeest.net>
> ---
> 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?
> 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?
Where is this status coming from?
> +
> + return res;
> +}
> +EXPORT_SYMBOL(of_device_is_reserved);
EXPORT_SYMBOL_GPL()?
thanks,
greg k-h
next prev parent reply other threads:[~2021-10-22 6:43 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-22 2:00 [PATCH 0/5] driver core, of: support for reserved devices Zev Weiss
2021-10-22 2:00 ` [PATCH 1/5] of: base: add function to check for status = "reserved" Zev Weiss
2021-10-22 6:43 ` Greg Kroah-Hartman [this message]
2021-10-22 7:38 ` Zev Weiss
2021-10-22 7:45 ` Greg Kroah-Hartman
2021-10-22 2:00 ` [PATCH 2/5] device property: add fwnode_device_is_reserved() Zev Weiss
2021-10-22 2:00 ` [PATCH 3/5] of: property: add support for fwnode_device_is_reserved() Zev Weiss
2021-10-22 2:00 ` [PATCH 4/5] driver core: inhibit automatic driver binding on reserved devices Zev Weiss
2021-10-22 6:46 ` Greg Kroah-Hartman
2021-10-22 8:32 ` Zev Weiss
2021-10-22 8:57 ` Greg Kroah-Hartman
2021-10-22 15:18 ` Patrick Williams
2021-10-23 8:56 ` Greg Kroah-Hartman
2021-10-25 5:38 ` Frank Rowand
2021-10-25 6:15 ` Greg Kroah-Hartman
2021-10-25 11:44 ` Patrick Williams
2021-10-25 12:58 ` Andy Shevchenko
2021-10-25 13:20 ` Patrick Williams
2021-10-25 13:34 ` Greg Kroah-Hartman
2021-10-25 14:02 ` Patrick Williams
2021-10-25 14:09 ` Greg Kroah-Hartman
2021-10-25 15:54 ` Patrick Williams
2021-10-25 18:36 ` Greg Kroah-Hartman
2021-10-22 16:27 ` Zev Weiss
2021-10-23 8:55 ` Greg Kroah-Hartman
2021-10-22 2:00 ` [PATCH 5/5] of: platform: instantiate " Zev Weiss
2021-10-22 2:58 ` [PATCH 0/5] driver core, of: support for " Rob Herring
2021-10-22 3:13 ` Zev Weiss
2021-10-22 6:50 ` Greg Kroah-Hartman
2021-10-22 6:50 ` Greg Kroah-Hartman
2021-10-22 9:00 ` Zev Weiss
2021-10-22 9:22 ` Greg Kroah-Hartman
2021-10-25 5:53 ` Frank Rowand
2021-10-25 13:57 ` Frank Rowand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YXJdi3IBzaqmSZ9b@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=andrew@aj.id.au \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=jk@codeconstruct.com.au \
--cc=joel@jms.id.au \
--cc=linux-kernel@vger.kernel.org \
--cc=openbmc@lists.ozlabs.org \
--cc=robh+dt@kernel.org \
--cc=zev@bewilderbeest.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).