Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 12 Nov 2022 17:47:36 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Subhajit Ghosh <subhajit.ghosh@vixtechnology.com>
Cc:     Jonathan Cameron <Jonathan.Cameron@Huawei.com>,
        Matt Ranostay <matt.ranostay@konsulko.com>, lars@metafoo.de,
        linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
Subject: Re: [PATCH] iio: light: apds9960: Fix iio_event_spec structures
Message-ID: <20221112174736.75d0b926@jic23-huawei>
In-Reply-To: <380ae3c3-14c2-db1a-954c-caf8180746f0@vixtechnology.com>
References: <20221110101241.10576-1-subhajit.ghosh@vixtechnology.com>
        <CAJCx=g=qRd+WaCLOHwnEjg1Myg4Ng=PK0sxcGgEG9VT+VpondA@mail.gmail.com>
        <b6ac2692-3152-dfc4-5388-7986042970f0@vixtechnology.com>
        <CAJCx=g=WaGiBFYJTTjNgzrnW3We0qpuMvyy9iFAVDC8Mkbscsg@mail.gmail.com>
        <20221111111049.0000467f@Huawei.com>
        <380ae3c3-14c2-db1a-954c-caf8180746f0@vixtechnology.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 11 Nov 2022 21:44:48 +0800
Subhajit Ghosh <subhajit.ghosh@vixtechnology.com> wrote:

> > Hmm.  Given that event enables often cover a couple of different things
> > (as done here) it isn't unknown for those to not be as easily covered
> > as you have done.  As such, we have drivers were the ABI allows for
> > enabling one event to end up enabling several others (even though they
> > have separate enable attributes).  It's always been permitted for one
> > IIO attribute write to have an effect on other attributes simply because
> > we can't represent all dependencies.
> > 
> > Now the bigger complexity / surprise here is the return of the either
> > direction in response to enabling either rising or falling. 
> > That is going to rather surprise your average writer of userspace cod This is where the inconsistency was found. When an ALS threshold rising   
> value was given and as soon as it was enabled, interrupts started firing
> in low light conditions as there was some value present in the ALS falling 
> threshold(reset value is not defined in the datasheet for this register), 
> but falling threshold value was neither fed nor enabled!
> 
> > So patch covers what we should definitely have had in the first place.
> > Hence it's a question of risk of someone running code that will be affected
> > by the ABI change.  One of those fingers crossed moments...  
> I understand that breaking existing userspace applications is not the best
> thing to do.

Given the risks around this one, I'm going to apply it to the togreg
branch of iio.git which is lined up for next merge cycle.  That should
in theory provide more time before it hits upstream / gets back ported
to stable releases for people to notice the change.

Hopefully no one will though!

Applied to the togreg branch of iio.git and pushed out initially as testing
to let 0-day poke at it.

Thanks,

Jonathan

> 
> > 
> > Jonathan  
> 
> Thank you for your time and comments.
> 
> Regards,
> Subhajit Ghosh
>