* [PATCH/RFC] iio: hi8435: do not enable all events by default @ 2017-05-23 8:08 Nikita Yushchenko 2017-05-24 19:27 ` Jonathan Cameron 0 siblings, 1 reply; 8+ messages in thread From: Nikita Yushchenko @ 2017-05-23 8:08 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay Cc: linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov, Nikita Yushchenko Having all events enabled by default is misleading. Userspace should explicitly enable events they want to receive. Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- drivers/iio/adc/hi8435.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c index ef5c286c8e67..0331739f235c 100644 --- a/drivers/iio/adc/hi8435.c +++ b/drivers/iio/adc/hi8435.c @@ -506,8 +506,6 @@ static int hi8435_probe(struct spi_device *spi) idev->channels = hi8435_channels; idev->num_channels = ARRAY_SIZE(hi8435_channels); - /* unmask all events */ - priv->event_scan_mask = ~(0); /* * There is a restriction in the chip - the hysteresis can not be odd. * If the hysteresis is set to odd value then chip gets into lock state -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-23 8:08 [PATCH/RFC] iio: hi8435: do not enable all events by default Nikita Yushchenko @ 2017-05-24 19:27 ` Jonathan Cameron 2017-05-25 5:47 ` Nikita Yushchenko 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Cameron @ 2017-05-24 19:27 UTC (permalink / raw) To: Nikita Yushchenko Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov On Tue, 23 May 2017 11:08:30 +0300 Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > Having all events enabled by default is misleading. > Userspace should explicitly enable events they want to receive. > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> I agree in principle, but this is a userspace ABI change. Sadly we can't do it with out risking breaking userspace code... One of those we should have caught in review, but now it's there we can't actually do anything about it unless we are absolutely sure no one will notice! Jonathan > --- > drivers/iio/adc/hi8435.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c > index ef5c286c8e67..0331739f235c 100644 > --- a/drivers/iio/adc/hi8435.c > +++ b/drivers/iio/adc/hi8435.c > @@ -506,8 +506,6 @@ static int hi8435_probe(struct spi_device *spi) > idev->channels = hi8435_channels; > idev->num_channels = ARRAY_SIZE(hi8435_channels); > > - /* unmask all events */ > - priv->event_scan_mask = ~(0); > /* > * There is a restriction in the chip - the hysteresis can not be odd. > * If the hysteresis is set to odd value then chip gets into lock state ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-24 19:27 ` Jonathan Cameron @ 2017-05-25 5:47 ` Nikita Yushchenko 2017-05-28 15:48 ` Jonathan Cameron 0 siblings, 1 reply; 8+ messages in thread From: Nikita Yushchenko @ 2017-05-25 5:47 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov 24.05.2017 22:27, Jonathan Cameron wrote: > On Tue, 23 May 2017 11:08:30 +0300 > Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > >> Having all events enabled by default is misleading. >> Userspace should explicitly enable events they want to receive. >> >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > I agree in principle, but this is a userspace ABI change. Sadly we > can't do it with out risking breaking userspace code... > > One of those we should have caught in review, but now it's there > we can't actually do anything about it unless we are absolutely > sure no one will notice! I see your point. Still, isn't there subsystem-level default that all events are disabled by default? If such, then current hi8435 state breaks subsystem-level rules, which is a [userspace-visible] bug. I'm not sure how far should we go in bug compatibility. One crazy idea could be - make default selectable via device tree (with default set to all-enabled to keep bug-compatibility). But perhaps that's over-reaction. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-25 5:47 ` Nikita Yushchenko @ 2017-05-28 15:48 ` Jonathan Cameron [not found] ` <CY4PR12MB12549ADA207663849E8DBE8583F20@CY4PR12MB1254.namprd12.prod.outlook.com> 2017-05-29 5:40 ` Nikita Yushchenko 0 siblings, 2 replies; 8+ messages in thread From: Jonathan Cameron @ 2017-05-28 15:48 UTC (permalink / raw) To: Nikita Yushchenko Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov On Thu, 25 May 2017 08:47:47 +0300 Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > 24.05.2017 22:27, Jonathan Cameron wrote: > > On Tue, 23 May 2017 11:08:30 +0300 > > Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > > > >> Having all events enabled by default is misleading. > >> Userspace should explicitly enable events they want to receive. > >> > >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > I agree in principle, but this is a userspace ABI change. Sadly we > > can't do it with out risking breaking userspace code... > > > > One of those we should have caught in review, but now it's there > > we can't actually do anything about it unless we are absolutely > > sure no one will notice! > > I see your point. > > Still, isn't there subsystem-level default that all events are disabled > by default? If such, then current hi8435 state breaks subsystem-level > rules, which is a [userspace-visible] bug. I'm not sure how far should > we go in bug compatibility. It is indeed the subsystem default (as much as we have one) This is a moderately obscure chip for linux systems, do we have a good handle on where it is being used - i.e. are most of the devices under control of people we can discuss this with? > > One crazy idea could be - make default selectable via device tree (with > default set to all-enabled to keep bug-compatibility). But perhaps > that's over-reaction. Yeah, wouldn't fly with the devicetree binding maintainers.. Jonathan ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CY4PR12MB12549ADA207663849E8DBE8583F20@CY4PR12MB1254.namprd12.prod.outlook.com>]
* Re: Fw: [PATCH/RFC] iio: hi8435: do not enable all events by default [not found] ` <CY4PR12MB12549ADA207663849E8DBE8583F20@CY4PR12MB1254.namprd12.prod.outlook.com> @ 2017-05-28 17:00 ` Chris Healy 0 siblings, 0 replies; 8+ messages in thread From: Chris Healy @ 2017-05-28 17:00 UTC (permalink / raw) To: knaack.h, lars, pmeerw, maitysanchayan, gregor.boirie, mranostay, linux-iio, linux-kernel, Jeff White, Vladimir Barinov On Sun, May 28, 2017 at 9:50 AM, Chris Healy <Chris.Healy@zii.aero> wrote: > > > > ________________________________ > From: Jonathan Cameron <jic23@kernel.org> > Sent: Sunday, May 28, 2017 8:48 AM > To: Nikita Yoush > Cc: Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Sanchayan > Maity; Gregor Boirie; Matt Ranostay; linux-iio@vger.kernel.org; > linux-kernel@vger.kernel.org; Chris Healy; Jeff White; Vladimir Barinov > Subject: Re: [PATCH/RFC] iio: hi8435: do not enable all events by default > > On Thu, 25 May 2017 08:47:47 +0300 > Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > >> 24.05.2017 22:27, Jonathan Cameron wrote: >> > On Tue, 23 May 2017 11:08:30 +0300 >> > Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: >> > >> >> Having all events enabled by default is misleading. >> >> Userspace should explicitly enable events they want to receive. >> >> >> >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> >> > I agree in principle, but this is a userspace ABI change. Sadly we >> > can't do it with out risking breaking userspace code... >> > >> > One of those we should have caught in review, but now it's there >> > we can't actually do anything about it unless we are absolutely >> > sure no one will notice! >> >> I see your point. >> >> Still, isn't there subsystem-level default that all events are disabled >> by default? If such, then current hi8435 state breaks subsystem-level >> rules, which is a [userspace-visible] bug. I'm not sure how far should >> we go in bug compatibility. > It is indeed the subsystem default (as much as we have one) > > This is a moderately obscure chip for linux systems, do we have a good > handle > on where it is being used - i.e. are most of the devices under control of > people we can discuss this with? The company I work for funded the initial work to get support for the this Holt HI-8435 upstream. We are presently using this driver and can accommodate this proposed userspace ABI change as we can adjust userspace if the ABI changes. Unfortunately, this doesn't answer the question of the drivers adoption elsewhere. >> >> One crazy idea could be - make default selectable via device tree (with >> default set to all-enabled to keep bug-compatibility). But perhaps >> that's over-reaction. > Yeah, wouldn't fly with the devicetree binding maintainers.. > > Jonathan > > ________________________________ > > > This email and any files transmitted with it are confidential & proprietary > to Zodiac Inflight Innovations. This information is intended solely for the > use of the individual or entity to which it is addressed. Access or > transmittal of the information contained in this e-mail, in full or in part, > to any other organization or persons is not authorized. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-28 15:48 ` Jonathan Cameron [not found] ` <CY4PR12MB12549ADA207663849E8DBE8583F20@CY4PR12MB1254.namprd12.prod.outlook.com> @ 2017-05-29 5:40 ` Nikita Yushchenko 2017-06-03 9:07 ` Jonathan Cameron 2017-06-09 9:17 ` Linus Walleij 1 sibling, 2 replies; 8+ messages in thread From: Nikita Yushchenko @ 2017-05-29 5:40 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov >> Still, isn't there subsystem-level default that all events are disabled >> by default? If such, then current hi8435 state breaks subsystem-level >> rules, which is a [userspace-visible] bug. I'm not sure how far should >> we go in bug compatibility. > > It is indeed the subsystem default (as much as we have one) > > This is a moderately obscure chip for linux systems, do we have a good handle > on where it is being used - i.e. are most of the devices under control of > people we can discuss this with? Company I work with, uses this chip in several boards; what they need is a service that monitors all connected chip's outputs and detects changes. They originally wanted gpio-style access to use with userspace polling, and were not pleased with entire IIO thing. However it's important for them to minimize required kernel patches against mainline, thus if mainline supports this chip as IIO device that's ok for them. Questions like default event enable state has little practical impact. It's more about keeping architecture clean. Nikita ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-29 5:40 ` Nikita Yushchenko @ 2017-06-03 9:07 ` Jonathan Cameron 2017-06-09 9:17 ` Linus Walleij 1 sibling, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2017-06-03 9:07 UTC (permalink / raw) To: Nikita Yushchenko Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov, Linus Walleij On Mon, 29 May 2017 08:40:26 +0300 Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > >> Still, isn't there subsystem-level default that all events are disabled > >> by default? If such, then current hi8435 state breaks subsystem-level > >> rules, which is a [userspace-visible] bug. I'm not sure how far should > >> we go in bug compatibility. > > > > It is indeed the subsystem default (as much as we have one) > > > > This is a moderately obscure chip for linux systems, do we have a good handle > > on where it is being used - i.e. are most of the devices under control of > > people we can discuss this with? > > Company I work with, uses this chip in several boards; what they need is > a service that monitors all connected chip's outputs and detects > changes. They originally wanted gpio-style access to use with userspace > polling, and were not pleased with entire IIO thing. However it's > important for them to minimize required kernel patches against mainline, > thus if mainline supports this chip as IIO device that's ok for them. Was always an odd corner case - and I'll admit it wasn't one that made me very comfortable either. One option would be to put a GPIO bridge driver in place so that both interfaces are available. I know this is something Linus and I have thought about in the past for various use cases. > > Questions like default event enable state has little practical impact. > It's more about keeping architecture clean. I'm going to go with not changing this, even though it matters little to either of you. Simply because we clearly do have a non trivial number of users and this would be ABI breakage. Jonathan > > Nikita > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH/RFC] iio: hi8435: do not enable all events by default 2017-05-29 5:40 ` Nikita Yushchenko 2017-06-03 9:07 ` Jonathan Cameron @ 2017-06-09 9:17 ` Linus Walleij 1 sibling, 0 replies; 8+ messages in thread From: Linus Walleij @ 2017-06-09 9:17 UTC (permalink / raw) To: Nikita Yushchenko Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Sanchayan Maity, Gregor Boirie, Matt Ranostay, linux-iio, linux-kernel, Chris Healy, Jeff White, Vladimir Barinov On Mon, May 29, 2017 at 7:40 AM, Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > They originally wanted gpio-style access to use with userspace > polling, and were not pleased with entire IIO thing. The GPIO userspace ABI supports events with timestamps. tools/gpio/gpio-event-mon.c Yours, Linus Walleij ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-06-09 9:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-23 8:08 [PATCH/RFC] iio: hi8435: do not enable all events by default Nikita Yushchenko 2017-05-24 19:27 ` Jonathan Cameron 2017-05-25 5:47 ` Nikita Yushchenko 2017-05-28 15:48 ` Jonathan Cameron [not found] ` <CY4PR12MB12549ADA207663849E8DBE8583F20@CY4PR12MB1254.namprd12.prod.outlook.com> 2017-05-28 17:00 ` Fw: " Chris Healy 2017-05-29 5:40 ` Nikita Yushchenko 2017-06-03 9:07 ` Jonathan Cameron 2017-06-09 9:17 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).