On Fri, Apr 2, 2021 at 2:36 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Thu, 01 Apr 2021 17:39:05 -0700
> Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> > Quoting Jonathan Cameron (2021-04-01 06:19:35)
> > > On Tue, 30 Mar 2021 17:38:03 -0700
> > > Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote:
> > >
> > > > On Sun, Mar 28, 2021 at 8:12 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > > > >
> > > > > Quoting Gwendal Grignou (2021-03-27 20:36:37)
> > > > > > Semtech SX9310 SAR sensor has a debouncer filter: only when N
> > > > > > measurements are above/below the far/close threshold an event is
> > > > > > sent to the host.
> > > > > > By default the debouncer is set to 2 events for the close to far
> > > > > > transition and 1 event (no debounce) for far to close.
> > > > > > It is a balance speed of detection and false positive avoidance.
> > > > > >
> > > > > > On some chromebooks, the debouncer is set to a larger number.
> > > > > >
> > > > > > This patch applies on top of commit 103d6ec82676 ("iio: sx9310: Support ACPI properties")
> > > > >
> > > > > The near/far debounce settings are already supported via sysfs. Can you
> > > > > use those instead of putting this into DT/ACPI? See
> > > > > sx9310_read_far_debounce() and associated code.
> > > > Stephen, I missed you already proposed these bindings earlier
> > > > [https://lore.kernel.org/linux-devicetree/20200906150247.3aaef3a3@archlinux/].
> > > > After Jonathan inputs, it is done via sysfs interface
> > > > [events/thresh_{falling|rising}_period].
> > > >
> > > > The debounce parameters are in the same class as the other parameters
> > > > already present in the DT. They are calculated during tests to meet
> > > > FCC requirements, in particular the time it takes to detect a human
> > > > body near the antennas.
> >
> > The same could be said for the threshold values but those are in sysfs.
>
> Good point.
>
> >
> > > > Depending on the SAR antenna design, it is a balance between lowering
> > > > the debouncer "period" to raise an event as soon as possible, and
> > > > increasing it to prevent false posible.
> > > >
> > > > In addition to controlling it from sysfs, could it be acceptable to
> > > > have it in DT/ACPI as well?
> > > > In the meantime, I will make sure semtech,sx9310.yaml matches
> > > > in_proximityX_hardwaregain_available and connect to that attribute.
> > > >
> >
> > My understanding from the previous discussions with Jonathan was that
> > anything that could be delayed until later should be done through sysfs.
> > That's why only some properties are in DT, because they were used during
> > initial compensation of the device that happens when the device driver
> > probes. These other values like thresholds and debounce weren't required
> > for that so I put them into sysfs.
>
> My intent wasn't so much things that 'could' be delayed until later, but
> rather the divide between policy (which should be in userspace control)
> and hardware factors. We have a bit of an edge case here so not clear
> cut.
>
> I may well have been wrong in the past on this :(
Looking at the code again, the current approach makes sense, and
having access through sysfs is nice since it allows easy
experimentation.
Let's forget this patchset, I will configure the required parameters from sysfs.
Gwendal.
>
> Jonathan
>
> >
> > >
> > >
> > > Ok, add something to make it clear that these effectively tied to the board
> > > hardware because of this FCC requirement.
> > >
> > > As long as that's clear the Rob is fine with the DT binding I don't mind.
> > >
> > > Is there a requirement as a result of this FCC stuff that we should not
> > > expose them to userspace control if they are provided in DT?
> > >
> > > If so we should figure out a sensible way of doing that without breaking
> > > the existing ABI.
> > >
>
