On 01/23/15 16:54, Bjorn Andersson wrote:
> +
> +static int pm8941_wled_set(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + struct pm8941_wled *wled;
> + u8 ctrl = 0;
> + u16 val;
> + int rc;
> + int i;
> +
> + wled = container_of(cdev, struct pm8941_wled, cdev);
> +
> + if (value != 0)
> + ctrl = PM8941_WLED_REG_MOD_EN_BIT;
> +
> + val = value * PM8941_WLED_REG_VAL_MAX / LED_FULL;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->addr + PM8941_WLED_REG_MOD_EN,
> + PM8941_WLED_REG_MOD_EN_MASK, ctrl);
> + if (rc)
> + return rc;
> +
> + for (i = 0; i < wled->cfg.num_strings; ++i) {
> + u8 v[2] = { val & 0xff, (val >> 8) & 0xf };
> +
> + rc = regmap_bulk_write(wled->regmap,
> + wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i,
> + v, 2);
> + if (rc)
> + return rc;
> + }
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->addr + PM8941_WLED_REG_SYNC,
> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL);
> + if (rc)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->addr + PM8941_WLED_REG_SYNC,
> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR);
> + return rc;
> +}
This doesn't seem to do anything for the OVP spike mentioned in this
patch[1]. Do you see that problem on your device? I imagine the PMIC is
the same.
[1]
https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/?id=fef9e15072562f0f28dc7066dcdd69388df81ed3
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Thu, Feb 12, 2015 at 8:26 PM, Stephen Boyd <[email protected]> wrote:
> On 01/23/15 16:54, Bjorn Andersson wrote:
>> +
>> +static int pm8941_wled_set(struct led_classdev *cdev,
>> + enum led_brightness value)
>> +{
>> + struct pm8941_wled *wled;
>> + u8 ctrl = 0;
>> + u16 val;
>> + int rc;
>> + int i;
>> +
>> + wled = container_of(cdev, struct pm8941_wled, cdev);
>> +
>> + if (value != 0)
>> + ctrl = PM8941_WLED_REG_MOD_EN_BIT;
>> +
>> + val = value * PM8941_WLED_REG_VAL_MAX / LED_FULL;
>> +
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->addr + PM8941_WLED_REG_MOD_EN,
>> + PM8941_WLED_REG_MOD_EN_MASK, ctrl);
>> + if (rc)
>> + return rc;
>> +
>> + for (i = 0; i < wled->cfg.num_strings; ++i) {
>> + u8 v[2] = { val & 0xff, (val >> 8) & 0xf };
>> +
>> + rc = regmap_bulk_write(wled->regmap,
>> + wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i,
>> + v, 2);
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->addr + PM8941_WLED_REG_SYNC,
>> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL);
>> + if (rc)
>> + return rc;
>> +
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->addr + PM8941_WLED_REG_SYNC,
>> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR);
>> + return rc;
>> +}
>
> This doesn't seem to do anything for the OVP spike mentioned in this
> patch[1]. Do you see that problem on your device? I imagine the PMIC is
> the same.
>
> [1]
> https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/?id=fef9e15072562f0f28dc7066dcdd69388df81ed3
>
That's odd, I can't find that fix in any officially supported releases
for 8974 - which Courtney used as reference for this driver.
Just to make sure I understand the solution; when disabling the sinks
the over-voltage-protection sometimes triggers, so we should detect
that and reset the ovp configuration?
I presume the side effect would be that the sinks would not give any
output until this is done?
Regards,
Bjorn
On 02/17/15 15:02, Bjorn Andersson wrote:
> On Thu, Feb 12, 2015 at 8:26 PM, Stephen Boyd <[email protected]> wrote:
>> On 01/23/15 16:54, Bjorn Andersson wrote:
>>> +
>>> +static int pm8941_wled_set(struct led_classdev *cdev,
>>> + enum led_brightness value)
>>> +{
>>> + struct pm8941_wled *wled;
>>> + u8 ctrl = 0;
>>> + u16 val;
>>> + int rc;
>>> + int i;
>>> +
>>> + wled = container_of(cdev, struct pm8941_wled, cdev);
>>> +
>>> + if (value != 0)
>>> + ctrl = PM8941_WLED_REG_MOD_EN_BIT;
>>> +
>>> + val = value * PM8941_WLED_REG_VAL_MAX / LED_FULL;
>>> +
>>> + rc = regmap_update_bits(wled->regmap,
>>> + wled->addr + PM8941_WLED_REG_MOD_EN,
>>> + PM8941_WLED_REG_MOD_EN_MASK, ctrl);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + for (i = 0; i < wled->cfg.num_strings; ++i) {
>>> + u8 v[2] = { val & 0xff, (val >> 8) & 0xf };
>>> +
>>> + rc = regmap_bulk_write(wled->regmap,
>>> + wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i,
>>> + v, 2);
>>> + if (rc)
>>> + return rc;
>>> + }
>>> +
>>> + rc = regmap_update_bits(wled->regmap,
>>> + wled->addr + PM8941_WLED_REG_SYNC,
>>> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + rc = regmap_update_bits(wled->regmap,
>>> + wled->addr + PM8941_WLED_REG_SYNC,
>>> + PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR);
>>> + return rc;
>>> +}
>> This doesn't seem to do anything for the OVP spike mentioned in this
>> patch[1]. Do you see that problem on your device? I imagine the PMIC is
>> the same.
>>
>> [1]
>> https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/?id=fef9e15072562f0f28dc7066dcdd69388df81ed3
>>
> That's odd, I can't find that fix in any officially supported releases
> for 8974 - which Courtney used as reference for this driver.
>
> Just to make sure I understand the solution; when disabling the sinks
> the over-voltage-protection sometimes triggers, so we should detect
> that and reset the ovp configuration?
>
> I presume the side effect would be that the sinks would not give any
> output until this is done?
>
It seems that commit is not very good at describing the problem. From
what I can tell, the inductor current may spike when the WLED is turned
off and the switch is stuck on (the circuit is a boost convertor). To
make sure this doesn't happen, we force an OVP so that the switch is
known to be open (i.e. off) and thus can't cause the current spike when
we disable.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, Feb 17, 2015 at 6:45 PM, Stephen Boyd <[email protected]> wrote:
> On 02/17/15 15:02, Bjorn Andersson wrote:
>> On Thu, Feb 12, 2015 at 8:26 PM, Stephen Boyd <[email protected]> wrote:
[..]
>>> This doesn't seem to do anything for the OVP spike mentioned in this
>>> patch[1]. Do you see that problem on your device? I imagine the PMIC is
>>> the same.
>>>
>>> [1]
>>> https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/?id=fef9e15072562f0f28dc7066dcdd69388df81ed3
>>>
>> That's odd, I can't find that fix in any officially supported releases
>> for 8974 - which Courtney used as reference for this driver.
>>
>> Just to make sure I understand the solution; when disabling the sinks
>> the over-voltage-protection sometimes triggers, so we should detect
>> that and reset the ovp configuration?
>>
>> I presume the side effect would be that the sinks would not give any
>> output until this is done?
>>
>
> It seems that commit is not very good at describing the problem. From
> what I can tell, the inductor current may spike when the WLED is turned
> off and the switch is stuck on (the circuit is a boost convertor). To
> make sure this doesn't happen, we force an OVP so that the switch is
> known to be open (i.e. off) and thus can't cause the current spike when
> we disable.
>
Apparently that commit is only half of the solution, there's another
commit earlier in history with the same commit message [2].
With both [1] and [2] in place there are a bunch of different
operations going on, so I'm not able to fully map these to your
description - which does make sense.
[2] https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/?id=b1a2f61113c69d2aba68c583101ac40a05cb2c5b
Regards,
Bjorn