Hi both!
Please trim your replies (removing code you are not commenting
on). Scolling 600 lines to find where discussion is is not fun.
Best regards,
Pavel
> >>>+static int qcom_flash_torch_on(struct qcom_flash_led *led)
> >>>+{
> >>>+ int rc, error;
> >>>+ struct qcom_flash_device *leds_dev = led_to_leds_dev(led);
> >>>+ struct device *dev = leds_dev->dev;
> >>>+
> >>>+ if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_DUAL) {
> >>>+ rc = qcom_flash_torch_regulator_on(leds_dev);
> >>>+ if (rc)
> >>>+ goto error_reg_write;
> >>>+ } else if (leds_dev->peripheral_subtype == QCOM_FLASH_SUBTYPE_SINGLE) {
> >>>+ rc = qcom_flash_fled_regulator_on(leds_dev);
> >>
> >>Why for torch mode you need to enable fled regulator?
> >
> >Based on [1], apparently the hardware present in the Single variant of the PMIC
> >has some limitation that requires the use of the flash regulator and the value
> >QCOM_FLASH_ENABLE_ALL to enable the LEDs for the torch mode. The Dual variant on
> >the other hand can just use the torch regulator and enables the LEDs with
> >QCOM_FLASH_ENABLE_MODULE.
> >
> >[1] https://github.com/AICP/kernel_lge_hammerhead/commit/0f47c747c074993655d0bfebd045e8ddd228fe4c
> >
> >I'm honestly not sure what the impact is on using the different regulators and
> >enable values. I have tested enabling the Dual PMIC with different enable values
> >and all seemed to work the same, so must be some hardware detail.
> >
> >I left that Single codepath in the hope that it is useful for devices that have
> >that variant of the hardware, but I have only actually tested the Dual PMIC,
> >which is the one present on the Nexus 5.
>
> Thanks for the explanation. Just wanted to confirm that it was not
> a mistake.
>
> >>
> >>>+ if (rc)
> >>>+ goto error_flash_set;
> >>>+
> >>>+ /*
> >>>+ * Write 0x80 to MODULE_ENABLE before writing
> >>>+ * 0xE0 in order to avoid a hardware bug caused
> >>>+ * by register value going from 0x00 to 0xE0.
> >>>+ */
> >>>+ rc = qcom_flash_masked_write(leds_dev,
> >>>+ QCOM_FLASH_ADDR_ENABLE_CONTROL,
> >>>+ QCOM_FLASH_ENABLE_MODULE_MASK,
> >>>+ QCOM_FLASH_ENABLE_MODULE);
> >>>+ if (rc) {
> >>>+ dev_err(dev, "Enable reg write failed(%d)\n", rc);
> >>>+ goto error_flash_set;
> >>>+ }
> >>>+ }
> >>>+
> >>>+ rc = qcom_flash_torch_reg_enable(leds_dev, true);
> >>>+ if (rc)
> >>>+ goto error_reg_write;
> >>>+
> >>>+ rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_ENABLE_CONTROL,
> >>>+ QCOM_FLASH_ENABLE_MASK,
> >>>+ leds_dev->torch_enable_cmd);
> >>>+ if (rc) {
> >>>+ dev_err(dev, "Enable reg write failed(%d)\n", rc);
> >>>+ goto error_reg_write;
> >>>+ }
> >>>+
> >>>+ rc = qcom_flash_masked_write(leds_dev, QCOM_FLASH_ADDR_LED_STROBE_CTRL,
> >>>+ led->flash_strobe_cmd,
> >>>+ led->flash_strobe_cmd);
> >>
> >>Just to make sure - the hardware requires strobe cmd to enable torch?
> >
> >Yes. The strobe value is the one that actually turns each of the LEDs on,
> >doesn't matter if it's on flash or torch mode. The difference in torch mode is
> >actually just that the timeout on the LEDs is disabled (done by writing 0x00
> >into the TORCH, 0xE4, register).
> >So for both modes, the LEDs are turned on by writing to the STROBE_CTRL, 0x47,
> >register. If torch is on they'll stay on indefinitely, while on flash mode
> >they'll turn off after the timeout.
> >
> >Perhaps it's just a naming issue?
>
> I propose to add these comments next to the calls in question.
--
http://www.livejournal.com/~pavelmachek
Attachment:
signature.asc
Description: Digital signature
