Re: [PATCH 3/4] leds-lp5523: change the return type of lp5523_set_mode()
On Tue, Aug 21, 2012 at 5:03 PM, Kim, Milo wrote: > The return value of this function is not handled any place, so > make it as void type. > > And three if-statements are replaced with switch-statements. > This one looks fine with me. I will merge it after you rework on first 2 patches Thanks, -Bryan > Signed-off-by: Milo(Woogyom) Kim > --- > drivers/leds/leds-lp5523.c | 24 +--- > 1 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c > index f090819..f8231f1 100644 > --- a/drivers/leds/leds-lp5523.c > +++ b/drivers/leds/leds-lp5523.c > @@ -150,7 +150,7 @@ static inline struct lp5523_chip *led_to_lp5523(struct > lp5523_led *led) > leds[led->id]); > } > > -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode); > +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode); > static int lp5523_set_engine_mode(struct lp5523_engine *engine, u8 mode); > static int lp5523_load_program(struct lp5523_engine *engine, const u8 > *pattern); > > @@ -789,26 +789,28 @@ static void lp5523_unregister_sysfs(struct i2c_client > *client) > /*--*/ > /* Set chip operating mode */ > /*--*/ > -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode) > +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode) > { > - int ret = 0; > - > /* if in that mode already do nothing, except for run */ > if (mode == engine->mode && mode != LP5523_CMD_RUN) > - return 0; > + return; > > - if (mode == LP5523_CMD_RUN) { > - ret = lp5523_run_program(engine); > - } else if (mode == LP5523_CMD_LOAD) { > + switch (mode) { > + case LP5523_CMD_RUN: > + lp5523_run_program(engine); > + break; > + case LP5523_CMD_LOAD: > lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); > lp5523_set_engine_mode(engine, LP5523_CMD_LOAD); > - } else if (mode == LP5523_CMD_DISABLED) { > + break; > + case LP5523_CMD_DISABLED: > lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); > + break; > + default: > + return; > } > > engine->mode = mode; > - > - return ret; > } > > /*--*/ > -- > 1.7.2.5 > > > Best Regards, > Milo > > -- Bryan Wu Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] leds-lp5523: change the return type of lp5523_set_mode()
The return value of this function is not handled any place, so make it as void type. And three if-statements are replaced with switch-statements. Signed-off-by: Milo(Woogyom) Kim --- drivers/leds/leds-lp5523.c | 24 +--- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c index f090819..f8231f1 100644 --- a/drivers/leds/leds-lp5523.c +++ b/drivers/leds/leds-lp5523.c @@ -150,7 +150,7 @@ static inline struct lp5523_chip *led_to_lp5523(struct lp5523_led *led) leds[led->id]); } -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode); +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_set_engine_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_load_program(struct lp5523_engine *engine, const u8 *pattern); @@ -789,26 +789,28 @@ static void lp5523_unregister_sysfs(struct i2c_client *client) /*--*/ /* Set chip operating mode */ /*--*/ -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode) +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode) { - int ret = 0; - /* if in that mode already do nothing, except for run */ if (mode == engine->mode && mode != LP5523_CMD_RUN) - return 0; + return; - if (mode == LP5523_CMD_RUN) { - ret = lp5523_run_program(engine); - } else if (mode == LP5523_CMD_LOAD) { + switch (mode) { + case LP5523_CMD_RUN: + lp5523_run_program(engine); + break; + case LP5523_CMD_LOAD: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); lp5523_set_engine_mode(engine, LP5523_CMD_LOAD); - } else if (mode == LP5523_CMD_DISABLED) { + break; + case LP5523_CMD_DISABLED: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); + break; + default: + return; } engine->mode = mode; - - return ret; } /*--*/ -- 1.7.2.5 Best Regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] leds-lp5523: change the return type of lp5523_set_mode()
On Tue, Aug 21, 2012 at 5:03 PM, Kim, Milo milo@ti.com wrote: The return value of this function is not handled any place, so make it as void type. And three if-statements are replaced with switch-statements. This one looks fine with me. I will merge it after you rework on first 2 patches Thanks, -Bryan Signed-off-by: Milo(Woogyom) Kim milo@ti.com --- drivers/leds/leds-lp5523.c | 24 +--- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c index f090819..f8231f1 100644 --- a/drivers/leds/leds-lp5523.c +++ b/drivers/leds/leds-lp5523.c @@ -150,7 +150,7 @@ static inline struct lp5523_chip *led_to_lp5523(struct lp5523_led *led) leds[led-id]); } -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode); +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_set_engine_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_load_program(struct lp5523_engine *engine, const u8 *pattern); @@ -789,26 +789,28 @@ static void lp5523_unregister_sysfs(struct i2c_client *client) /*--*/ /* Set chip operating mode */ /*--*/ -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode) +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode) { - int ret = 0; - /* if in that mode already do nothing, except for run */ if (mode == engine-mode mode != LP5523_CMD_RUN) - return 0; + return; - if (mode == LP5523_CMD_RUN) { - ret = lp5523_run_program(engine); - } else if (mode == LP5523_CMD_LOAD) { + switch (mode) { + case LP5523_CMD_RUN: + lp5523_run_program(engine); + break; + case LP5523_CMD_LOAD: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); lp5523_set_engine_mode(engine, LP5523_CMD_LOAD); - } else if (mode == LP5523_CMD_DISABLED) { + break; + case LP5523_CMD_DISABLED: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); + break; + default: + return; } engine-mode = mode; - - return ret; } /*--*/ -- 1.7.2.5 Best Regards, Milo -- Bryan Wu bryan...@canonical.com Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] leds-lp5523: change the return type of lp5523_set_mode()
The return value of this function is not handled any place, so make it as void type. And three if-statements are replaced with switch-statements. Signed-off-by: Milo(Woogyom) Kim milo@ti.com --- drivers/leds/leds-lp5523.c | 24 +--- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c index f090819..f8231f1 100644 --- a/drivers/leds/leds-lp5523.c +++ b/drivers/leds/leds-lp5523.c @@ -150,7 +150,7 @@ static inline struct lp5523_chip *led_to_lp5523(struct lp5523_led *led) leds[led-id]); } -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode); +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_set_engine_mode(struct lp5523_engine *engine, u8 mode); static int lp5523_load_program(struct lp5523_engine *engine, const u8 *pattern); @@ -789,26 +789,28 @@ static void lp5523_unregister_sysfs(struct i2c_client *client) /*--*/ /* Set chip operating mode */ /*--*/ -static int lp5523_set_mode(struct lp5523_engine *engine, u8 mode) +static void lp5523_set_mode(struct lp5523_engine *engine, u8 mode) { - int ret = 0; - /* if in that mode already do nothing, except for run */ if (mode == engine-mode mode != LP5523_CMD_RUN) - return 0; + return; - if (mode == LP5523_CMD_RUN) { - ret = lp5523_run_program(engine); - } else if (mode == LP5523_CMD_LOAD) { + switch (mode) { + case LP5523_CMD_RUN: + lp5523_run_program(engine); + break; + case LP5523_CMD_LOAD: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); lp5523_set_engine_mode(engine, LP5523_CMD_LOAD); - } else if (mode == LP5523_CMD_DISABLED) { + break; + case LP5523_CMD_DISABLED: lp5523_set_engine_mode(engine, LP5523_CMD_DISABLED); + break; + default: + return; } engine-mode = mode; - - return ret; } /*--*/ -- 1.7.2.5 Best Regards, Milo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/