All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adam Williamson <awilliam-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: "Li, Aubrey" <aubrey.li-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Lan Tianyu <tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org,
	lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org,
	robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	shigorin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	jan.brummer-sgYkR6nYkOfYtjvyW6yDsg@public.gmane.org,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org
Subject: Re: [PATCH V3] ACPI: Add _DEP(Operation Region Dependencies) support to fix battery issue on the Asus T100TA
Date: Sun, 23 Nov 2014 20:23:32 -0800	[thread overview]
Message-ID: <1416803012.2669.68.camel@redhat.com> (raw)
In-Reply-To: <54729A7B.4050007-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On Mon, 2014-11-24 at 10:39 +0800, Li, Aubrey wrote:
> On 2014/11/23 21:22, Lan Tianyu wrote:
> > ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> > assign a higher priority in start ordering due to future operation region
> > accesses.
> > 
> > On Asus T100TA, ACPI battery info are read from a I2C slave device via
> > I2C operation region. Before I2C operation region handler is installed,
> > battery _STA always returns 0. There is a _DEP method of designating
> > start order under battery device node.
> > 
> > This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> > Introducing acpi_dep_list and adding dep_unmet count in the struct
> > acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> > valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> > master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> > count will increase by one if there is a device under its _DEP. Driver's probe() should
> > return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> > region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> > whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> > When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> > for the device in order to resolve battery _STA issue on the Asus T100TA.
> 
> Well, Can we explicitly tied this up with ASUS T100TA in the code?
> If I understand correctly, the assumption in the patch is that the
> battery device only depends on I2C device, which is true on ASUS T100TA,
> but may not on the other platforms.
> 
> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
> 
> Device (BATC)
>         {
>             Name (_HID, EisaId ("PNP0C0A"))  // _HID: Hardware ID
> --------snip--------
>             Name (_DEP, Package (0x03)  // _DEP: Dependencies
>             {
>                 I2C1,
>                 GPO2,
>                 GPO0
>             })
> 
> Thanks,
> -Aubrey

It certainly works for more than *just* the T100 - fedlet users have
reported working battery status on at least the Miix 2 and Venue 8 Pro
(I can personally confirm it works on the Venue 8 Pro). However, Bastien
Nocera reported failure on an Onda v975w with a slightly earlier version
of the patch in the bug report -
https://bugzilla.kernel.org/show_bug.cgi?id=69011#c58 , "Tested with the
same patch on a Onda v975w, and it tries very hard to detect the battery
but fails."
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net

WARNING: multiple messages have this Message-ID (diff)
From: Adam Williamson <awilliam@redhat.com>
To: "Li, Aubrey" <aubrey.li@linux.intel.com>
Cc: Lan Tianyu <tianyu.lan@intel.com>,
	rjw@rjwysocki.net, lenb@kernel.org, wsa@the-dreams.de,
	robert.moore@intel.com, lv.zheng@intel.com, shigorin@gmail.com,
	jan.brummer@tabos.org, mika.westerberg@linux.intel.com,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org, devel@acpica.org
Subject: Re: [PATCH V3] ACPI: Add _DEP(Operation Region Dependencies) support to fix battery issue on the Asus T100TA
Date: Sun, 23 Nov 2014 20:23:32 -0800	[thread overview]
Message-ID: <1416803012.2669.68.camel@redhat.com> (raw)
In-Reply-To: <54729A7B.4050007@linux.intel.com>

On Mon, 2014-11-24 at 10:39 +0800, Li, Aubrey wrote:
> On 2014/11/23 21:22, Lan Tianyu wrote:
> > ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> > assign a higher priority in start ordering due to future operation region
> > accesses.
> > 
> > On Asus T100TA, ACPI battery info are read from a I2C slave device via
> > I2C operation region. Before I2C operation region handler is installed,
> > battery _STA always returns 0. There is a _DEP method of designating
> > start order under battery device node.
> > 
> > This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> > Introducing acpi_dep_list and adding dep_unmet count in the struct
> > acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> > valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> > master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> > count will increase by one if there is a device under its _DEP. Driver's probe() should
> > return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> > region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> > whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> > When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> > for the device in order to resolve battery _STA issue on the Asus T100TA.
> 
> Well, Can we explicitly tied this up with ASUS T100TA in the code?
> If I understand correctly, the assumption in the patch is that the
> battery device only depends on I2C device, which is true on ASUS T100TA,
> but may not on the other platforms.
> 
> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
> 
> Device (BATC)
>         {
>             Name (_HID, EisaId ("PNP0C0A"))  // _HID: Hardware ID
> --------snip--------
>             Name (_DEP, Package (0x03)  // _DEP: Dependencies
>             {
>                 I2C1,
>                 GPO2,
>                 GPO0
>             })
> 
> Thanks,
> -Aubrey

It certainly works for more than *just* the T100 - fedlet users have
reported working battery status on at least the Miix 2 and Venue 8 Pro
(I can personally confirm it works on the Venue 8 Pro). However, Bastien
Nocera reported failure on an Onda v975w with a slightly earlier version
of the patch in the bug report -
https://bugzilla.kernel.org/show_bug.cgi?id=69011#c58 , "Tested with the
same patch on a Onda v975w, and it tries very hard to detect the battery
but fails."
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net


  parent reply	other threads:[~2014-11-24  4:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-23 13:22 [PATCH V3] ACPI: Add _DEP(Operation Region Dependencies) support to fix battery issue on the Asus T100TA Lan Tianyu
2014-11-24  2:39 ` Li, Aubrey
     [not found]   ` <54729A7B.4050007-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-11-24  4:23     ` Adam Williamson [this message]
2014-11-24  4:23       ` Adam Williamson
2014-11-24  5:23     ` Michael Shigorin
2014-11-24  5:23       ` Michael Shigorin
2014-11-24 15:27     ` Rafael J. Wysocki
2014-11-24 15:27       ` Rafael J. Wysocki
2014-11-25  0:18       ` Li, Aubrey
     [not found]         ` <5473CADA.6040307-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-11-25  1:16           ` Rafael J. Wysocki
2014-11-25  1:16             ` Rafael J. Wysocki
     [not found] ` <1416748974-24124-1-git-send-email-tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2014-11-25 22:31   ` Rafael J. Wysocki
2014-11-25 22:31     ` Rafael J. Wysocki

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=1416803012.2669.68.camel@redhat.com \
    --to=awilliam-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
    --cc=aubrey.li-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org \
    --cc=jan.brummer-sgYkR6nYkOfYtjvyW6yDsg@public.gmane.org \
    --cc=lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \
    --cc=robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=shigorin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /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.