* [PATCH] gpio: twl4030: Remove redundant assignment
@ 2014-03-22 8:32 Alexander Shiyan
2014-03-27 9:07 ` Linus Walleij
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Shiyan @ 2014-03-22 8:32 UTC (permalink / raw)
To: linux-gpio; +Cc: Linus Walleij, Alexandre Courbot, Alexander Shiyan
Variable "status" is never used, so remove it and add warning if
any error happen.
Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
drivers/gpio/gpio-twl4030.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index 8b88ca2..3ebb1a5 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -139,7 +139,6 @@ static u8 cached_leden;
static void twl4030_led_set_value(int led, int value)
{
u8 mask = LEDEN_LEDAON | LEDEN_LEDAPWM;
- int status;
if (led)
mask <<= 1;
@@ -148,8 +147,9 @@ static void twl4030_led_set_value(int led, int value)
cached_leden &= ~mask;
else
cached_leden |= mask;
- status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
- TWL4030_LED_LEDEN_REG);
+
+ WARN_ON_ONCE(twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
+ TWL4030_LED_LEDEN_REG));
}
static int twl4030_set_gpio_direction(int gpio, int is_input)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] gpio: twl4030: Remove redundant assignment
2014-03-22 8:32 [PATCH] gpio: twl4030: Remove redundant assignment Alexander Shiyan
@ 2014-03-27 9:07 ` Linus Walleij
0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2014-03-27 9:07 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: linux-gpio, Alexandre Courbot
On Sat, Mar 22, 2014 at 9:32 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Variable "status" is never used, so remove it and add warning if
> any error happen.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
Looks sound, patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gpio: twl4030: Remove redundant assignment
2014-02-27 9:24 ` Linus Walleij
2014-02-27 9:32 ` Peter Ujfalusi
@ 2014-02-27 10:59 ` Alexander Shiyan
1 sibling, 0 replies; 8+ messages in thread
From: Alexander Shiyan @ 2014-02-27 10:59 UTC (permalink / raw)
To: Linus Walleij
Cc: Roger Quadros, Tony Lindgren, Peter Ujfalusi, linux-gpio,
Alexandre Courbot
Четверг, 27 февраля 2014, 10:24 +01:00 от Linus Walleij <linus.walleij@linaro.org>:
> On Mon, Feb 24, 2014 at 4:34 PM, Roger Quadros <rogerq@ti.com> wrote:
> > On 02/24/2014 04:25 PM, Linus Walleij wrote:
> >> On Sat, Feb 15, 2014 at 2:34 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> >>
> >>> - status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
> >>> - TWL4030_LED_LEDEN_REG);
> >>> +
> >>> + twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
> >>> + TWL4030_LED_LEDEN_REG);
> >>
> >> Isn't the right fix to actually *check* this status instead?
> >>
> >> TI dudes?
> >
> > Yes we should check for error. But the only action we can take is maybe print an error message
> > as all the users of this function return void. e.g. twl_set().
> >
> > It seems the set() hook of struct gpio_chip also doesn't expect any return value. Wondering if that should change.
>
> I think in this case simply printing an error and bailing out is
> just fine, people will be able to debug from there.
WARN_ON_ONCE() is enough?
v2?
---
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gpio: twl4030: Remove redundant assignment
2014-02-27 9:24 ` Linus Walleij
@ 2014-02-27 9:32 ` Peter Ujfalusi
2014-02-27 10:59 ` Alexander Shiyan
1 sibling, 0 replies; 8+ messages in thread
From: Peter Ujfalusi @ 2014-02-27 9:32 UTC (permalink / raw)
To: Linus Walleij, Roger Quadros
Cc: Alexander Shiyan, Tony Lindgren, linux-gpio, Alexandre Courbot
On 02/27/2014 11:24 AM, Linus Walleij wrote:
> On Mon, Feb 24, 2014 at 4:34 PM, Roger Quadros <rogerq@ti.com> wrote:
>> On 02/24/2014 04:25 PM, Linus Walleij wrote:
>>> On Sat, Feb 15, 2014 at 2:34 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
>>>
>>>> - status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>>>> - TWL4030_LED_LEDEN_REG);
>>>> +
>>>> + twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>>>> + TWL4030_LED_LEDEN_REG);
>>>
>>> Isn't the right fix to actually *check* this status instead?
>>>
>>> TI dudes?
>>
>> Yes we should check for error. But the only action we can take is maybe print an error message
>> as all the users of this function return void. e.g. twl_set().
>>
>> It seems the set() hook of struct gpio_chip also doesn't expect any return value. Wondering if that should change.
>
> I think in this case simply printing an error and bailing out is
> just fine, people will be able to debug from there.
FWIW: if a write to twl4030 fails we are anyways in a big trouble. This is the
PMIC in the system, so it is likely that we do not even boot in such case.
--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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] gpio: twl4030: Remove redundant assignment
2014-02-24 15:34 ` Roger Quadros
@ 2014-02-27 9:24 ` Linus Walleij
2014-02-27 9:32 ` Peter Ujfalusi
2014-02-27 10:59 ` Alexander Shiyan
0 siblings, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2014-02-27 9:24 UTC (permalink / raw)
To: Roger Quadros
Cc: Alexander Shiyan, Tony Lindgren, Peter Ujfalusi, linux-gpio,
Alexandre Courbot
On Mon, Feb 24, 2014 at 4:34 PM, Roger Quadros <rogerq@ti.com> wrote:
> On 02/24/2014 04:25 PM, Linus Walleij wrote:
>> On Sat, Feb 15, 2014 at 2:34 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
>>
>>> - status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>>> - TWL4030_LED_LEDEN_REG);
>>> +
>>> + twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>>> + TWL4030_LED_LEDEN_REG);
>>
>> Isn't the right fix to actually *check* this status instead?
>>
>> TI dudes?
>
> Yes we should check for error. But the only action we can take is maybe print an error message
> as all the users of this function return void. e.g. twl_set().
>
> It seems the set() hook of struct gpio_chip also doesn't expect any return value. Wondering if that should change.
I think in this case simply printing an error and bailing out is
just fine, people will be able to debug from there.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gpio: twl4030: Remove redundant assignment
2014-02-24 14:25 ` Linus Walleij
@ 2014-02-24 15:34 ` Roger Quadros
2014-02-27 9:24 ` Linus Walleij
0 siblings, 1 reply; 8+ messages in thread
From: Roger Quadros @ 2014-02-24 15:34 UTC (permalink / raw)
To: Linus Walleij, Alexander Shiyan, Tony Lindgren, Peter Ujfalusi
Cc: linux-gpio, Alexandre Courbot
Hi,
On 02/24/2014 04:25 PM, Linus Walleij wrote:
> On Sat, Feb 15, 2014 at 2:34 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
>
>> Variable "status" is never used, so remove it.
>>
>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>> ---
>> drivers/gpio/gpio-twl4030.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
>> index 8b88ca2..fa15beb 100644
>> --- a/drivers/gpio/gpio-twl4030.c
>> +++ b/drivers/gpio/gpio-twl4030.c
>> @@ -139,7 +139,6 @@ static u8 cached_leden;
>> static void twl4030_led_set_value(int led, int value)
>> {
>> u8 mask = LEDEN_LEDAON | LEDEN_LEDAPWM;
>> - int status;
>>
>> if (led)
>> mask <<= 1;
>> @@ -148,8 +147,9 @@ static void twl4030_led_set_value(int led, int value)
>> cached_leden &= ~mask;
>> else
>> cached_leden |= mask;
>> - status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>> - TWL4030_LED_LEDEN_REG);
>> +
>> + twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
>> + TWL4030_LED_LEDEN_REG);
>
> Isn't the right fix to actually *check* this status instead?
>
> TI dudes?
Yes we should check for error. But the only action we can take is maybe print an error message
as all the users of this function return void. e.g. twl_set().
It seems the set() hook of struct gpio_chip also doesn't expect any return value. Wondering if that should change.
cheers,
-roger
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gpio: twl4030: Remove redundant assignment
2014-02-15 13:34 Alexander Shiyan
@ 2014-02-24 14:25 ` Linus Walleij
2014-02-24 15:34 ` Roger Quadros
0 siblings, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2014-02-24 14:25 UTC (permalink / raw)
To: Alexander Shiyan, Roger Quadros, Tony Lindgren, Peter Ujfalusi
Cc: linux-gpio, Alexandre Courbot
On Sat, Feb 15, 2014 at 2:34 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Variable "status" is never used, so remove it.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> drivers/gpio/gpio-twl4030.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
> index 8b88ca2..fa15beb 100644
> --- a/drivers/gpio/gpio-twl4030.c
> +++ b/drivers/gpio/gpio-twl4030.c
> @@ -139,7 +139,6 @@ static u8 cached_leden;
> static void twl4030_led_set_value(int led, int value)
> {
> u8 mask = LEDEN_LEDAON | LEDEN_LEDAPWM;
> - int status;
>
> if (led)
> mask <<= 1;
> @@ -148,8 +147,9 @@ static void twl4030_led_set_value(int led, int value)
> cached_leden &= ~mask;
> else
> cached_leden |= mask;
> - status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
> - TWL4030_LED_LEDEN_REG);
> +
> + twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
> + TWL4030_LED_LEDEN_REG);
Isn't the right fix to actually *check* this status instead?
TI dudes?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] gpio: twl4030: Remove redundant assignment
@ 2014-02-15 13:34 Alexander Shiyan
2014-02-24 14:25 ` Linus Walleij
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Shiyan @ 2014-02-15 13:34 UTC (permalink / raw)
To: linux-gpio; +Cc: Linus Walleij, Alexandre Courbot, Alexander Shiyan
Variable "status" is never used, so remove it.
Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
drivers/gpio/gpio-twl4030.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index 8b88ca2..fa15beb 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -139,7 +139,6 @@ static u8 cached_leden;
static void twl4030_led_set_value(int led, int value)
{
u8 mask = LEDEN_LEDAON | LEDEN_LEDAPWM;
- int status;
if (led)
mask <<= 1;
@@ -148,8 +147,9 @@ static void twl4030_led_set_value(int led, int value)
cached_leden &= ~mask;
else
cached_leden |= mask;
- status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
- TWL4030_LED_LEDEN_REG);
+
+ twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
+ TWL4030_LED_LEDEN_REG);
}
static int twl4030_set_gpio_direction(int gpio, int is_input)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-03-27 9:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-22 8:32 [PATCH] gpio: twl4030: Remove redundant assignment Alexander Shiyan
2014-03-27 9:07 ` Linus Walleij
-- strict thread matches above, loose matches on Subject: below --
2014-02-15 13:34 Alexander Shiyan
2014-02-24 14:25 ` Linus Walleij
2014-02-24 15:34 ` Roger Quadros
2014-02-27 9:24 ` Linus Walleij
2014-02-27 9:32 ` Peter Ujfalusi
2014-02-27 10:59 ` Alexander Shiyan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.