On Sun, 9 Oct 2016, Alexander Shiyan wrote:
> >Воскресенье, 9 октября 2016, 16:37 +03:00 от Alexander Kurz <akurz@xxxxxxxx>:
> >
> >Fix the register access shift argument calculation introduced with
> >commit a59ce6584d56 ("leds: leds-mc13783: Add MC34708 LED support")
> >and re-enable access to the "keypad" led for MC13892 MFC devices.
> >
> >Signed-off-by: Alexander Kurz < akurz@xxxxxxxx >
> >---
> > drivers/leds/leds-mc13783.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/leds/leds-mc13783.c b/drivers/leds/leds-mc13783.c
> >index a2e4c17..2421cf10 100644
> >--- a/drivers/leds/leds-mc13783.c
> >+++ b/drivers/leds/leds-mc13783.c
> >@@ -84,8 +84,9 @@ static int mc13xxx_led_set(struct led_classdev *led_cdev,
> > case MC13892_LED_MD:
> > case MC13892_LED_AD:
> > case MC13892_LED_KP:
> >-reg = (led->id - MC13892_LED_MD) / 2;
> >-shift = 3 + (led->id - MC13892_LED_MD) * 12;
> >+off = led->id - MC13892_LED_MD;
> >+reg = off / 2;
> >+shift = 3 + (off - reg * 2) * 12;
> > break;
> > case MC13892_LED_R:
> > case MC13892_LED_G:
>
> I am suspect the next caclulation is more readable:
>
> reg = (led->id - MC13892_LED_MD) / 2;
> off = ((led->id - MC13892_LED_MD) % 2) * 12;
> shift = 3 + off;
Probably yes, but it introduces different styles of calculating
basically the same data in this function (see some lines above and
below the patched lines / not visible in the patch context)
I just kept it to a uniform style.
