| Message ID | 20191016073842.1300297-2-thierry.reding@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pwm: stm32: Minor cleanups | expand |
On Wed, Oct 16, 2019 at 09:38:40AM +0200, Thierry Reding wrote: > Remove usage of the ternary operator to assign values for register > fields. This removes clutter and improves readability. > > Signed-off-by: Thierry Reding <thierry.reding@gmail.com> > --- > drivers/pwm/pwm-stm32.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > index 9430b4cd383f..b12fb11b7a55 100644 > --- a/drivers/pwm/pwm-stm32.c > +++ b/drivers/pwm/pwm-stm32.c > @@ -493,11 +493,19 @@ static const struct pwm_ops stm32pwm_ops = { > static int stm32_pwm_set_breakinput(struct stm32_pwm *priv, > int index, int level, int filter) > { > - u32 bke = (index == 0) ? TIM_BDTR_BKE : TIM_BDTR_BK2E; > - int shift = (index == 0) ? TIM_BDTR_BKF_SHIFT : TIM_BDTR_BK2F_SHIFT; > - u32 mask = (index == 0) ? TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF > - : TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; > - u32 bdtr = bke; > + u32 bke, shift, mask, bdtr; > + > + if (index == 0) { > + bke = TIM_BDTR_BKE; > + shift = TIM_BDTR_BKF_SHIFT; > + mask = TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF; > + } else { > + bke = TIM_BDTR_BK2E; > + shift = TIM_BDTR_BK2F_SHIFT; > + mask = TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; > + } > + > + bdtr = bke; Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Is index always in {0, 1}? Maybe a comment or a check about that would be helpful. (-> separate patch I think). Best regards Uwe
On Wed, Oct 16, 2019 at 10:26:12AM +0200, Uwe Kleine-König wrote: > On Wed, Oct 16, 2019 at 09:38:40AM +0200, Thierry Reding wrote: > > Remove usage of the ternary operator to assign values for register > > fields. This removes clutter and improves readability. > > > > Signed-off-by: Thierry Reding <thierry.reding@gmail.com> > > --- > > drivers/pwm/pwm-stm32.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c > > index 9430b4cd383f..b12fb11b7a55 100644 > > --- a/drivers/pwm/pwm-stm32.c > > +++ b/drivers/pwm/pwm-stm32.c > > @@ -493,11 +493,19 @@ static const struct pwm_ops stm32pwm_ops = { > > static int stm32_pwm_set_breakinput(struct stm32_pwm *priv, > > int index, int level, int filter) > > { > > - u32 bke = (index == 0) ? TIM_BDTR_BKE : TIM_BDTR_BK2E; > > - int shift = (index == 0) ? TIM_BDTR_BKF_SHIFT : TIM_BDTR_BK2F_SHIFT; > > - u32 mask = (index == 0) ? TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF > > - : TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; > > - u32 bdtr = bke; > > + u32 bke, shift, mask, bdtr; > > + > > + if (index == 0) { > > + bke = TIM_BDTR_BKE; > > + shift = TIM_BDTR_BKF_SHIFT; > > + mask = TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF; > > + } else { > > + bke = TIM_BDTR_BK2E; > > + shift = TIM_BDTR_BK2F_SHIFT; > > + mask = TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; > > + } > > + > > + bdtr = bke; > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Is index always in {0, 1}? Maybe a comment or a check about that would > be helpful. (-> separate patch I think). The bindings say that index can only be 0 or 1. I guess strictly it might actually depend on the number of break inputs, but given these register definitions, there's only ever two. But yeah, it might be a good idea to sanitize the values upon probe. I'll add another patch to do that. Thierry
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 9430b4cd383f..b12fb11b7a55 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -493,11 +493,19 @@ static const struct pwm_ops stm32pwm_ops = { static int stm32_pwm_set_breakinput(struct stm32_pwm *priv, int index, int level, int filter) { - u32 bke = (index == 0) ? TIM_BDTR_BKE : TIM_BDTR_BK2E; - int shift = (index == 0) ? TIM_BDTR_BKF_SHIFT : TIM_BDTR_BK2F_SHIFT; - u32 mask = (index == 0) ? TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF - : TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; - u32 bdtr = bke; + u32 bke, shift, mask, bdtr; + + if (index == 0) { + bke = TIM_BDTR_BKE; + shift = TIM_BDTR_BKF_SHIFT; + mask = TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF; + } else { + bke = TIM_BDTR_BK2E; + shift = TIM_BDTR_BK2F_SHIFT; + mask = TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F; + } + + bdtr = bke; /* * The both bits could be set since only one will be wrote
Remove usage of the ternary operator to assign values for register fields. This removes clutter and improves readability. Signed-off-by: Thierry Reding <thierry.reding@gmail.com> --- drivers/pwm/pwm-stm32.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)