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

@@ -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

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

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));

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 *

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;
 

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"))

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;

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;
 
 /**

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);
 }

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);
 
 /*