* [PATCH] iio: trigger: stm32-timer: fix the usage of uninitialized variables
@ 2019-09-30 20:44 Yizhuo
2019-10-01 8:43 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Yizhuo @ 2019-09-30 20:44 UTC (permalink / raw)
Cc: csong, Lars-Peter Clausen, Maxime Coquelin, linux-iio, zhiyunq,
linux-kernel, Yizhuo, Benjamin Gaignard, linux-arm-kernel,
Peter Meerwald-Stadler, Hartmut Knaack, linux-stm32,
Jonathan Cameron, Alexandre Torgue
Several functions in this file are trying to use regmap_read() to
initialize the specific variable, however, if regmap_read() fails,
the variable could be uninitialized but used directly, which is
potentially unsafe. The return value of regmap_read() should be
checked and handled. This patch fixes most of the uninitialized
variables, but those in function stm32_tt_read_frequency() are
hard to handle and need extra effot.
Signed-off-by: Yizhuo <yzhai003@ucr.edu>
---
drivers/iio/trigger/stm32-timer-trigger.c | 98 ++++++++++++++++++++---
1 file changed, 85 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
index a5dfe65cd9b9..f8ea7bcbb739 100644
--- a/drivers/iio/trigger/stm32-timer-trigger.c
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -107,6 +107,7 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
unsigned long long prd, div;
int prescaler = 0;
u32 ccer, cr1;
+ int ret;
/* Period and prescaler values depends of clock rate */
div = (unsigned long long)clk_get_rate(priv->clk);
@@ -132,11 +133,21 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
}
/* Check if nobody else use the timer */
- regmap_read(priv->regmap, TIM_CCER, &ccer);
+ ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CCER.\n");
+ return ret;
+ }
+
if (ccer & TIM_CCER_CCXE)
return -EBUSY;
- regmap_read(priv->regmap, TIM_CR1, &cr1);
+ ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR1.\n");
+ return ret;
+ }
+
if (!(cr1 & TIM_CR1_CEN))
clk_enable(priv->clk);
@@ -164,12 +175,23 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
static void stm32_timer_stop(struct stm32_timer_trigger *priv)
{
u32 ccer, cr1;
+ int ret;
+
+ ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
+ if (ret) {
+ dev_err(priv->dev, "Fail to read TIM_CCER.\n");
+ return;
+ }
- regmap_read(priv->regmap, TIM_CCER, &ccer);
if (ccer & TIM_CCER_CCXE)
return;
- regmap_read(priv->regmap, TIM_CR1, &cr1);
+ ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
+ if (ret) {
+ dev_err(priv->dev, "Fail to read TIM_CR1.\n");
+ return;
+ }
+
if (cr1 & TIM_CR1_CEN)
clk_disable(priv->clk);
@@ -403,20 +425,36 @@ static int stm32_counter_read_raw(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
u32 dat;
+ int ret;
switch (mask) {
case IIO_CHAN_INFO_RAW:
- regmap_read(priv->regmap, TIM_CNT, &dat);
+ ret = regmap_read(priv->regmap, TIM_CNT, &dat);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CNT.\n");
+ return ret;
+ }
+
*val = dat;
return IIO_VAL_INT;
case IIO_CHAN_INFO_ENABLE:
- regmap_read(priv->regmap, TIM_CR1, &dat);
+ ret = regmap_read(priv->regmap, TIM_CR1, &dat);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR1.\n");
+ return ret;
+ }
+
*val = (dat & TIM_CR1_CEN) ? 1 : 0;
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- regmap_read(priv->regmap, TIM_SMCR, &dat);
+ ret = regmap_read(priv->regmap, TIM_SMCR, &dat);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_SMCR.\n");
+ return ret;
+ }
+
dat &= TIM_SMCR_SMS;
*val = 1;
@@ -438,6 +476,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
u32 dat;
+ int ret;
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -449,13 +488,23 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_ENABLE:
if (val) {
- regmap_read(priv->regmap, TIM_CR1, &dat);
+ ret = regmap_read(priv->regmap, TIM_CR1, &dat);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR1.\n");
+ return ret;
+ }
+
if (!(dat & TIM_CR1_CEN))
clk_enable(priv->clk);
regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
TIM_CR1_CEN);
} else {
- regmap_read(priv->regmap, TIM_CR1, &dat);
+ ret = regmap_read(priv->regmap, TIM_CR1, &dat);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR1.\n");
+ return ret;
+ }
+
regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
0);
if (dat & TIM_CR1_CEN)
@@ -517,8 +566,13 @@ static int stm32_get_trigger_mode(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
u32 smcr;
+ int ret;
- regmap_read(priv->regmap, TIM_SMCR, &smcr);
+ ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_SMCR.\n");
+ return ret;
+ }
return (smcr & TIM_SMCR_SMS) == TIM_SMCR_SMS ? 0 : -EINVAL;
}
@@ -557,6 +611,7 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
int sms = stm32_enable_mode2sms(mode);
u32 val;
+ int ret;
if (sms < 0)
return sms;
@@ -565,7 +620,12 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
* enable counter clock, so it can use it. Keeps it in sync with CEN.
*/
if (sms == 6) {
- regmap_read(priv->regmap, TIM_CR1, &val);
+ ret = regmap_read(priv->regmap, TIM_CR1, &val);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR1.\n");
+ return ret;
+ }
+
if (!(val & TIM_CR1_CEN))
clk_enable(priv->clk);
}
@@ -594,8 +654,14 @@ static int stm32_get_enable_mode(struct iio_dev *indio_dev,
{
struct stm32_timer_trigger *priv = iio_priv(indio_dev);
u32 smcr;
+ int ret;
+
+ ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_SMCR.\n");
+ return ret;
+ }
- regmap_read(priv->regmap, TIM_SMCR, &smcr);
smcr &= TIM_SMCR_SMS;
return stm32_sms2enable_mode(smcr);
@@ -706,13 +772,19 @@ EXPORT_SYMBOL(is_stm32_timer_trigger);
static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
{
u32 val;
+ int ret;
/*
* Master mode selection 2 bits can only be written and read back when
* timer supports it.
*/
regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
- regmap_read(priv->regmap, TIM_CR2, &val);
+ ret = regmap_read(priv->regmap, TIM_CR2, &val);
+ if (ret) {
+ dev_err(priv->dev, "fail to read TIM_CR2.\n");
+ return;
+ }
+
regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
priv->has_trgo2 = !!val;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: trigger: stm32-timer: fix the usage of uninitialized variables
2019-09-30 20:44 [PATCH] iio: trigger: stm32-timer: fix the usage of uninitialized variables Yizhuo
@ 2019-10-01 8:43 ` Jonathan Cameron
2019-10-02 7:56 ` Benjamin GAIGNARD
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2019-10-01 8:43 UTC (permalink / raw)
To: Yizhuo
Cc: csong, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
zhiyunq, linux-kernel, Benjamin Gaignard, Maxime Coquelin,
Hartmut Knaack, linux-stm32, linux-arm-kernel, Alexandre Torgue
On Mon, 30 Sep 2019 13:44:49 -0700
Yizhuo <yzhai003@ucr.edu> wrote:
> Several functions in this file are trying to use regmap_read() to
> initialize the specific variable, however, if regmap_read() fails,
> the variable could be uninitialized but used directly, which is
> potentially unsafe. The return value of regmap_read() should be
> checked and handled. This patch fixes most of the uninitialized
> variables, but those in function stm32_tt_read_frequency() are
> hard to handle and need extra effot.
>
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>
Hi Yizhuo,
This is a slightly interesting case.
From an 'obviously' correct point of view, it makes sense to
always handle the errors and avoid the potentially uninitialised
cases.
From the point of view of a 'fix' we need to show if these can
actually happen. These calls are regmap-mmio so the error
path is in clock enable / disable if there is a clock provided.
Looking at mfd/stm-timers.c there is a clock called "int" provided.
Now it's possible that clock enable can never fail, but that is
getting too hard to chase down.
As such, I think this fix is worthwhile, but I'm not going to
mark it for stable without a report of it actually being
possible to trigger it.
I'll leave this on the list for a little longer though as I'd definitely
like an ack from Benjamin or someone else more familiar with the driver
than I am.
Thanks,
Jonathan
> ---
> drivers/iio/trigger/stm32-timer-trigger.c | 98 ++++++++++++++++++++---
> 1 file changed, 85 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
> index a5dfe65cd9b9..f8ea7bcbb739 100644
> --- a/drivers/iio/trigger/stm32-timer-trigger.c
> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
> @@ -107,6 +107,7 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
> unsigned long long prd, div;
> int prescaler = 0;
> u32 ccer, cr1;
> + int ret;
>
> /* Period and prescaler values depends of clock rate */
> div = (unsigned long long)clk_get_rate(priv->clk);
> @@ -132,11 +133,21 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
> }
>
> /* Check if nobody else use the timer */
> - regmap_read(priv->regmap, TIM_CCER, &ccer);
> + ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CCER.\n");
> + return ret;
> + }
> +
> if (ccer & TIM_CCER_CCXE)
> return -EBUSY;
>
> - regmap_read(priv->regmap, TIM_CR1, &cr1);
> + ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
> + return ret;
> + }
> +
> if (!(cr1 & TIM_CR1_CEN))
> clk_enable(priv->clk);
>
> @@ -164,12 +175,23 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
> static void stm32_timer_stop(struct stm32_timer_trigger *priv)
> {
> u32 ccer, cr1;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
> + if (ret) {
> + dev_err(priv->dev, "Fail to read TIM_CCER.\n");
> + return;
> + }
>
> - regmap_read(priv->regmap, TIM_CCER, &ccer);
> if (ccer & TIM_CCER_CCXE)
> return;
>
> - regmap_read(priv->regmap, TIM_CR1, &cr1);
> + ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
> + if (ret) {
> + dev_err(priv->dev, "Fail to read TIM_CR1.\n");
> + return;
> + }
> +
> if (cr1 & TIM_CR1_CEN)
> clk_disable(priv->clk);
>
> @@ -403,20 +425,36 @@ static int stm32_counter_read_raw(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> u32 dat;
> + int ret;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - regmap_read(priv->regmap, TIM_CNT, &dat);
> + ret = regmap_read(priv->regmap, TIM_CNT, &dat);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CNT.\n");
> + return ret;
> + }
> +
> *val = dat;
> return IIO_VAL_INT;
>
> case IIO_CHAN_INFO_ENABLE:
> - regmap_read(priv->regmap, TIM_CR1, &dat);
> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
> + return ret;
> + }
> +
> *val = (dat & TIM_CR1_CEN) ? 1 : 0;
> return IIO_VAL_INT;
>
> case IIO_CHAN_INFO_SCALE:
> - regmap_read(priv->regmap, TIM_SMCR, &dat);
> + ret = regmap_read(priv->regmap, TIM_SMCR, &dat);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
> + return ret;
> + }
> +
> dat &= TIM_SMCR_SMS;
>
> *val = 1;
> @@ -438,6 +476,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> u32 dat;
> + int ret;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -449,13 +488,23 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>
> case IIO_CHAN_INFO_ENABLE:
> if (val) {
> - regmap_read(priv->regmap, TIM_CR1, &dat);
> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
> + return ret;
> + }
> +
> if (!(dat & TIM_CR1_CEN))
> clk_enable(priv->clk);
> regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
> TIM_CR1_CEN);
> } else {
> - regmap_read(priv->regmap, TIM_CR1, &dat);
> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
> + return ret;
> + }
> +
> regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
> 0);
> if (dat & TIM_CR1_CEN)
> @@ -517,8 +566,13 @@ static int stm32_get_trigger_mode(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> u32 smcr;
> + int ret;
>
> - regmap_read(priv->regmap, TIM_SMCR, &smcr);
> + ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
> + return ret;
> + }
>
> return (smcr & TIM_SMCR_SMS) == TIM_SMCR_SMS ? 0 : -EINVAL;
> }
> @@ -557,6 +611,7 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> int sms = stm32_enable_mode2sms(mode);
> u32 val;
> + int ret;
>
> if (sms < 0)
> return sms;
> @@ -565,7 +620,12 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
> * enable counter clock, so it can use it. Keeps it in sync with CEN.
> */
> if (sms == 6) {
> - regmap_read(priv->regmap, TIM_CR1, &val);
> + ret = regmap_read(priv->regmap, TIM_CR1, &val);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
> + return ret;
> + }
> +
> if (!(val & TIM_CR1_CEN))
> clk_enable(priv->clk);
> }
> @@ -594,8 +654,14 @@ static int stm32_get_enable_mode(struct iio_dev *indio_dev,
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> u32 smcr;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
> + return ret;
> + }
>
> - regmap_read(priv->regmap, TIM_SMCR, &smcr);
> smcr &= TIM_SMCR_SMS;
>
> return stm32_sms2enable_mode(smcr);
> @@ -706,13 +772,19 @@ EXPORT_SYMBOL(is_stm32_timer_trigger);
> static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
> {
> u32 val;
> + int ret;
>
> /*
> * Master mode selection 2 bits can only be written and read back when
> * timer supports it.
> */
> regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
> - regmap_read(priv->regmap, TIM_CR2, &val);
> + ret = regmap_read(priv->regmap, TIM_CR2, &val);
> + if (ret) {
> + dev_err(priv->dev, "fail to read TIM_CR2.\n");
> + return;
> + }
> +
> regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
> priv->has_trgo2 = !!val;
> }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: trigger: stm32-timer: fix the usage of uninitialized variables
2019-10-01 8:43 ` Jonathan Cameron
@ 2019-10-02 7:56 ` Benjamin GAIGNARD
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin GAIGNARD @ 2019-10-02 7:56 UTC (permalink / raw)
To: Jonathan Cameron, Yizhuo
Cc: csong, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
zhiyunq, linux-kernel, Maxime Coquelin, Hartmut Knaack,
linux-stm32, linux-arm-kernel, Alexandre TORGUE
On 10/1/19 10:43 AM, Jonathan Cameron wrote:
> On Mon, 30 Sep 2019 13:44:49 -0700
> Yizhuo <yzhai003@ucr.edu> wrote:
>
>> Several functions in this file are trying to use regmap_read() to
>> initialize the specific variable, however, if regmap_read() fails,
>> the variable could be uninitialized but used directly, which is
>> potentially unsafe. The return value of regmap_read() should be
>> checked and handled. This patch fixes most of the uninitialized
>> variables, but those in function stm32_tt_read_frequency() are
>> hard to handle and need extra effot.
>>
>> Signed-off-by: Yizhuo <yzhai003@ucr.edu>
> Hi Yizhuo,
>
> This is a slightly interesting case.
>
> From an 'obviously' correct point of view, it makes sense to
> always handle the errors and avoid the potentially uninitialised
> cases.
>
> From the point of view of a 'fix' we need to show if these can
> actually happen. These calls are regmap-mmio so the error
> path is in clock enable / disable if there is a clock provided.
> Looking at mfd/stm-timers.c there is a clock called "int" provided.
> Now it's possible that clock enable can never fail, but that is
> getting too hard to chase down.
>
> As such, I think this fix is worthwhile, but I'm not going to
> mark it for stable without a report of it actually being
> possible to trigger it.
>
> I'll leave this on the list for a little longer though as I'd definitely
> like an ack from Benjamin or someone else more familiar with the driver
> than I am.
>
> Thanks,
>
> Jonathan
Hi Yizhuo,
The only reason that could lead a regmap_read to failed is an issue on
the clock
but is this case we will be blocked in regmap_read() itself so the error
will never
be displayed.
For me testing all regmap_read() return values only add code and not
bring additional informations
so I NACK this patch.
Benjamin
>> ---
>> drivers/iio/trigger/stm32-timer-trigger.c | 98 ++++++++++++++++++++---
>> 1 file changed, 85 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>> index a5dfe65cd9b9..f8ea7bcbb739 100644
>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>> @@ -107,6 +107,7 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
>> unsigned long long prd, div;
>> int prescaler = 0;
>> u32 ccer, cr1;
>> + int ret;
>>
>> /* Period and prescaler values depends of clock rate */
>> div = (unsigned long long)clk_get_rate(priv->clk);
>> @@ -132,11 +133,21 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
>> }
>>
>> /* Check if nobody else use the timer */
>> - regmap_read(priv->regmap, TIM_CCER, &ccer);
>> + ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CCER.\n");
>> + return ret;
>> + }
>> +
>> if (ccer & TIM_CCER_CCXE)
>> return -EBUSY;
>>
>> - regmap_read(priv->regmap, TIM_CR1, &cr1);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
>> + return ret;
>> + }
>> +
>> if (!(cr1 & TIM_CR1_CEN))
>> clk_enable(priv->clk);
>>
>> @@ -164,12 +175,23 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
>> static void stm32_timer_stop(struct stm32_timer_trigger *priv)
>> {
>> u32 ccer, cr1;
>> + int ret;
>> +
>> + ret = regmap_read(priv->regmap, TIM_CCER, &ccer);
>> + if (ret) {
>> + dev_err(priv->dev, "Fail to read TIM_CCER.\n");
>> + return;
>> + }
>>
>> - regmap_read(priv->regmap, TIM_CCER, &ccer);
>> if (ccer & TIM_CCER_CCXE)
>> return;
>>
>> - regmap_read(priv->regmap, TIM_CR1, &cr1);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &cr1);
>> + if (ret) {
>> + dev_err(priv->dev, "Fail to read TIM_CR1.\n");
>> + return;
>> + }
>> +
>> if (cr1 & TIM_CR1_CEN)
>> clk_disable(priv->clk);
>>
>> @@ -403,20 +425,36 @@ static int stm32_counter_read_raw(struct iio_dev *indio_dev,
>> {
>> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> u32 dat;
>> + int ret;
>>
>> switch (mask) {
>> case IIO_CHAN_INFO_RAW:
>> - regmap_read(priv->regmap, TIM_CNT, &dat);
>> + ret = regmap_read(priv->regmap, TIM_CNT, &dat);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CNT.\n");
>> + return ret;
>> + }
>> +
>> *val = dat;
>> return IIO_VAL_INT;
>>
>> case IIO_CHAN_INFO_ENABLE:
>> - regmap_read(priv->regmap, TIM_CR1, &dat);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
>> + return ret;
>> + }
>> +
>> *val = (dat & TIM_CR1_CEN) ? 1 : 0;
>> return IIO_VAL_INT;
>>
>> case IIO_CHAN_INFO_SCALE:
>> - regmap_read(priv->regmap, TIM_SMCR, &dat);
>> + ret = regmap_read(priv->regmap, TIM_SMCR, &dat);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
>> + return ret;
>> + }
>> +
>> dat &= TIM_SMCR_SMS;
>>
>> *val = 1;
>> @@ -438,6 +476,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>> {
>> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> u32 dat;
>> + int ret;
>>
>> switch (mask) {
>> case IIO_CHAN_INFO_RAW:
>> @@ -449,13 +488,23 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>>
>> case IIO_CHAN_INFO_ENABLE:
>> if (val) {
>> - regmap_read(priv->regmap, TIM_CR1, &dat);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
>> + return ret;
>> + }
>> +
>> if (!(dat & TIM_CR1_CEN))
>> clk_enable(priv->clk);
>> regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
>> TIM_CR1_CEN);
>> } else {
>> - regmap_read(priv->regmap, TIM_CR1, &dat);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &dat);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
>> + return ret;
>> + }
>> +
>> regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
>> 0);
>> if (dat & TIM_CR1_CEN)
>> @@ -517,8 +566,13 @@ static int stm32_get_trigger_mode(struct iio_dev *indio_dev,
>> {
>> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> u32 smcr;
>> + int ret;
>>
>> - regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> + ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
>> + return ret;
>> + }
>>
>> return (smcr & TIM_SMCR_SMS) == TIM_SMCR_SMS ? 0 : -EINVAL;
>> }
>> @@ -557,6 +611,7 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> int sms = stm32_enable_mode2sms(mode);
>> u32 val;
>> + int ret;
>>
>> if (sms < 0)
>> return sms;
>> @@ -565,7 +620,12 @@ static int stm32_set_enable_mode(struct iio_dev *indio_dev,
>> * enable counter clock, so it can use it. Keeps it in sync with CEN.
>> */
>> if (sms == 6) {
>> - regmap_read(priv->regmap, TIM_CR1, &val);
>> + ret = regmap_read(priv->regmap, TIM_CR1, &val);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR1.\n");
>> + return ret;
>> + }
>> +
>> if (!(val & TIM_CR1_CEN))
>> clk_enable(priv->clk);
>> }
>> @@ -594,8 +654,14 @@ static int stm32_get_enable_mode(struct iio_dev *indio_dev,
>> {
>> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>> u32 smcr;
>> + int ret;
>> +
>> + ret = regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_SMCR.\n");
>> + return ret;
>> + }
>>
>> - regmap_read(priv->regmap, TIM_SMCR, &smcr);
>> smcr &= TIM_SMCR_SMS;
>>
>> return stm32_sms2enable_mode(smcr);
>> @@ -706,13 +772,19 @@ EXPORT_SYMBOL(is_stm32_timer_trigger);
>> static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
>> {
>> u32 val;
>> + int ret;
>>
>> /*
>> * Master mode selection 2 bits can only be written and read back when
>> * timer supports it.
>> */
>> regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
>> - regmap_read(priv->regmap, TIM_CR2, &val);
>> + ret = regmap_read(priv->regmap, TIM_CR2, &val);
>> + if (ret) {
>> + dev_err(priv->dev, "fail to read TIM_CR2.\n");
>> + return;
>> + }
>> +
>> regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
>> priv->has_trgo2 = !!val;
>> }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-02 7:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 20:44 [PATCH] iio: trigger: stm32-timer: fix the usage of uninitialized variables Yizhuo
2019-10-01 8:43 ` Jonathan Cameron
2019-10-02 7:56 ` Benjamin GAIGNARD
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).