Hi Alexander,
On 10/10/2016 02:52 PM, Alexander Kurz wrote:
Hi Jacek,
On Mon, 10 Oct 2016, Jacek Anaszewski wrote:
Hi Alexander,
Thanks for the patch.
On 10/09/2016 03:37 PM, Alexander Kurz wrote:
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:
While trying to analyze these calculations I noticed a discrepancy
between DT documentation and the enum in the header file.
Documentation/devicetree/bindings/mfd/mc13xxx.txt:
MC13892 LED IDs:
0 : Main display
1 : AUX display
2 : Keypad
3 : Red
4 : Green
5 : Blue
include/linux/mfd/mc13xxx.h:
enum {
/* MC13783 LED IDs */
MC13783_LED_MD,
MC13783_LED_AD,
MC13783_LED_KP,
MC13783_LED_R1,
MC13783_LED_G1,
MC13783_LED_B1,
MC13783_LED_R2,
MC13783_LED_G2,
MC13783_LED_B2,
MC13783_LED_R3,
MC13783_LED_G3,
MC13783_LED_B3,
/* MC13892 LED IDs */
MC13892_LED_MD,
MC13892_LED_AD,
MC13892_LED_KP,
MC13892_LED_R,
MC13892_LED_G,
MC13892_LED_B,
/* MC34708 LED IDs */
MC34708_LED_R,
MC34708_LED_G,
}
Keypad LED enum identifier is 15, but documentation states
that it is 2. The code seems to expect the former:
off = led->id - MC13892_LED_MD; //15 - 13 = 2
reg = off / 2; // 2 / 2 = 1
shift = 3 + (off - reg * 2) * 12; // 3 + (2 - 1*2) = 3
It should write value << 3 to the register 52.
According to the documentation [0] LEDKP0, LEDKP1 and LEDKP2
are driven by bits 9,10 and 11 respectively. The calculations
don't seem to be in line with that. Could you point what I
am missing?
[0] http://www.nxp.com/files/analog/doc/data_sheet/MC13892.pdf
The numbering mentioned in the DT documentation is used as offset
for the device specific struct mc13xxx_led_devtype.led_min
if (of_property_read_u32(child, "reg", &tmp))
continue;
pdata->led[i].id = leds->devtype->led_min + tmp;
e.g. MC13892_LED_KP = MC13892_LED_MD + 2
Hope this clarifies your question,
Thanks but not entirely - what with the second part of question related
to the register layout?
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
