git: b344de4d0d16 - main - Extend device_get_property API
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 10 Mar 2022 11:29:36 UTC
The branch main has been updated by mw:
URL: https://cgit.FreeBSD.org/src/commit/?id=b344de4d0d163cbd8bf88cb5d226c18fe96f488f
commit b344de4d0d163cbd8bf88cb5d226c18fe96f488f
Author: Kornel Duleba <mindal@semihalf.com>
AuthorDate: 2022-01-28 09:28:37 +0000
Commit: Marcin Wojtas <mw@FreeBSD.org>
CommitDate: 2022-03-10 11:11:32 +0000
Extend device_get_property API
In order to support various types of data stored in device
tree properties or ACPI _DSD packages, create a new enum so
the caller can specify the expected type of a property they
want to read, according to the binding. The bus logic will use
that information to process the underlying data.
For example in DT all integer properties are stored in BE format.
In order to get constant results across different platforms we
need to convert its endianness to match the host.
Another example are ACPI_TYPE_INTEGER properties stored
as uint64_t. Before this patch the ACPI logic would refuse
to read them if the provided buffer was smaller than 8 bytes.
Now this can be handled by using DEVICE_PROP_UINT32 type.
Modify the existing consumers of this API to reflect the changes
and update the man pages accordingly.
Reviewed by: mw
Obtained from: Semihalf
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D33457
---
share/man/man9/BUS_GET_PROPERTY.9 | 23 ++++++++++++-----
share/man/man9/device_get_property.9 | 26 +++++++++++++++++--
sys/dev/acpica/acpi.c | 23 +++++++++++++++--
sys/dev/fdt/simplebus.c | 49 +++++++++++++++++++++++++++++++++---
sys/dev/mii/dp83867phy.c | 3 ++-
sys/dev/mmc/mmc_helpers.c | 7 +++---
sys/dev/sdhci/sdhci_xenon.c | 9 ++++---
sys/kern/bus_if.m | 1 +
sys/kern/subr_bus.c | 27 ++++++++++++++++----
sys/sys/bus.h | 21 +++++++++++++---
10 files changed, 160 insertions(+), 29 deletions(-)
diff --git a/share/man/man9/BUS_GET_PROPERTY.9 b/share/man/man9/BUS_GET_PROPERTY.9
index 89edbd95b68c..6d6bc14b4d8f 100644
--- a/share/man/man9/BUS_GET_PROPERTY.9
+++ b/share/man/man9/BUS_GET_PROPERTY.9
@@ -25,7 +25,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd August 19, 2021
+.Dd February 18, 2022
.Dt BUS_GET_PROPERTY 9
.Os
.Sh NAME
@@ -36,27 +36,38 @@
.In sys/bus.h
.Ft ssize_t
.Fn BUS_GET_PROPERTY "device_t dev" "device_t child" "const char *propname" \
- "void *propvalue" "size_t size"
+ "void *propvalue" "size_t size" "device_property_type_t type"
.Sh DESCRIPTION
The
.Fn BUS_GET_PROPERTY
method
-is called from driver code which wants to access child's specific data stored
+is called from driver code which wants to access a child's specific data stored
on the bus.
-Property consits of its name and value.
+A property has a name and an associated value.
Implementation shall copy to
.Fa propvalue
at most
.Fa size
bytes.
+.Pp
+.Fn BUS_GET_PROPERTY
+supports different property types specified via the
+.Fa type
+argument.
+The
+.Fa size
+is guaranteed to be a multiple of the underlying property type.
+If a type is not supported,
+.Fn BUS_GET_PROPERTY
+shall return -1.
.Sh NOTES
If
.Fa propvalue
is NULL or
.Fa size
-is zero, then implementation shall only return size of the property.
+is zero, the implementation shall return only the size of the property.
.Sh RETURN VALUES
-Property's size if successful, otherwise -1.
+The property size if successful, otherwise -1.
.Sh SEE ALSO
.Xr device 9 ,
.Xr device_get_property 9
diff --git a/share/man/man9/device_get_property.9 b/share/man/man9/device_get_property.9
index 1d9bd5058b1a..d925f5f224db 100644
--- a/share/man/man9/device_get_property.9
+++ b/share/man/man9/device_get_property.9
@@ -25,7 +25,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd August 19, 2021
+.Dd February 18, 2022
.Dt DEVICE_GET_PROPERTY 9
.Os
.Sh NAME
@@ -36,13 +36,35 @@
.In sys/param.h
.In sys/bus.h
.Ft ssize_t
-.Fn device_get_property "device_t dev" "const char *prop" "void *val" "size_t sz"
+.Fn device_get_property "device_t dev" "const char *prop" "void *val" "size_t sz" \
+ "device_property_type_t type"
.Ft bool
.Fn device_has_property "device_t dev" "const char *prop"
.Sh DESCRIPTION
Access device specific data provided by the parent bus.
Drivers can use these properties to obtain device capabilities and set
necessary quirks.
+.Pp
+The underlying property type is specified with the
+.Fa type
+argument.
+Currently the following types are supported:
+.Bl -tag -width ".Dv DEVICE_PROP_BUFFER"
+.It Dv DEVICE_PROP_BUFFER
+The underlying property is a string of bytes.
+.It Dv DEVICE_PROP_ANY
+Wildcard property type.
+.It Dv DEVICE_PROP_UINT32
+The underlying property is an array of unsigned 32 bit integers.
+The
+.Fa sz
+argument shall be a multiple of 4.
+.It Dv DEVICE_PROP_UINT64
+The underlying property is an array of unsigned 64 bit integers.
+The
+.Fa sz
+argument shall be a multiple of 8.
+.El
.Sh NOTES
You can pass NULL as pointer to property's value when calling
.Fn device_get_property
diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 87212a029ea5..831e618d430f 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -145,7 +145,7 @@ static void acpi_delete_resource(device_t bus, device_t child, int type,
static uint32_t acpi_isa_get_logicalid(device_t dev);
static int acpi_isa_get_compatid(device_t dev, uint32_t *cids, int count);
static ssize_t acpi_bus_get_prop(device_t bus, device_t child, const char *propname,
- void *propvalue, size_t size);
+ void *propvalue, size_t size, device_property_type_t type);
static int acpi_device_id_probe(device_t bus, device_t dev, char **ids, char **match);
static ACPI_STATUS acpi_device_eval_obj(device_t bus, device_t dev,
ACPI_STRING pathname, ACPI_OBJECT_LIST *parameters,
@@ -1928,7 +1928,7 @@ acpi_find_dsd(device_t bus, device_t dev)
static ssize_t
acpi_bus_get_prop(device_t bus, device_t child, const char *propname,
- void *propvalue, size_t size)
+ void *propvalue, size_t size, device_property_type_t type)
{
ACPI_STATUS status;
const ACPI_OBJECT *obj;
@@ -1938,13 +1938,32 @@ acpi_bus_get_prop(device_t bus, device_t child, const char *propname,
if (ACPI_FAILURE(status))
return (-1);
+ switch (type) {
+ case DEVICE_PROP_ANY:
+ case DEVICE_PROP_BUFFER:
+ case DEVICE_PROP_UINT32:
+ case DEVICE_PROP_UINT64:
+ break;
+ default:
+ return (-1);
+ }
+
switch (obj->Type) {
case ACPI_TYPE_INTEGER:
+ if (type == DEVICE_PROP_UINT32) {
+ if (propvalue != NULL && size >= sizeof(uint32_t))
+ *((uint32_t *)propvalue) = obj->Integer.Value;
+ return (sizeof(uint32_t));
+ }
if (propvalue != NULL && size >= sizeof(uint64_t))
*((uint64_t *) propvalue) = obj->Integer.Value;
return (sizeof(uint64_t));
case ACPI_TYPE_STRING:
+ if (type != DEVICE_PROP_ANY &&
+ type != DEVICE_PROP_BUFFER)
+ return (-1);
+
if (propvalue != NULL && size > 0)
memcpy(propvalue, obj->String.Pointer,
MIN(size, obj->String.Length));
diff --git a/sys/dev/fdt/simplebus.c b/sys/dev/fdt/simplebus.c
index 4d58f9b3ea69..b5a10f83055c 100644
--- a/sys/dev/fdt/simplebus.c
+++ b/sys/dev/fdt/simplebus.c
@@ -56,7 +56,8 @@ static struct resource_list *simplebus_get_resource_list(device_t bus,
device_t child);
static ssize_t simplebus_get_property(device_t bus, device_t child,
- const char *propname, void *propvalue, size_t size);
+ const char *propname, void *propvalue, size_t size,
+ device_property_type_t type);
/*
* ofw_bus interface
*/
@@ -356,14 +357,56 @@ simplebus_get_resource_list(device_t bus __unused, device_t child)
static ssize_t
simplebus_get_property(device_t bus, device_t child, const char *propname,
- void *propvalue, size_t size)
+ void *propvalue, size_t size, device_property_type_t type)
{
phandle_t node = ofw_bus_get_node(child);
+ ssize_t ret, i;
+ uint32_t *buffer;
+ uint64_t val;
+
+ switch (type) {
+ case DEVICE_PROP_ANY:
+ case DEVICE_PROP_BUFFER:
+ case DEVICE_PROP_UINT32:
+ case DEVICE_PROP_UINT64:
+ break;
+ default:
+ return (-1);
+ }
if (propvalue == NULL || size == 0)
return (OF_getproplen(node, propname));
- return (OF_getencprop(node, propname, propvalue, size));
+ /*
+ * Integer values are stored in BE format.
+ * If caller declared that the underlying property type is uint32_t
+ * we need to do the conversion to match host endianness.
+ */
+ if (type == DEVICE_PROP_UINT32)
+ return (OF_getencprop(node, propname, propvalue, size));
+
+ /*
+ * uint64_t also requires endianness handling.
+ * In FDT every 8 byte value is stored using two uint32_t variables
+ * in BE format. Now, since the upper bits are stored as the first
+ * of the pair, both halves require swapping.
+ */
+ if (type == DEVICE_PROP_UINT64) {
+ ret = OF_getencprop(node, propname, propvalue, size);
+ if (ret <= 0) {
+ return (ret);
+ }
+
+ buffer = (uint32_t *)propvalue;
+
+ for (i = 0; i < size / 4; i += 2) {
+ val = (uint64_t)buffer[i] << 32 | buffer[i + 1];
+ ((uint64_t *)buffer)[i / 2] = val;
+ }
+ return (ret);
+ }
+
+ return (OF_getprop(node, propname, propvalue, size));
}
static struct resource *
diff --git a/sys/dev/mii/dp83867phy.c b/sys/dev/mii/dp83867phy.c
index 478be10b7f9f..297ac0f83ae1 100644
--- a/sys/dev/mii/dp83867phy.c
+++ b/sys/dev/mii/dp83867phy.c
@@ -145,7 +145,8 @@ dp_attach(device_t dev)
sc = device_get_softc(dev);
mii_sc = &sc->mii_sc;
- size = device_get_property(dev, "max-speed", &maxspeed, sizeof(maxspeed));
+ size = device_get_property(dev, "max-speed", &maxspeed,
+ sizeof(maxspeed), DEVICE_PROP_UINT32);
if (size <= 0)
maxspeed = 0;
diff --git a/sys/dev/mmc/mmc_helpers.c b/sys/dev/mmc/mmc_helpers.c
index 1a90f291fc47..db3337b24534 100644
--- a/sys/dev/mmc/mmc_helpers.c
+++ b/sys/dev/mmc/mmc_helpers.c
@@ -89,10 +89,11 @@ mmc_parse_mmc_speed(device_t dev, struct mmc_host *host)
int
mmc_parse(device_t dev, struct mmc_helper *helper, struct mmc_host *host)
{
- uint64_t bus_width, max_freq;
+ uint32_t bus_width, max_freq;
bus_width = 0;
- if (device_get_property(dev, "bus-width", &bus_width, sizeof(uint64_t)) <= 0)
+ if (device_get_property(dev, "bus-width", &bus_width,
+ sizeof(bus_width), DEVICE_PROP_UINT32) <= 0)
bus_width = 1;
if (bus_width >= 4)
@@ -106,7 +107,7 @@ mmc_parse(device_t dev, struct mmc_helper *helper, struct mmc_host *host)
* operates on
*/
if (device_get_property(dev, "max-frequency", &max_freq,
- sizeof(uint64_t)) > 0)
+ sizeof(max_freq), DEVICE_PROP_UINT32) > 0)
host->f_max = max_freq;
if (device_has_property(dev, "broken-cd"))
diff --git a/sys/dev/sdhci/sdhci_xenon.c b/sys/dev/sdhci/sdhci_xenon.c
index 6dc0974c4e4e..59669affe4c5 100644
--- a/sys/dev/sdhci/sdhci_xenon.c
+++ b/sys/dev/sdhci/sdhci_xenon.c
@@ -471,20 +471,21 @@ static void
sdhci_xenon_parse_prop(device_t dev)
{
struct sdhci_xenon_softc *sc;
- uint64_t val;
+ uint32_t val;
sc = device_get_softc(dev);
val = 0;
- if (device_get_property(dev, "quirks", &val, sizeof(val)) > 0)
+ if (device_get_property(dev, "quirks",
+ &val, sizeof(val), DEVICE_PROP_UINT32) > 0)
sc->slot->quirks = val;
sc->znr = XENON_ZNR_DEF_VALUE;
if (device_get_property(dev, "marvell,xenon-phy-znr",
- &val, sizeof(val)) > 0)
+ &val, sizeof(val), DEVICE_PROP_UINT32) > 0)
sc->znr = val & XENON_ZNR_MASK;
sc->zpr = XENON_ZPR_DEF_VALUE;
if (device_get_property(dev, "marvell,xenon-phy-zpr",
- &val, sizeof(val)) > 0)
+ &val, sizeof(val), DEVICE_PROP_UINT32) > 0)
sc->zpr = val & XENON_ZPR_MASK;
if (device_has_property(dev, "marvell,xenon-phy-slow-mode"))
sc->slow_mode = true;
diff --git a/sys/kern/bus_if.m b/sys/kern/bus_if.m
index cf6470c220ef..1e4102e52d29 100644
--- a/sys/kern/bus_if.m
+++ b/sys/kern/bus_if.m
@@ -938,6 +938,7 @@ METHOD ssize_t get_property {
const char *_propname;
void *_propvalue;
size_t _size;
+ device_property_type_t type;
} DEFAULT bus_generic_get_property;
/**
diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 8a05c28c96b1..416353566306 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -2717,17 +2717,34 @@ device_verbose(device_t dev)
}
ssize_t
-device_get_property(device_t dev, const char *prop, void *val, size_t sz)
+device_get_property(device_t dev, const char *prop, void *val, size_t sz,
+ device_property_type_t type)
{
device_t bus = device_get_parent(dev);
- return (BUS_GET_PROPERTY(bus, dev, prop, val, sz));
+ switch (type) {
+ case DEVICE_PROP_ANY:
+ case DEVICE_PROP_BUFFER:
+ break;
+ case DEVICE_PROP_UINT32:
+ if (sz % 4 != 0)
+ return (-1);
+ break;
+ case DEVICE_PROP_UINT64:
+ if (sz % 8 != 0)
+ return (-1);
+ break;
+ default:
+ return (-1);
+ }
+
+ return (BUS_GET_PROPERTY(bus, dev, prop, val, sz, type));
}
bool
device_has_property(device_t dev, const char *prop)
{
- return (device_get_property(dev, prop, NULL, 0) >= 0);
+ return (device_get_property(dev, prop, NULL, 0, DEVICE_PROP_ANY) >= 0);
}
/**
@@ -4133,11 +4150,11 @@ bus_generic_write_ivar(device_t dev, device_t child, int index,
*/
ssize_t
bus_generic_get_property(device_t dev, device_t child, const char *propname,
- void *propvalue, size_t size)
+ void *propvalue, size_t size, device_property_type_t type)
{
if (device_get_parent(dev) != NULL)
return (BUS_GET_PROPERTY(device_get_parent(dev), child,
- propname, propvalue, size));
+ propname, propvalue, size, type));
return (-1);
}
diff --git a/sys/sys/bus.h b/sys/sys/bus.h
index 0b4f85a7fd0d..8ed748b72bab 100644
--- a/sys/sys/bus.h
+++ b/sys/sys/bus.h
@@ -60,6 +60,20 @@ typedef enum device_state {
DS_ATTACHED = 30, /**< @brief attach method called */
} device_state_t;
+/**
+ * @brief Device proprty types.
+ *
+ * Those are used by bus logic to encode requested properties,
+ * e.g. in DT all properties are stored as BE and need to be converted
+ * to host endianness.
+ */
+typedef enum device_property_type {
+ DEVICE_PROP_ANY = 0,
+ DEVICE_PROP_BUFFER = 1,
+ DEVICE_PROP_UINT32 = 2,
+ DEVICE_PROP_UINT64 = 3,
+} device_property_type_t;
+
/**
* @brief Device information exported to userspace.
* The strings are placed one after the other, separated by NUL characters.
@@ -444,9 +458,9 @@ bus_dma_tag_t
bus_space_tag_t
bus_generic_get_bus_tag(device_t dev, device_t child);
int bus_generic_get_domain(device_t dev, device_t child, int *domain);
-ssize_t bus_generic_get_property(device_t dev, device_t child,
+ssize_t bus_generic_get_property(device_t dev, device_t child,
const char *propname, void *propvalue,
- size_t size);
+ size_t size, device_property_type_t type);
struct resource_list *
bus_generic_get_resource_list(device_t, device_t);
int bus_generic_map_resource(device_t dev, device_t child, int type,
@@ -637,7 +651,8 @@ int device_set_unit(device_t dev, int unit); /* XXX DONT USE XXX */
int device_shutdown(device_t dev);
void device_unbusy(device_t dev);
void device_verbose(device_t dev);
-ssize_t device_get_property(device_t dev, const char *prop, void *val, size_t sz);
+ssize_t device_get_property(device_t dev, const char *prop, void *val,
+ size_t sz, device_property_type_t type);
bool device_has_property(device_t dev, const char *prop);
/*