On Apr 1, 2015, at 9:23 AM, David Higgs <[email protected]> wrote:
> On Wed, Apr 1, 2015 at 7:52 AM, Martin Pieuchot <[email protected]> wrote:
> On 31/03/15(Tue) 23:06, David Higgs wrote:
> > This was much more straightforward than expected.
> >
> > - Replace an array with a LIST of allocated sensors.
> > - Remove or rescope variables counting sensors.
> > - Allocated sensors are always attached.
> > - Drop an unnecessary size calculation.
>
> Do you need this for an upcoming change? I fail to see the benefit to
> have a malloc(9) call per item, afaik theses items are really small and
> you have at most 10 of them...
>
> No, I just extrapolated too much from your suggestion to put attached sensors
> on a LIST. I'll adjust and resubmit.
>
Updated diff:
- Create a LIST to hold attached sensors
- Remove or rescope variables counting sensors.
- Drop an unnecessary size calculation.
--- a/upd.c
+++ b/upd.c
@@ -23,6 +23,7 @@
#include <sys/kernel.h>
#include <sys/malloc.h>
#include <sys/device.h>
+#include <sys/queue.h>
#include <sys/sensors.h>
#include <dev/usb/hid.h>
@@ -77,20 +78,19 @@ struct upd_report {
struct upd_sensor {
struct ksensor ksensor;
struct hid_item hitem;
- int attached;
+ LIST_ENTRY(upd_sensor) next;
};
struct upd_softc {
struct uhidev sc_hdev;
- int sc_num_sensors;
u_int sc_max_repid;
- u_int sc_max_sensors;
/* sensor framework */
struct ksensordev sc_sensordev;
struct sensor_task *sc_sensortask;
struct upd_report *sc_reports;
struct upd_sensor *sc_sensors;
+ LIST_HEAD(, upd_sensor) sc_sensorlist;
};
int upd_match(struct device *, void *, void *);
@@ -156,13 +156,14 @@ upd_attach(struct device *parent, struct
struct upd_usage_entry *entry;
struct upd_sensor *sensor;
int size;
+ int num_sensors;
void *desc;
sc->sc_hdev.sc_intr = upd_intr;
sc->sc_hdev.sc_parent = uha->parent;
sc->sc_reports = NULL;
sc->sc_sensors = NULL;
- sc->sc_max_sensors = nitems(upd_usage_table);
+ LIST_INIT(&sc->sc_sensorlist);
strlcpy(sc->sc_sensordev.xname, sc->sc_hdev.sc_dev.dv_xname,
sizeof(sc->sc_sensordev.xname));
@@ -173,14 +174,13 @@ upd_attach(struct device *parent, struct
sc->sc_reports = mallocarray(sc->sc_max_repid,
sizeof(struct upd_report), M_USBDEV, M_WAITOK | M_ZERO);
- sc->sc_sensors = mallocarray(sc->sc_max_sensors,
+ sc->sc_sensors = mallocarray(nitems(upd_usage_table),
sizeof(struct upd_sensor), M_USBDEV, M_WAITOK | M_ZERO);
- size = sc->sc_max_sensors * sizeof(struct upd_sensor);
- sc->sc_num_sensors = 0;
+ num_sensors = 0;
uhidev_get_report_desc(uha->parent, &desc, &size);
for (hdata = hid_start_parse(desc, size, hid_feature);
hid_get_item(hdata, &item) &&
- sc->sc_num_sensors < sc->sc_max_sensors; ) {
+ num_sensors < nitems(upd_usage_table); ) {
DPRINTF(("upd: repid=%d\n", item.report_ID));
if (item.kind != hid_feature ||
item.report_ID < 0 ||
@@ -196,7 +196,7 @@ upd_attach(struct device *parent, struct
if (sensor != NULL)
continue;
- sensor = &sc->sc_sensors[sc->sc_num_sensors];
+ sensor = &sc->sc_sensors[num_sensors];
memcpy(&sensor->hitem, &item, sizeof(struct hid_item));
strlcpy(sensor->ksensor.desc, entry->usage_name,
sizeof(sensor->ksensor.desc));
@@ -205,8 +205,8 @@ upd_attach(struct device *parent, struct
sensor->ksensor.status = SENSOR_S_UNKNOWN;
sensor->ksensor.value = 0;
sensor_attach(&sc->sc_sensordev, &sensor->ksensor);
- sensor->attached = 1;
- sc->sc_num_sensors++;
+ LIST_INSERT_HEAD(&sc->sc_sensorlist, sensor, next);
+ num_sensors++;
if (sc->sc_reports[item.report_ID].enabled)
continue;
@@ -216,7 +216,7 @@ upd_attach(struct device *parent, struct
sc->sc_reports[item.report_ID].enabled = 1;
}
hid_end_parse(hdata);
- DPRINTF(("upd: sc_num_sensors=%d\n", sc->sc_num_sensors));
+ DPRINTF(("upd: num_sensors=%d\n", num_sensors));
sc->sc_sensortask = sensor_task_register(sc, upd_refresh, 6);
if (sc->sc_sensortask == NULL) {
@@ -235,7 +235,6 @@ upd_detach(struct device *self, int flag
{
struct upd_softc *sc = (struct upd_softc *)self;
struct upd_sensor *sensor;
- int i;
if (sc->sc_sensortask != NULL) {
wakeup(&sc->sc_sensortask);
@@ -244,10 +243,8 @@ upd_detach(struct device *self, int flag
sensordev_deinstall(&sc->sc_sensordev);
- for (i = 0; i < sc->sc_num_sensors; i++) {
- sensor = &sc->sc_sensors[i];
- if (sensor->attached)
- sensor_detach(&sc->sc_sensordev, &sensor->ksensor);
+ LIST_FOREACH(sensor, &sc->sc_sensorlist, next) {
+ sensor_detach(&sc->sc_sensordev, &sensor->ksensor);
DPRINTF(("upd_detach: %s\n", sensor->ksensor.desc));
}
@@ -306,10 +303,8 @@ struct upd_sensor *
upd_lookup_sensor(struct upd_softc *sc, int page, int usage)
{
struct upd_sensor *sensor = NULL;
- int i;
- for (i = 0; i < sc->sc_num_sensors; i++) {
- sensor = &sc->sc_sensors[i];
+ LIST_FOREACH(sensor, &sc->sc_sensorlist, next) {
if (page == HID_GET_USAGE_PAGE(sensor->hitem.usage) &&
usage == HID_GET_USAGE(sensor->hitem.usage))
return (sensor);
@@ -324,14 +319,12 @@ upd_update_sensors(struct upd_softc *sc,
struct upd_sensor *sensor;
ulong hdata, batpres;
ulong adjust;
- int i;
sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT);
batpres = sensor ? sensor->ksensor.value : -1;
- for (i = 0; i < sc->sc_num_sensors; i++) {
- sensor = &sc->sc_sensors[i];
- if (!(sensor->hitem.report_ID == repid && sensor->attached))
+ LIST_FOREACH(sensor, &sc->sc_sensorlist, next) {
+ if (sensor->hitem.report_ID != repid)
continue;
/* invalidate battery dependent sensors */
