On Mon, 11 Jun 2018 16:57:31 +0200 Mathieu Othacehe <m.othacehe@xxxxxxxxx> wrote: > Hi Jonathan, > > I'll send a v3 as a separate patch, here are some answers to your > comments. > > > I'm not totally clear what this is. From a really quick look at the > > docs, I think we are looking at in phase and quadrature elements > > of the amplitude. As the amplitude is basically a light intensity > > measurement > > That's my understanding too, I'll rephrase. > > > > > in_intensity0_q_raw > > in_intensity0_i_raw > > in_intensity0_scale if it is possible to relate this anything real > > I think in reality it does have a unit, we just have no visibility > > of what that is. > > Ok. > > > > > We also have a phase measurement, but naturally only one of htem > > in_phase0_raw > > in_phase0_scale > > Does that mean adding a new iio_chan_type "IIO_PHASE"? Yes, it rather looks like we need it. > > > Hmm. Thinking more on this, we could treat this as a separate channel. > > Given it's light intensity it would be an intensity channel. > > It's in some sense the combination of the split quadrature elements > > above > > in_intensity0_raw > > in_intensity0_scale. Note for these that the scale is assumed by > > most userspace code to be fixed, so you'll need to roll the exponent > > part into the _raw value not the _scale.# > > Seems fine! > > > in_proximity0_calib_cs_i -> in_intensity0_i_calibbias > > in_proximity0_calib_cs_q -> in_intensity0_q_calibbias > > in_proximity0_calib_cs_gain =-> in_intensity0_calibscale (I think the cs will effect intensity?) > > Ok. > > > + The gain read from in_proximity0_hardwaregain shall > > + be written into in_proximity0_calib_cs_gain when > > + calibrating crosstalk. > > > > I'm not following this one, hardwaregain is usually a write > > parameter. So should be under control of the userspace. > > The sensor has an "Automatic Gain Control" (AGC) which sets the analog > signal levels at an optimum level by controlling programmable gain > amplifiers according to the datasheet. > > The AGC value in an output of the sensor at it is supposed to be > saved when calibrating crosstalk in "Crosstalk Gain" registers. > > Would it be ok to use hardwaregain as a read-only register for this > purpose? I'm not really keen on doing that (as hardware gain has a well defined different meaning). This is a rather opaque device specific value. > > > > > + > > + As the crosstalk is dependant on the emitter current, > > + the amplitude read from in_proximity0_amplitude shall > > + be written into in_proximity0_calib_ampl_ref when > > + calibrating crosstalk. > > > > This last one is trickier from the description. Do we know how it > > is applied by the hardware? Is it a simple offset? > > > > Looking at the document an1724.pdf this doesn't seem to be something > > that it necessarily makes any sense to expose to userspace. It is > > a calibration that has no external inputs - just a sequence of > > internal actions. Hence I would just do this on power up. > > No I don't know how it is applied. However, it can't be done on power up > as it requires the emitter to be blocked from reaching the photodiode. This is interesting as it's specifically documented as requiring no external actions. Oh well, another clear datasheet. > > I guess the principle here is to measure the amplitude of the received > signal while the emitter is blocked so that it can be substracted to the > further measures. Would it be ok to use in_intensity0_calibbias for it? For the control parameter yes if it meets the ABI description, for the value during calibration no. Calibbias is a control parameter not an output. > > > +What: /sys/bus/iio/devices/iio:deviceX/in_proximity0_calib_distance_ref > > +What: /sys/bus/iio/devices/iio:deviceX/in_proximity0_calib_temp_ref > > > > Hmm. So these last two are C in the equation. > > Seems so (the sensor is computing the C from those two). > > > + Finally, the c constant is set by the sensor in > > + function of in_proximity0_calib_temp_ref and > > + in_proximity0_calib_distance_ref values. > > + > > + To fill in_proximity0_calib_distance_ref, a distance > > + measurement at a known distance has to be made. The > > + result of the substraction between the known distance > > + and the measured value shall be stored in > > + in_proximity0_calib_distance_ref. The sensor > > + temperature at the time of this measurement read shall > > + be stored in in_proximity0_calib_temp_ref. > > + > > + Get those values from hardware and show them when read > > + from. > > > > I'm slightly less bothered by getting these perfect as they are very > > chip specific and generic userspace code is unlikely to play with them. > > We may want to revisit these in the future... > > > > If we are using phase and intensity channels as suggested above, > > these will want adjusting to take that into account. > > > Those calib fields would become: > > in_proximity0_calib_temp_ref -> in_temp0_calibbias > in_proximity0_calib_distance_ref -> in_proximity0_calibbias These are kind of fine, as they are the linear offsets. Doesn't really match well with the quadratic term though. > > It would also require a new channel "in_intensity1_raw" for the ambient > light output. Is it ok? That one is fine. > > > Treat the emitter as an output current channel and all this becomes > > simpler. > > Ok. > > Thanks for your patience, You are welcome, this is a fiddly device! Sane hardware would store all these in on chip flash, but I guess it's a cost thing to not do so. > > Mathieu Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
