On Mon, Mar 06, 2023 at 11:15:10AM +0200, Matti Vaittinen wrote: > Support ROHM BU27034 ALS sensor > > This series adds support for ROHM BU27034 Ambient Light Sensor. > > The BU27034 has configurable gain and measurement (integration) time > settings. Both of these have inversely proportional relation to the > sensor's intensity channel scale. > > Many users only set the scale, which means that many drivers attempt to > 'guess' the best gain+time combination to meet the scale. Usually this > is the biggest integration time which allows setting the requested > scale. Typically, increasing the integration time has better accuracy > than increasing the gain, which often amplifies the noise as well as the > real signal. > > However, there may be cases where more responsive sensors are needed. > So, in some cases the longest integration times may not be what the user > prefers. The driver has no way of knowing this. > > Hence, the approach taken by this series is to allow user to set both > the scale and the integration time with following logic: > > 1. When scale is set, the existing integration time is tried to be > maintained as a first priority. > 1a) If the requested scale can't be met by current time, then also > other time + gain combinations are searched. If scale can be met > by some other integration time, then the new time may be applied. > If the time setting is common for all channels, then also other > channels must be able to maintain their scale with this new time > (by changing their gain). The new times are scanned in the order > of preference (typically the longest times first). > 1b) If the requested scale can be met using current time, then only > the gain for the channel is changed. > > 2. When the integration time change - scale is tried to be maintained. > When integration time change is requested also gain for all impacted > channels is adjusted so that the scale is not changed, or is chaned > as little as possible. This is different from the RFCv1 where the > request was rejected if suitable gain couldn't be found for some > channel(s). > > This logic is simple. When total gain (either caused by time or hw-gain) > is doubled, the scale gets halved. Also, the supported times are given a > 'multiplier' value which tells how much they increase the total gain. > > However, when I wrote this logic in bu27034 driver, I made quite a few > errors on the way - and driver got pretty big. As I am writing drivers > for two other sensors (RGB C/IR + flicker BU27010 and RGB C/IR BU27008) > with similar gain-time-scale logic I thought that adding common helpers > for these computations might be wise. I hope this way all the bugs will > be concentrated in one place and not in every individual driver ;) > > Hence, this series also intriduces IIO gain-time-scale helpers > (abbreviated as gts-helpers) + a couple of KUnit tests for the most > hairy parts. > > I can't help thinking that there should've been simpler way of computing > the gain-time-scale conversions. Also, pretty good speed improvements > might be available if some of the do_div()s could be replaced by >>. > This, however, is not a priority for my light-sensor use-case where > speed demands are not that big. > > Finally, these added helpers do provide some value also for drivers > which only: > a) allow gain change > or > b) allow changing both the time and gain but so that the time-change is > not reflected in register values. > > For a) we provide the gain - selector (register value) table format + > selector to gain look-ups, gain <-> scale conversions and the available > scales helpers. > > For latter case we also provide the time-tables, and actually all the > APIs should be usable by setting the time multiplier to 1. (not testeted > thoroughly though). A few comments can still be applied here. Can you comment on the discussion against the previous version? -- With Best Regards, Andy Shevchenko
