Page MenuHomeFreeBSD
Differential D33457 Diff 100979 sys/sys/bus.h

Changeset View

Standalone View

View Options

sys/sys/bus.h

Show First 20 Lines • Show All 55 Lines • ▼ Show 20 Lines
typedef enum device_state { typedef enum device_state {
DS_NOTPRESENT = 10, /**< @brief not probed or probe failed */ DS_NOTPRESENT = 10, /**< @brief not probed or probe failed */
DS_ALIVE = 20, /**< @brief probe succeeded */ DS_ALIVE = 20, /**< @brief probe succeeded */
DS_ATTACHING = 25, /**< @brief currently attaching */ DS_ATTACHING = 25, /**< @brief currently attaching */
DS_ATTACHED = 30, /**< @brief attach method called */ DS_ATTACHED = 30, /**< @brief attach method called */
} device_state_t; } 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,
andrewUnsubmitted
Not Done
Inline Actions

Is the integer data always 32-bit? I think it should be named DEVICE_PROP_INT as it may be used to read a 64-bit integer used as a physical address.

andrew: Is the integer data always 32-bit? I think it should be named `DEVICE_PROP_INT` as it may be…
kdAuthorUnsubmitted
Done
Inline Actions

Yes, my intention here is to put emphasis on the 32bit size of underlying property.
On OFW based systems DEVICE_PROP_UINT32 triggers usage of be32toh, which assumes that the underlying property is a uint32_t array.

ACPI stores integers(ACPI_TYPE_INTEGER) as uint64_t.
Consider the following scenario: (without this patch applied)

uint32_t temp;
(...)
device_get_property(..., &temp, sizeof(temp));

This will work just fine with ofw, but will fail on ACPI systems as the buffer is not big enough to store the property.
As a workaround in this case I'm doing an explicit conversion to uint32_t.
Now since the caller specified DEVICE_PROP_UINT32 I believe that the cast to uint32_t should not be a surprise.

I've also thought about adding some helpers, e.g

ssize_t device_get_property_u32(device_t dev, const char *prop, uint32_t *val)

Now I'm not sure if that won't pollute the API too much.
Any thoughts?

kd: Yes, my intention here is to put emphasis on the 32bit size of underlying property. On OFW…
mwUnsubmitted
Not Done
Inline Actions

Yes, my intention here is to put emphasis on the 32bit size of underlying property.
On OFW based systems DEVICE_PROP_UINT32 triggers usage of be32toh, which assumes that the underlying property is a uint32_t array.

ACPI stores integers(ACPI_TYPE_INTEGER) as uint64_t.
Consider the following scenario: (without this patch applied)

uint32_t temp;
(...)
device_get_property(..., &temp, sizeof(temp));

This will work just fine with ofw, but will fail on ACPI systems as the buffer is not big enough to store the property.
As a workaround in this case I'm doing an explicit conversion to uint32_t.
Now since the caller specified DEVICE_PROP_UINT32 I believe that the cast to uint32_t should not be a surprise.

How about keeping DEVICE_PROP_UINT32 for explicit casting and adding DEVICE_PROP_INT for the cases where the maxium supported type will be returned (ofw - 32-bit, acpi - 64-bit)? The binding is somewhat an ABI and it's user's responsibility to parse this with caution.

I've also thought about adding some helpers, e.g

ssize_t device_get_property_u32(device_t dev, const char *prop, uint32_t *val)

Now I'm not sure if that won't pollute the API too much.
Any thoughts?

Type passed as parameter works better IMO. Linux has a callback-per-type (overall > 10) and I do not fancy following this direction.

mw: > Yes, my intention here is to put emphasis on the 32bit size of underlying property. > On OFW…
mwUnsubmitted
Not Done
Inline Actions

Hi @andrew Do you have any thoughts after our previous considerations?

mw: Hi @andrew Do you have any thoughts after our previous considerations?
DEVICE_PROP_UINT64 = 3,
} device_property_type_t;
/**
* @brief Device information exported to userspace. * @brief Device information exported to userspace.
* The strings are placed one after the other, separated by NUL characters. * The strings are placed one after the other, separated by NUL characters.
* Fields should be added after the last one and order maintained for compatibility * Fields should be added after the last one and order maintained for compatibility
*/ */
#define BUS_USER_BUFFER (3*1024) #define BUS_USER_BUFFER (3*1024)
struct u_device { struct u_device {
uintptr_t dv_handle; uintptr_t dv_handle;
uintptr_t dv_parent; uintptr_t dv_parent;
▲ Show 20 Lines • Show All 554 Lines • ▼ Show 20 Lines
void device_set_flags(device_t dev, u_int32_t flags); void device_set_flags(device_t dev, u_int32_t flags);
void device_set_softc(device_t dev, void *softc); void device_set_softc(device_t dev, void *softc);
void device_free_softc(void *softc); void device_free_softc(void *softc);
void device_claim_softc(device_t dev); void device_claim_softc(device_t dev);
int device_set_unit(device_t dev, int unit); /* XXX DONT USE XXX */ int device_set_unit(device_t dev, int unit); /* XXX DONT USE XXX */
int device_shutdown(device_t dev); int device_shutdown(device_t dev);
void device_unbusy(device_t dev); void device_unbusy(device_t dev);
void device_verbose(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); bool device_has_property(device_t dev, const char *prop);
/* /*
* Access functions for devclass. * Access functions for devclass.
*/ */
int devclass_add_driver(devclass_t dc, driver_t *driver, int devclass_add_driver(devclass_t dc, driver_t *driver,
int pass, devclass_t *dcp); int pass, devclass_t *dcp);
devclass_t devclass_create(const char *classname); devclass_t devclass_create(const char *classname);
▲ Show 20 Lines • Show All 349 Lines • Show Last 20 Lines