From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754553AbbGXT1l (ORCPT ); Fri, 24 Jul 2015 15:27:41 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:50669 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752134AbbGXT1k (ORCPT ); Fri, 24 Jul 2015 15:27:40 -0400 From: "Rafael J. Wysocki" To: Viresh Kumar Cc: Linux PM list , Linux Kernel Mailing List , Russell King - ARM Linux Subject: Re: [PATCH 2/2] cpufreq: Separate CPU device removal from CPU online Date: Fri, 24 Jul 2015 21:54:30 +0200 Message-ID: <4877095.RtPZqi4Ng3@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.1.0-rc5+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20150724021916.GA11315@linux> References: <7868353.pEStq1MJ2a@vostro.rjw.lan> <1532128.uWbA5iRc7E@vostro.rjw.lan> <20150724021916.GA11315@linux> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday, July 24, 2015 07:49:16 AM Viresh Kumar wrote: > On 23-07-15, 22:56, Rafael J. Wysocki wrote: > > > > + if (policy && policy->kobj_cpu != cpu) { > > > > > > Why are you comparing cpu against kobj_cpu ? I don't think it can ever > > > be false. > > So what I meant was that the expression 'policy->kobj_cpu != cpu' will > never return 'false'. Because policy->kobj_cpu is going to get set to > the cpu for which we allocated the policy. And so it wouldn't match > for any other CPU. Unless kobj_cpu is removed before another one is registered (not a very realistic scenario, but there's no guarantee that it will never happen). But in that case we won't do the right thing anyway ... > > It can. When we're adding a CPU that has a policy already, because it is > > "related" to a previously registered CPU. > > In this case also the expression will return true. > > > > > + ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq"); > > > > + if (ret) { > > > > + dev_dbg(dev, "%s: Failed to create link (%d)\n", > > > > > > dev_err > > > > Well, I'm wondering about this. Why does this have to be dev_err()? > > Isn't this an error? We need to create a symlink, we failed and > atleast the user should know about it. Why hide such failures ? dev_err() doesn't mean "device error message". It means "high priority device message". The user will know that the link is not there anyway and the question here is whether or not the associated kernel message has to be high-priority. -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.