On Sat, Dec 18, 2021 at 11:58 AM Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote: > > Semtech SAR sensor SX9324 is an evolution of the SX9310: > It has 4 phases that can be configure to capture and process data > from any of 3 CS pins and provide independent detection: > proximity, table proximity or body proximity. > > Gather antenna data: > echo sx9324-dev3 > trigger/current_trigger > echo 1 > scan_elements/in_proximity0_en > echo 1 > buffer/enable > od -v -An --endian=big -t d2 -w2 /dev/iio\:device3 Can you shift this right by two spaces? > (at 10Hz, the default). > > Trigger events: > Setting: > thresh_falling_period: 2 (events) > thresh_rising_period: 2 (events) > in_proximity0_thresh_either_value: 300 > in_proximity0_thresh_either_hysteresis: 72 Ditto. > using iio_event_monitor /dev/iio\:deviceX, approaching my hand to the > antenna pad, I see: > ... > Event: time: 1634763907532035297, type: proximity, channel: 0, evtype: > thresh, direction: falling > Event: time: 1634763910138104640, type: proximity, channel: 0, evtype: > thresh, direction: rising ... > + * Based on SX9324 driver and copy of datasheet at: > + * https://edit.wpgdadawant.com/uploads/news_file/program/2019/30184/tech_files/program_30184_suggest_other_file.pdf Can you provide the link to the datasheet as Datasheet: tag? ... > + for (i = 0; i < SX9324_NUM_PINS; i++) { > + pin_idx = (val & SX9324_REG_AFE_PH0_PIN_MASK(i)) >> (2 * i); > + len += scnprintf(buf + len, PAGE_SIZE - len, "%s,", Shouldn't this be sysfs_emit_at()? > + sx9324_cs_pin_usage[pin_idx]); > + } ... > + *val = 1 << FIELD_GET(SX9324_REG_PROX_CTRL0_GAIN_MASK, regval); Hmm... perhaps BIT()? ... > + switch (mask) { > + case IIO_CHAN_INFO_HARDWAREGAIN: > + *type = IIO_VAL_INT; > + *length = ARRAY_SIZE(sx9324_gain_vals); > + *vals = sx9324_gain_vals; > + return IIO_AVAIL_LIST; > + case IIO_CHAN_INFO_SAMP_FREQ: > + *type = IIO_VAL_INT_PLUS_MICRO; > + *length = ARRAY_SIZE(sx9324_samp_freq_table) * 2; > + *vals = (int *)sx9324_samp_freq_table; > + return IIO_AVAIL_LIST; > + } > + > + return -EINVAL; Make it the default case? ... > + regval = FIELD_GET(SX9324_REG_PROX_CTRL5_FAR_DEBOUNCE_MASK, regval); > + if (regval) > + *val = 1 << regval; BIT() ? > + else > + *val = 0; ... > + regval = FIELD_GET(SX9324_REG_PROX_CTRL5_CLOSE_DEBOUNCE_MASK, regval); > + if (regval) > + *val = 1 << regval; Ditto? > + else > + *val = 0; ... > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + return sx9324_set_samp_freq(data, val, val2); > + case IIO_CHAN_INFO_HARDWAREGAIN: > + return sx9324_write_gain(data, chan, val); > + } > + > + return -EINVAL; In some cases you have default in some not, can it be consistent? ... > + dev_err(&data->client->dev, > + "initial compensation timed out: 0x%02x\n", > + val); Isn't it what user space will get anyway? ... > +static const struct acpi_device_id sx9324_acpi_match[] = { > + { "STH9324", SX9324_WHOAMI_VALUE}, Missed whitespace? > + { } > +}; ... > +static const struct of_device_id sx9324_of_match[] = { > + { .compatible = "semtech,sx9324", (void *)SX9324_WHOAMI_VALUE}, Ditto. > + { } > +}; -- With Best Regards, Andy Shevchenko
