| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 27 Mar 2017 16:43:26 +0200
From: Benjamin Gaignard <benjamin.gaignard@...aro.org>
To: William Breathitt Gray <vilhelm.gray@...il.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org,
Hartmut Knaack <knaack.h@....de>,
Lars-Peter Clausen <lars@...afoo.de>,
Peter Meerwald-Stadler <pmeerw@...erw.net>, mwelling@...e.org,
Fabrice Gasnier <fabrice.gasnier@...com>,
Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>,
Benjamin Gaignard <benjamin.gaignard@...com>
Subject: Re: [PATCH v3 2/2] iio: stm32 trigger: Add counter enable modes
2017-03-27 15:55 GMT+02:00 William Breathitt Gray <vilhelm.gray@...il.com>:
> On Mon, Mar 27, 2017 at 11:43:24AM +0200, Benjamin Gaignard wrote:
>>Device counting could be controlled by the level or the edges of
>>a trigger.
>>in_count0_enable_mode attibute allow to set the control mode.
>>
>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...com>
>
> This patch is pretty straight-forward and all right with me. A minor
> nitpick inline regarding documentation.
>
> Reviewed-by: William Breathitt Gray <vilhelm.gray@...il.com>
>
>>---
>> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 23 +++++++
>> drivers/iio/trigger/stm32-timer-trigger.c | 70 ++++++++++++++++++++++
>> 2 files changed, 93 insertions(+)
>>
>>diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>index bf795ad..c0a1edc 100644
>>--- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>@@ -59,3 +59,26 @@ Description:
>> quadrature:
>> Encoder A and B inputs are mixed to get direction
>> and count with a scale of 0.25.
>>+
>>+What: /sys/bus/iio/devices/iio:deviceX/in_count_enable_mode_available
>>+KernelVersion: 4.12
>>+Contact: benjamin.gaignard@...com
>>+Description:
>>+ Reading returns the list possible enable modes.
>>+
>>+What: /sys/bus/iio/devices/iio:deviceX/in_count0_enable_mode
>>+KernelVersion: 4.12
>>+Contact: benjamin.gaignard@...com
>>+Description:
>>+ Configure the device counter enable modes, in all case
>>+ counting direction is set by in_count0_count_direction
>>+ attribute and the counter is clocked by the internal clock.
>>+ always:
>>+ Counter is always ON.
>>+
>>+ gated:
>>+ Counting is enabled when connected trigger signal
>>+ level is high else counting is disabled.
>>+
>>+ triggered:
>>+ Counting start on rising edge of the connected trigger.
>
> When I read the description of the "triggered" mode, I get the
> impression that counting is restarted every time there is a rising edge
> from the connected trigger signal. However, from our previous
> discussions I believe this mode is simply a latched enable.
>
> Perhaps it may be beneficial for clarity to rework the description to
> say something along the lines of: "Counting is enabled on rising edge of
> the connected trigger, and remains enabled for the duration of this
> selected mode."
I will update that in v4.
Thanks
>
> William Breathitt Gray
>
>>diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>index 7db904c..0f1a2cf 100644
>>--- a/drivers/iio/trigger/stm32-timer-trigger.c
>>+++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>@@ -353,6 +353,74 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>> .write_raw = stm32_counter_write_raw
>> };
>>
>>+static const char *const stm32_enable_modes[] = {
>>+ "always",
>>+ "gated",
>>+ "triggered",
>>+};
>>+
>>+static int stm32_enable_mode2sms(int mode)
>>+{
>>+ switch (mode) {
>>+ case 0:
>>+ return 0;
>>+ case 1:
>>+ return 5;
>>+ case 2:
>>+ return 6;
>>+ }
>>+
>>+ return -EINVAL;
>>+}
>>+
>>+static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>>+ const struct iio_chan_spec *chan,
>>+ unsigned int mode)
>>+{
>>+ struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+ int sms = stm32_enable_mode2sms(mode);
>>+
>>+ if (sms < 0)
>>+ return sms;
>>+
>>+ regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
>>+
>>+ return 0;
>>+}
>>+
>>+static int stm32_sms2enable_mode(int mode)
>>+{
>>+ switch (mode) {
>>+ case 0:
>>+ return 0;
>>+ case 5:
>>+ return 1;
>>+ case 6:
>>+ return 2;
>>+ }
>>+
>>+ return -EINVAL;
>>+}
>>+
>>+static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>>+ const struct iio_chan_spec *chan)
>>+{
>>+ struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>+ u32 smcr;
>>+
>>+ regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>+ smcr &= TIM_SMCR_SMS;
>>+
>>+ return stm32_sms2enable_mode(smcr);
>>+}
>>+
>>+static const struct iio_enum stm32_enable_mode_enum = {
>>+ .items = stm32_enable_modes,
>>+ .num_items = ARRAY_SIZE(stm32_enable_modes),
>>+ .set = stm32_set_enable_mode,
>>+ .get = stm32_get_enable_mode
>>+};
>>+
>> static const char *const stm32_quadrature_modes[] = {
>> "channel_A",
>> "channel_B",
>>@@ -466,6 +534,8 @@ static ssize_t stm32_count_set_preset(struct iio_dev *indio_dev,
>> IIO_ENUM_AVAILABLE("count_direction", &stm32_count_direction_enum),
>> IIO_ENUM("quadrature_mode", IIO_SEPARATE, &stm32_quadrature_mode_enum),
>> IIO_ENUM_AVAILABLE("quadrature_mode", &stm32_quadrature_mode_enum),
>>+ IIO_ENUM("enable_mode", IIO_SEPARATE, &stm32_enable_mode_enum),
>>+ IIO_ENUM_AVAILABLE("enable_mode", &stm32_enable_mode_enum),
>> {}
>> };
>>
>>--
>>1.9.1
>>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Powered by blists - more mailing lists