Re: [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Jon Hunter]

On 20/09/16 19:02, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 Ã 18:40 +0100, Jon Hunter a Ãcrit :
>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>  
>>> Nyan boards come with an embedded controller that controls when to
>>> enable and disable the charge. Thus, it should not be left up to the
>>> kernel to handle that.
>>>  
>>> Using the ti,external-control property allows specifying this use-case.
>>  
>> So the bq24735 is populated under the EC's 'i2c-tunnel' property which
>> is there to specifically interface it's child devices to the host. So I
>> am a bit confused why this is expose to the host if it should not be used?
> 
> Well, it needs to access the information in the read-only registers provided by
> the chip, which is allowed by the setup in place that you described.

Is this to expose the current state to the kernel so we can monitor the
battery state?

> However, the EC has its internal state machine that decides when to start
> charging, etc and so should be the only one to write registers, to avoid
> conflicts.
> 
>> Again you may right and I did find the original series [0] for this
>> which specifically references the Acer Chromebook that needs this.
>> However, I am not sure why this was never populated? Is there any other
>> history here?
> 
> I am also confused about why it wasn't applied earlier. However, the cros kernel
> is using the very same scheme.

Do you have a reference?

>> What is the actual problem you see without making this change?
> 
> There is a risk of conflict (even though it's probably not that significant),
> given the low variety of possible cases here. The idea is simply to say that the
> EC is in charge and to let it do its job without interfering.
> 
>> The original series states ...
>>  
>> "On Acer Chromebook 13 (CB5-311) this module fails to load if the
>> charger is not inserted, and will error when it is removed."
> 
> I'm confused about that comment. At this point (and with this patch), it works
> normally.

Ok, I think Thierry prefers to only apply fixes for problems that can be
reproduced. Is there a simple way to check the battery status and
charging status via say the sysfs? If I can test that this has no
negative impact may be it is ok.

Cheers
Jon

-- 
nvpublic