| Message ID | 1433200158-6890-4-git-send-email-vz@mleia.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 02, 2015 at 02:09:15AM +0300, Vladimir Zapolskiy wrote: > The change cleans up gpiolib callbacks, and the main intention is to > remove bitwise operations on GPIO level value as a preceding change to > switch gpiolib callbacks to utilize bool type to represent GPIO level. > > No functional change. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com> > Cc: Lars-Peter Clausen <lars@metafoo.de> > Cc: Axel Lin <axel.lin@ingics.com> > Cc: patches@opensource.wolfsonmicro.com > --- Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com> Thanks, Charles -- 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
On Tue, Jun 02, 2015 at 02:09:15AM +0300, Vladimir Zapolskiy wrote: > @@ -1783,18 +1783,13 @@ static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) > { > struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); > unsigned int mask, val; > - int ret; > > mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK; > val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GPn_FN_SHIFT) | > WM8903_GPn_DIR; > > - ret = regmap_update_bits(wm8903->regmap, > - WM8903_GPIO_CONTROL_1 + offset, mask, val); > - if (ret < 0) > - return ret; > - > - return 0; > + return regmap_update_bits(wm8903->regmap, > + WM8903_GPIO_CONTROL_1 + offset, mask, val); > } > > static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) This appears to be an unrelated coding style change.
Hello Mark, On 02.06.2015 22:41, Mark Brown wrote: > On Tue, Jun 02, 2015 at 02:09:15AM +0300, Vladimir Zapolskiy wrote: > >> @@ -1783,18 +1783,13 @@ static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) >> { >> struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); >> unsigned int mask, val; >> - int ret; >> >> mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK; >> val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GPn_FN_SHIFT) | >> WM8903_GPn_DIR; >> >> - ret = regmap_update_bits(wm8903->regmap, >> - WM8903_GPIO_CONTROL_1 + offset, mask, val); >> - if (ret < 0) >> - return ret; >> - >> - return 0; >> + return regmap_update_bits(wm8903->regmap, >> + WM8903_GPIO_CONTROL_1 + offset, mask, val); >> } >> >> static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) > > This appears to be an unrelated coding style change. > this particular patch is named "simplify gpiolib callbacks". Do you prefer to separate the change here in .direction_in implementation from the rest? -- With best wishes, Vladimir -- 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
On Tue, Jun 02, 2015 at 11:18:23PM +0300, Vladimir Zapolskiy wrote: > On 02.06.2015 22:41, Mark Brown wrote: > > On Tue, Jun 02, 2015 at 02:09:15AM +0300, Vladimir Zapolskiy wrote: > >> @@ -1783,18 +1783,13 @@ static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) > >> { > >> struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); > >> unsigned int mask, val; > >> - int ret; > >> > >> mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK; > >> val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GPn_FN_SHIFT) | > >> WM8903_GPn_DIR; > >> > >> - ret = regmap_update_bits(wm8903->regmap, > >> - WM8903_GPIO_CONTROL_1 + offset, mask, val); > >> - if (ret < 0) > >> - return ret; > >> - > >> - return 0; > >> + return regmap_update_bits(wm8903->regmap, > >> + WM8903_GPIO_CONTROL_1 + offset, mask, val); > >> } > >> > >> static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) > > This appears to be an unrelated coding style change. > this particular patch is named "simplify gpiolib callbacks". > Do you prefer to separate the change here in .direction_in > implementation from the rest? It doesn't appear to make any changes related to boolean variables (AFAICT it's just removing the ret variable) so it shouldn't be in a change about boolean variables.
On 02.06.2015 23:31, Mark Brown wrote: > On Tue, Jun 02, 2015 at 11:18:23PM +0300, Vladimir Zapolskiy wrote: >> On 02.06.2015 22:41, Mark Brown wrote: >>> On Tue, Jun 02, 2015 at 02:09:15AM +0300, Vladimir Zapolskiy wrote: > >>>> @@ -1783,18 +1783,13 @@ static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) >>>> { >>>> struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); >>>> unsigned int mask, val; >>>> - int ret; >>>> >>>> mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK; >>>> val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GPn_FN_SHIFT) | >>>> WM8903_GPn_DIR; >>>> >>>> - ret = regmap_update_bits(wm8903->regmap, >>>> - WM8903_GPIO_CONTROL_1 + offset, mask, val); >>>> - if (ret < 0) >>>> - return ret; >>>> - >>>> - return 0; >>>> + return regmap_update_bits(wm8903->regmap, >>>> + WM8903_GPIO_CONTROL_1 + offset, mask, val); >>>> } >>>> >>>> static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) > >>> This appears to be an unrelated coding style change. > >> this particular patch is named "simplify gpiolib callbacks". > >> Do you prefer to separate the change here in .direction_in >> implementation from the rest? > > It doesn't appear to make any changes related to boolean variables > (AFAICT it's just removing the ret variable) so it shouldn't be in a > change about boolean variables. > Okay, will split the change then. -- With best wishes, Vladimir -- 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
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c index 93f1ce0..efac26b 100644 --- a/sound/soc/codecs/wm8903.c +++ b/sound/soc/codecs/wm8903.c @@ -1783,18 +1783,13 @@ static int wm8903_gpio_direction_in(struct gpio_chip *chip, unsigned offset) { struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); unsigned int mask, val; - int ret; mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK; val = (WM8903_GPn_FN_GPIO_INPUT << WM8903_GPn_FN_SHIFT) | WM8903_GPn_DIR; - ret = regmap_update_bits(wm8903->regmap, - WM8903_GPIO_CONTROL_1 + offset, mask, val); - if (ret < 0) - return ret; - - return 0; + return regmap_update_bits(wm8903->regmap, + WM8903_GPIO_CONTROL_1 + offset, mask, val); } static int wm8903_gpio_get(struct gpio_chip *chip, unsigned offset) @@ -1812,27 +1807,26 @@ static int wm8903_gpio_direction_out(struct gpio_chip *chip, { struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); unsigned int mask, val; - int ret; mask = WM8903_GPn_FN_MASK | WM8903_GPn_DIR_MASK | WM8903_GPn_LVL_MASK; - val = (WM8903_GPn_FN_GPIO_OUTPUT << WM8903_GPn_FN_SHIFT) | - (value << WM8903_GPn_LVL_SHIFT); + val = WM8903_GPn_FN_GPIO_OUTPUT << WM8903_GPn_FN_SHIFT; + if (value) + val |= 0x1 << WM8903_GPn_LVL_SHIFT; - ret = regmap_update_bits(wm8903->regmap, - WM8903_GPIO_CONTROL_1 + offset, mask, val); - if (ret < 0) - return ret; - - return 0; + return regmap_update_bits(wm8903->regmap, + WM8903_GPIO_CONTROL_1 + offset, mask, val); } static void wm8903_gpio_set(struct gpio_chip *chip, unsigned offset, int value) { struct wm8903_priv *wm8903 = gpio_to_wm8903(chip); + unsigned int val = 0; + + if (value) + val = 0x1 << WM8903_GPn_LVL_SHIFT; regmap_update_bits(wm8903->regmap, WM8903_GPIO_CONTROL_1 + offset, - WM8903_GPn_LVL_MASK, - !!value << WM8903_GPn_LVL_SHIFT); + WM8903_GPn_LVL_MASK, val); } static struct gpio_chip wm8903_template_chip = {
The change cleans up gpiolib callbacks, and the main intention is to remove bitwise operations on GPIO level value as a preceding change to switch gpiolib callbacks to utilize bool type to represent GPIO level. No functional change. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com> Cc: Lars-Peter Clausen <lars@metafoo.de> Cc: Axel Lin <axel.lin@ingics.com> Cc: patches@opensource.wolfsonmicro.com --- sound/soc/codecs/wm8903.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)