Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933071AbbD1IxG (ORCPT ); Tue, 28 Apr 2015 04:53:06 -0400 Received: from mail-ob0-f182.google.com ([209.85.214.182]:35233 "EHLO mail-ob0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752077AbbD1IxC (ORCPT ); Tue, 28 Apr 2015 04:53:02 -0400 MIME-Version: 1.0 In-Reply-To: <553F426B.3050601@linux.vnet.ibm.com> References: <1430202214-13807-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com> <1430202214-13807-3-git-send-email-shilpa.bhat@linux.vnet.ibm.com> <553F426B.3050601@linux.vnet.ibm.com> Date: Tue, 28 Apr 2015 14:23:01 +0530 Message-ID: Subject: Re: [PATCH v2 2/2] cpufreq: powernv: Register for OCC related opal_message notification From: Viresh Kumar To: Shilpasri G Bhat Cc: "linuxppc-dev@ozlabs.org" , Linux Kernel Mailing List , "Rafael J. Wysocki" , Preeti U Murthy , "linux-pm@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2283 Lines: 44 On 28 April 2015 at 13:48, Shilpasri G Bhat wrote: > My bad I haven't added explicit comment to state reason behind this change. > > I modified the definition of *throttle_check() to match the function definition > to be called via smp_call() instead of adding an additional wrapper around > *throttle_check(). > > OCC is a chip entity and any local throttle state changes should be associated > to cpus belonging to that chip. The *throttle_check() will read the core > register PMSR to verify throttling. All the cores in a chip will have the same > throttled state as they are managed by a the same OCC in that chip. > > smp_call() is required to ensure *throttle_check() is called on a cpu belonging > to the chip for which we have received throttled/unthrottled notification. We > could be handling throttled/unthrottled notification of 'chip1' in 'chip2' so do > an smp_call() on 'chip1'. Okay. Lets talk about the code that is already present in mainline. Isn't that suffering from this issue ? If yes, then you need to bugfix that separately. > We are irq_disabled in powernv_cpufreq_occ_msg() the notification handler. > Thus the use of kworker to do an smp_call and restore policy->cur. > > OCC_RESET is global event it affects frequency of all chips. Pmax capping is > local event, it affects the frequency of a chip. > >> That's a lot. I am not an expert here and so really can't comment on >> the internals of ppc. But, is it patch solving a single problem ? I don't >> know, I somehow got the impression that it can be split into multiple >> (smaller & review-able) patches. Only if it makes sense. Your call. > > All the changes introduced in this patch is centered around opal_message > notification handler powernv_cpufreq_occ_msg(). I can split it into multiple > patches but it all will be relevant only to solve the above problem. And that's what I meant here. Yes, this all is solving a central problem, but a patch must be divided into separate, independently working, entities. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/