All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Paul Kocialkowski <contact@paulk.fr>, linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org, Alexandre Courbot <gnurou@gmail.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
Date: Wed, 21 Sep 2016 08:30:53 +0100	[thread overview]
Message-ID: <90828c8d-e46d-0956-d6b3-e88fc90f3049@nvidia.com> (raw)
In-Reply-To: <1474394567.1215.14.camel@paulk.fr>


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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Jon Hunter <jonathanh@nvidia.com>
To: Paul Kocialkowski <contact@paulk.fr>, <linux-kernel@vger.kernel.org>
Cc: <devicetree@vger.kernel.org>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	<linux-tegra@vger.kernel.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
Date: Wed, 21 Sep 2016 08:30:53 +0100	[thread overview]
Message-ID: <90828c8d-e46d-0956-d6b3-e88fc90f3049@nvidia.com> (raw)
In-Reply-To: <1474394567.1215.14.camel@paulk.fr>


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

WARNING: multiple messages have this Message-ID (diff)
From: jonathanh@nvidia.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
Date: Wed, 21 Sep 2016 08:30:53 +0100	[thread overview]
Message-ID: <90828c8d-e46d-0956-d6b3-e88fc90f3049@nvidia.com> (raw)
In-Reply-To: <1474394567.1215.14.camel@paulk.fr>


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

  reply	other threads:[~2016-09-21  7:30 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-28 17:32 [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions Paul Kocialkowski
2016-08-28 17:32 ` Paul Kocialkowski
2016-08-28 17:32 ` Paul Kocialkowski
2016-08-28 17:32 ` [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
     [not found]   ` <20160828173246.32621-2-contact-W9ppeneeCTY@public.gmane.org>
2016-09-20 17:40     ` Jon Hunter
2016-09-20 17:40       ` Jon Hunter
2016-09-20 17:40       ` Jon Hunter
2016-09-20 18:02       ` Paul Kocialkowski
2016-09-20 18:02         ` Paul Kocialkowski
2016-09-21  7:30         ` Jon Hunter [this message]
2016-09-21  7:30           ` Jon Hunter
2016-09-21  7:30           ` Jon Hunter
2016-09-21  7:56           ` Paul Kocialkowski
2016-09-21  7:56             ` Paul Kocialkowski
2016-09-21  7:56             ` Paul Kocialkowski
2016-09-21 10:10             ` Jon Hunter
2016-09-21 10:10               ` Jon Hunter
2016-09-21 10:10               ` Jon Hunter
2016-09-21 11:03               ` Paul Kocialkowski
2016-09-21 11:03                 ` Paul Kocialkowski
2016-08-28 17:32 ` [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
2016-09-20 17:41   ` Jon Hunter
2016-09-20 17:41     ` Jon Hunter
2016-09-20 17:41     ` Jon Hunter
     [not found]     ` <a9485a88-55a5-9dd1-1a1b-a6c0a78ff476-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-09-20 17:53       ` Paul Kocialkowski
2016-09-20 17:53         ` Paul Kocialkowski
2016-09-20 17:53         ` Paul Kocialkowski
2016-09-20 17:56         ` Jon Hunter
2016-09-20 17:56           ` Jon Hunter
2016-09-20 17:56           ` Jon Hunter
     [not found]           ` <d05b8e5a-adf7-9057-b42a-0da1fde84953-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-09-20 18:02             ` Paul Kocialkowski
2016-09-20 18:02               ` Paul Kocialkowski
2016-09-20 18:02               ` Paul Kocialkowski
2016-09-21  7:34               ` Jon Hunter
2016-09-21  7:34                 ` Jon Hunter
2016-09-21  7:34                 ` Jon Hunter
     [not found]                 ` <6501341f-e14c-4876-dcb7-60a33b7621c4-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-09-21  7:43                   ` Paul Kocialkowski
2016-09-21  7:43                     ` Paul Kocialkowski
2016-09-21  7:43                     ` Paul Kocialkowski
2016-09-21  9:15                     ` Jon Hunter
2016-09-21  9:15                       ` Jon Hunter
2016-09-21  9:15                       ` Jon Hunter
2016-09-21  9:31                       ` Paul Kocialkowski
2016-09-21  9:31                         ` Paul Kocialkowski
2016-08-28 17:32 ` [PATCH 4/4] ARM: tegra: nyan-blaze: " Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
2016-08-28 17:32   ` Paul Kocialkowski
     [not found]   ` <20160828173246.32621-4-contact-W9ppeneeCTY@public.gmane.org>
2016-09-20 17:42     ` Jon Hunter
2016-09-20 17:42       ` Jon Hunter
2016-09-20 17:42       ` Jon Hunter
     [not found] ` <20160828173246.32621-1-contact-W9ppeneeCTY@public.gmane.org>
2016-09-20 17:15   ` [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions Jon Hunter
2016-09-20 17:15     ` Jon Hunter
2016-09-20 17:15     ` Jon Hunter
     [not found]     ` <ef69b6d2-ce6b-ff76-5c04-ab2176eb6ecc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-09-20 18:14       ` Paul Kocialkowski
2016-09-20 18:14         ` Paul Kocialkowski
2016-09-20 18:14         ` Paul Kocialkowski
     [not found]         ` <1474395289.1215.20.camel-W9ppeneeCTY@public.gmane.org>
2016-09-21  7:52           ` Jon Hunter
2016-09-21  7:52             ` Jon Hunter
2016-09-21  7:52             ` Jon Hunter
     [not found]             ` <0ca614bf-4d27-9948-03e3-472d059f2153-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-09-21  8:26               ` Paul Kocialkowski
2016-09-21  8:26                 ` Paul Kocialkowski
2016-09-21  8:26                 ` Paul Kocialkowski
     [not found]                 ` <1474446366.1239.17.camel-W9ppeneeCTY@public.gmane.org>
2016-09-21  9:06                   ` Jon Hunter
2016-09-21  9:06                     ` Jon Hunter
2016-09-21  9:06                     ` Jon Hunter
2016-09-21  9:31                     ` Paul Kocialkowski
2016-09-21  9:31                       ` Paul Kocialkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=90828c8d-e46d-0956-d6b3-e88fc90f3049@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=contact@paulk.fr \
    --cc=devicetree@vger.kernel.org \
    --cc=gnurou@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.