From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753068AbbDBMEy (ORCPT <rfc822;w@1wt.eu>);
	Thu, 2 Apr 2015 08:04:54 -0400
Received: from mailout3.samsung.com ([203.254.224.33]:33105 "EHLO
	mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753001AbbDBMEv (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 2 Apr 2015 08:04:51 -0400
X-AuditID: cbfee61a-f79c06d000004e71-54-551d30603adc
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
To: Thomas Schlichter <thomas.schlichter@web.de>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>, linux-acpi@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH] fix sysfs showing the correct C-states after AC->DC and
 DC->AC transitions
Date: Thu, 02 Apr 2015 14:04:35 +0200
Message-id: <2713092.F08tekMTF5@amdc1032>
User-Agent: KMail/4.8.4 (Linux/3.2.0-70-generic-pae; KDE/4.8.5; i686; ; )
In-reply-to: <3701793.GEjYLvWeTc@netbook>
References: <3701793.GEjYLvWeTc@netbook>
MIME-version: 1.0
Content-transfer-encoding: 7Bit
Content-type: text/plain; charset=ISO-8859-1
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNLMWRmVeSWpSXmKPExsVy+t9jQd1EA9lQg7vP2SzmfZa12PnwLZvF
	8n39jBaXd81hs/jce4TR4szpS6wWc741Mzuwe2xa1cnmcefaHjaPLVfbWTw+b5LzuP1sG0sA
	axSXTUpqTmZZapG+XQJXxpRFPUwFU8UqZix4xdzAeFKwi5GTQ0LARGLztYesELaYxIV769m6
	GLk4hASmM0psfLaKCcL5xijx++UERpAqNgEriYntq8BsEQEDiYkHzoB1MAucAepYeJcNJCEs
	kCjxvncZC4jNIqAq0XJgFhOIzSugKXG0uQ8sLirgKbFz+gGwQZxA8Q9v5wKdwQG0TUNi76sU
	iHJBiR+T74GVMwvIS+zbP5UVwtaR2N86jW0Co8AsJGWzkJTNQlK2gJF5FaNoakFyQXFSeq6h
	XnFibnFpXrpecn7uJkZwkD+T2sG4ssHiEKMAB6MSD2/GHplQIdbEsuLK3EOMEhzMSiK8D7Rl
	Q4V4UxIrq1KL8uOLSnNSiw8xSnOwKInzKtm3hQgJpCeWpGanphakFsFkmTg4pRoYV2z7yLbt
	7zw38225urqcTSzcR1V/cWRKm/KFus+yCYurqt6/R1a4633c3Ll1rYWt/95u/Ozd3rVKS4rp
	8v3HR1/zV5f/m+1hcVHpKt9JlYrStwFbjCsmvtP5ov4pM3SWwvWd9+fz8Jv1LhL7cUlueteV
	6rUttgeeBb6rXZoZGpp9lM/7vr6LEktxRqKhFnNRcSIAjd3tRG4CAAA=
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On Wednesday, April 01, 2015 08:04:34 PM Thomas Schlichter wrote:
> Hello,

Hi,

> I do have a Samsung NC20 netbook which provides the C-states C1 and C2 to the 
> OS when connected to AC, and additionally provides the C3 C-state when 
> disconnected from AC. With the current kernels I have these two problems 
> (regressions):
> 
> 1. The number of C-states shown in sysfs is fixed to the number of C-states
>    present at boot.
>    If I boot with AC connected, I always only see the C-states up to C2 even
>    if I disconnect AC.
> 
>    The reason is commit 130a5f692425e6237229598a8624da0a247f33d5
>    "ACPI / cpuidle: remove dev->state_count setting". It removes the update of
>    dev->state_count, but sysfs uses exactly this variable to show the
>    C-states.
> 
>    The fix is to use drv->state_count in sysfs. As this is currently the last
>    user of dev->state_count, this variable can be completely removed. And this
>    is exactly what Bartlomiej Zolnierkiewicz's not yet applied patch "[PATCH
>    v2 9/9] cpuidle: remove state_count field from struct cpuidle_device"
>    http://marc.info/?m=138756533317836 does.
> 
>    Long story short: That patch fixes problem 1 for me.

Rafael, please apply that patch (as Thomas has noticed it is required
to fix issue with dynamic dev/drv->state_count updates, sorry for not
catching it earlier).

[ Originally the whole series was dropped when patch #8 ("[PATCH v2 8/9]
  intel_idle: use the common cpuidle_[un]register() routines") turned out
  to introduce some problems.  Patches #1-7 were later re-merged but not
  patch #9 (which is independent of patch #8). ]

> 2. After a AC->DC or DC->AC transition, the name and description of the
>    POLLING C-state become "<null>".
> 
>    Here, the reason is commit d7c7f103262bc2248548ed0e113e916e843c4eeb
>    "cpuidle: don't call poll_idle_init() for every cpu". It only calls
>    poll_idle_init() during cpuidle_register_driver() instead of
>    cpuidle_enable_device() and thus does not re-initialize the fields of
>    drv->states[0] after acpi_processor_setup_cpuidle_states() cleared them.
> 
>    Here, the fix is to _not_ clear drv->states[0] in
>    acpi_processor_setup_cpuidle_states(). For this purpose I created a small
>    patch.

This patch looks fine to me.  FWIW:

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> For your convenience, both of the fixing patches are attached to this mail.
> The first one is necessary for all kernels since 3.14, the second one for all
> kernels since 3.13. So I'd propose to push both of them also to the necessary
> stable kernels.

Sounds good to me.  Thank you for working on this.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics