Re: [PATCH v3 4/4] power: bq24190_charger: Delay before polling reset flag

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 02-04-17 22:16, Liam Breck wrote:
> On Sun, Apr 2, 2017 at 5:29 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 01-04-17 22:25, Liam Breck wrote:
>>>
>>> From: Liam Breck <kernel@networkimprov.net>
>>>
>>> On chip reset, polling loop was reading reset register immediately.
>>> Instead, call udelay() before reading chip register.
>>
>>
>> Why ? What does this buy us ?
>>
>> Also I've yet to hear back from you on my patch to remove doing
>> resets of the device altogether have you find any bad side-effects
>> of that patch in your testing ?
>
> On a DT-configured device, we don't want any charger settings done
> prior to probe(), as we are going to apply DT values. So
> register_reset() on probe seems right.

If no charger settings are done prior to probe, why bother with
a reset ? "seems right" does not seem like a good argument when
reset has shown to cause problems in real world scenarios and
as I explained in my commit msg there really are no strong
arguments in favor of doing the reset and several against it.

If you really really want to keep doing a reset for no good
reasons then we could introduce a flag which DT can set to
force a reset on probe, but IMHO the reset should just be
removed all together.

> If that can trigger a charger
> bug, we should find a way to make the charger recover.
>
> I will test for the possible bug you described when I have a chance;
> I'm occupied with other stuff near-future. For the time being, let's
> use the no_register_reset property from your first patchset.

As I said above I really see nothing gained from doing the
reset and "seems right" is not really a good technical argument
for keeping it.

Regards,

Hans