From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752682AbbJZCON (ORCPT ); Sun, 25 Oct 2015 22:14:13 -0400 Received: from mail-bl2on0083.outbound.protection.outlook.com ([65.55.169.83]:56352 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752364AbbJZCOL convert rfc822-to-8bit (ORCPT ); Sun, 25 Oct 2015 22:14:11 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; alien8.de; dkim=none (message not signed) header.d=none;alien8.de; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0NWT1JJ-08-A5M-02 X-M-MSG: Date: Mon, 26 Oct 2015 09:58:45 +0800 From: Huang Rui To: Guenter Roeck CC: Borislav Petkov , Peter Zijlstra , "Jean Delvare" , Andy Lutomirski , "Andreas Herrmann" , Thomas Gleixner , Ingo Molnar , "Rafael J. Wysocki" , "Len Brown" , John Stultz , =?iso-8859-1?Q?Fr=E9d=E9ric?= Weisbecker , , , , Andreas Herrmann , Aravind Gopalakrishnan , Borislav Petkov , Fengguang Wu , Aaron Lu , Tony Li Subject: Re: [PATCH v2 01/10] hwmon: (fam15h_power) Refactor attributes for dynamically added Message-ID: <20151026015843.GA8036@hr-amur2> References: <1445308109-17970-1-git-send-email-ray.huang@amd.com> <1445308109-17970-2-git-send-email-ray.huang@amd.com> <562A393C.3000707@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset="gb2312" Content-Disposition: inline In-Reply-To: <562A393C.3000707@roeck-us.net> User-Agent: Mutt/1.5.21 (2010-09-15) Content-Transfer-Encoding: 8BIT X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(199003)(164054003)(479174004)(377454003)(24454002)(189002)(47776003)(11100500001)(19580405001)(23696002)(19580395003)(50466002)(48336001)(2950100001)(77096005)(5008740100001)(5007970100001)(92566002)(189998001)(83506001)(97736004)(110136002)(50986999)(54356999)(76176999)(105586002)(106466001)(101416001)(86362001)(33656002)(87936001)(33716001)(47136003)(4001350100001)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM3PR12MB0858;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0858;2:jv2xPmQ1uh8YFhYzQb6isZQCnP3EaS35G1oAn88suIFjg81GqSaEwTQwat30jhQNQPRIFjIKoU6s7iZZjxtasfnzGC9zgYZXnQqK9793Vc0iVt8AuYO9caMgiMzkHzbwXcNip99ApzVt8BkSssPiSw7dqOdHhnpMZjC+9WmDwZA=;3:qjacPxT67EAyxEqVZ2W778dM5Ss5BJe0DBcX3RMcJOOT8sGmaFuTdFLHHYnJF75RjlOU3xWvyRU79xU8yzNJQF1YG1l9Gfa7JAf/qBtV/gVMx3LjUKdlqlKKki94dKe5nTM3HrB+n6GNj5+Sq7W7zD8EnudDvTR2oG2ca9fk9PkTRRmxlM2tTAWtqotmNRazUnxFIT+aKG25Pq9YBqfvkhJmKwdyeZkcIiBnDW1SUPccxTy7zl8ht9oEiyRx+iuQ;25:HKKeTuVO4BwbVxe+Ngpk8SGI0Z5E6Uw2OGrKAPyMez1E/OJy2n4XYLFibIjFU/vdxqx1TfIZNA1IXXDK2af+nEhD0Jau7pE9XQcfyEICPUp/jTLwo4vmTyYAKTyBwDhVY8E05ORGlK5YzNRHHI9OEyAF15Z14uiz7DOxxsX3ygXvfZVhEKVGvqp+Z7Dv9axSFKCXyvLYHuicZ1y60267EohAsPAdYcJxDtnuZ/j1zZXleTIkBX+DedJoDTgjnvz34mQv5cLyeQo7ukjKDEACjg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0858; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0858;20:ZGVk+8LmkX6aff8ssyDuenVePX1itHGsUZWKKIxuA9jaQJ1EGC3sf0k9/X+6UO0GkHv4ZI9hAcWDYaE5Z+9o3eVEiaHDixzbZyXg/QF5uc2Cs/IdZ2JzIYnSfJWfw1fXJo+YHLXyrpVDBqdvJZcedZw+9r+UJPNGJjxtVQPQNseJCD6lx+91Mx3LdVnLBF4OTMVE0PEcdvRq8WVwviZLQGs90puwZi/ykd/8OLQoQEydiyHmM30Gwe7gjkii69322cP3vI3yXgxfiHKtjPf41jmQhpAo2EQdatReCe3Caq4qL02KsVRgRoDh26Z5oWmm6n2LYMr9X277PXeLjJpIb/RpNndAet0mKYv6Q49E3OipyP6F2IX6ckN0eO7iPTJJU4b+J0VR0jyd18Kuc7O8fLJXKixv+q/dUlZ9wkgqv0GUVc6rKVEcATmQuOtd36C6tjPtYjFOAxJzuKbdjZGfhcgP/R/0+V1gelIZDy0uHB2wKo2nRV6lbzJF40Vyt92E;4:VQT9byG5uIiDjT88eu9ZS0xei+WQGj4H9lVpvSvtfiOrqHlmoW4OnbRcnRd9+UO0xtY/zdVBDqHURQq/OpDpgXbzCnp0Gg/9jIgpl/Vezug1STNjXfrRWv7q/R9qo0Lf1flbKbo56OQMuEBqZHWhHlad35WHZU5BDErPia5/RpGK3nhgUCagsLIrlWjXZ0InuIuzonhsvPtCrBcQkRD407VXUDH3UEH926QF5T2Mi4RNrxNUeKVkAfZtOP5yocmkYdW41Luf3lm3QVMQMbXItidUvJmZT8RRUK3Vc38ZBklZi5H6j515IK7bNwlh//VAUj2tC6guOhLwhxCSXbLCur39UYE9/zdua1RypRlNvSsegCHk/8iPe22qXh5iRL/a X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001)(102215026);SRVR:DM3PR12MB0858;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0858; X-Forefront-PRVS: 0741C77572 X-Microsoft-Exchange-Diagnostics: =?gb2312?B?MTtETTNQUjEyTUIwODU4OzIzOlRyRXFNcjlFeVpjYnY5bllCb25KZGVUNGxp?= =?gb2312?B?MzMvR2NMY3psSE5ENW04eW9Mdy9EQVdCM3RmNWRmYmNhVFgzNkV3QjlyYWsw?= =?gb2312?B?MEg3QlJGcXlsZHd5NytEc0FKSVd1cFU5dGtKUmI2bXNGTUJ0ZHpnZ2R0OGp3?= =?gb2312?B?TStXdlArQjJtajdXRHhaNkpBcmY5bGI2TDhFZDhWMEpFcTVIU2c5S0NRSnNP?= =?gb2312?B?YytsWkdTSjZmM3N0OXNmTXZZLzVrNFROMHNaemVIZkFCQTlya1ZZZm4yckgr?= =?gb2312?B?cVRkVnFxQytPODU3Q3hOcGFwdzE3NmtvLzlOVmVkT2poNDFHT2pzemFacDF5?= =?gb2312?B?b0pVTFEraGJaam1SZm1PLzh3dEhLbGdWK29kVE5VdmZrUzdjaWVNRFJQMkds?= =?gb2312?B?SmZnUDl3QUVwRDJ0ZllreGxSLzJrSWVGdC9yUFZNSlNqeitCZWtqSzhSajZZ?= =?gb2312?B?N1IvOVRzeDI0cGlmSWhrVFB1eWF5ZUFDdi8vcXN5a1g5NVBJeWVJZ0ZpdGRZ?= =?gb2312?B?TWo5bUVvc0RpK3J3VllkYVY0OXl5dU9xMElZV0NuS1ZpcytVVlRrVkxNQXMy?= =?gb2312?B?dXZVbjBzdWp2UjZpdXpDcWNFSHFpZit1dFNqM2FuQzIvSU83QUNXemJxcDdF?= =?gb2312?B?UlVlWk8zd29NU0lLeFp6NzVrNWJjV0VwYnoxNnZTZm52MmdKdWx6SjVpUmVT?= =?gb2312?B?UDlMZGFELzFKZlMwSC81VXdtQnl6OHRQdWZNTHVndlYzNThtQ3N6VFZuV0dp?= =?gb2312?B?em1FbEs5Zk9PQ3RWd0RUempUaFA0YVhCemhJd2tEeXMxTHpvL0htNDZjWDNU?= =?gb2312?B?SytaOVFXR05RTitLRWxVTHo5NDgyYlZGUCtqdEZaWERwKzhsZDAzMmNFZ0xp?= =?gb2312?B?SkZ3R1Jrb3dTdHNLenZSVEQ3WWJlREZVZGJKb29XNFp6b2c5YkdoY3NzTE8y?= =?gb2312?B?SE1uT2ZuTTFQZi8ra01ucUpuazNSM2RMVW12cjB6ZVBJZE9ncUtzSXJJeVM3?= =?gb2312?B?cWwwVzFqd0pheTloeVQ5WXBRc01seDVSVGdSQzVwYmt0ZEFXdnk3T1MrR2N4?= =?gb2312?B?NjY2MzJ3YkMyN1N3c3cwSzFiSVF3NGdyMCtOL1RmREczVzJiL1E2T0VRY0kz?= =?gb2312?B?N2oxMXdtUGtqWFBXNGgyRmdENGdjUzRVR3h2TnhVdXRjTTV2N1l6RUlaWTAx?= =?gb2312?B?dzhJalFZRDVZR1EyN0tzUzJHUFBCeU16Qk5jWXB2UTNYdFVaRlprSG9VdzBU?= =?gb2312?B?Skp5K2dSMW5ENEZVa0R0elJ0dXQ3V2k1SFNWOHRFM1hzRmF5NFBJZXk1RGF1?= =?gb2312?B?cmY0bDVMeWRzR2pzTTFqWnlNaTg0YUxMUFdTZXJmRnZueGlVSjRSelVXV3dp?= =?gb2312?B?RVNzK0J5UnQ2Q2drZVd4ZDB0TWFnTStOM1RJNGtBPT0=?= X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0858;5:+FZlON0QlHtMPAOTrjrVmE9DRS2+L+gwItGhLEiKLAdO/++FEzaYVCCKr3Ivlc755/jBq823Dy90U0YUQHenfeZWDVR9LpO2DDyz3ZFDPwjNqblbLwcdbM2NOkmrFR11Bm0x8/5E2Dwqj7oUngX75w==;24:zpyUztM396vi5AhaWk6Y5EuGGJfhC3f/eGUjjbL4RgDoidjlje/PvHjN2i9hBDrpJ3orjrwtxC859llDpHY+YhVaprPADZ2gOxpi4aEsJw4=;20:XbiZStaM1Rw1ncE7rUy4J+T5P9I8yFBnZ2eY7SmFa0XefzQJvs2plwIEwLImNAARmlnoKUAbGzymc67+tTAsZs9xgYzj7Pok3EsjxVyURphUOVWqAnAm4jeBWxVESkS0ooK5/Jmg05PylMhJqjiStNd5mqg0Ap6FI/IZveuVvQmwVA0xfeKVlS8n6GgHGxkTAJUPprzkaJ8i2XbgroOcgRORETDzpy/XP+gL6XfQ6Wq8FR/kaZBBrALGY374nsPW SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2015 01:59:45.2725 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR12MB0858 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 23, 2015 at 06:42:20AM -0700, Guenter Roeck wrote: > On 10/19/2015 07:28 PM, Huang Rui wrote: > >Attributes depend on the CPU model the driver gets loaded on. > >Therefore, add those attributes dynamically at init time. This is more > >flexible to control the different attributes on different platforms. > > > >Suggestedy-by: Borislav Petkov > >Signed-off-by: Huang Rui > >Cc: Guenter Roeck > >Cc: Peter Zijlstra > >Cc: Ingo Molnar > >--- > > drivers/hwmon/fam15h_power.c | 70 ++++++++++++++++++++++++++++---------------- > > 1 file changed, 45 insertions(+), 25 deletions(-) > > > >diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c > >index e80ee23..41d022e 100644 > >--- a/drivers/hwmon/fam15h_power.c > >+++ b/drivers/hwmon/fam15h_power.c > >@@ -41,12 +41,17 @@ MODULE_LICENSE("GPL"); > > #define REG_TDP_RUNNING_AVERAGE 0xe0 > > #define REG_TDP_LIMIT3 0xe8 > > > >+#define FAM15H_MIN_NUM_ATTRS 2 > >+#define FAM15H_NUM_GROUPS 2 > >+ > > struct fam15h_power_data { > > struct pci_dev *pdev; > > unsigned int tdp_to_watts; > > unsigned int base_tdp; > > unsigned int processor_pwr_watts; > > unsigned int cpu_pwr_sample_ratio; > >+ const struct attribute_group *fam15h_power_groups[FAM15H_NUM_GROUPS]; > >+ struct attribute_group fam15h_power_group; > > }; > > > > static ssize_t show_power(struct device *dev, > >@@ -105,29 +110,31 @@ static ssize_t show_power_crit(struct device *dev, > > } > > static DEVICE_ATTR(power1_crit, S_IRUGO, show_power_crit, NULL); > > > >-static umode_t fam15h_power_is_visible(struct kobject *kobj, > >- struct attribute *attr, > >- int index) > >+static int fam15h_power_init_attrs(struct pci_dev *pdev, > >+ struct fam15h_power_data *data) > > { > >- /* power1_input is only reported for Fam15h, Models 00h-0fh */ > >- if (attr == &dev_attr_power1_input.attr && > >- (boot_cpu_data.x86 != 0x15 || boot_cpu_data.x86_model > 0xf)) > >- return 0; > >+ int n = FAM15H_MIN_NUM_ATTRS; > >+ struct attribute **fam15h_power_attrs; > > > >- return attr->mode; > >-} > >+ if (boot_cpu_data.x86 == 0x15 && boot_cpu_data.x86_model <= 0xf) > >+ n += 1; > > > >-static struct attribute *fam15h_power_attrs[] = { > >- &dev_attr_power1_input.attr, > >- &dev_attr_power1_crit.attr, > >- NULL > >-}; > >+ fam15h_power_attrs = devm_kcalloc(&pdev->dev, n, > >+ sizeof(*fam15h_power_attrs), > >+ GFP_KERNEL); > > > >-static const struct attribute_group fam15h_power_group = { > >- .attrs = fam15h_power_attrs, > >- .is_visible = fam15h_power_is_visible, > >-}; > >-__ATTRIBUTE_GROUPS(fam15h_power); > >+ if (!fam15h_power_attrs) > >+ return -ENOMEM; > >+ > >+ n = 0; > >+ fam15h_power_attrs[n++] = &dev_attr_power1_crit.attr; > >+ if (boot_cpu_data.x86 == 0x15 && boot_cpu_data.x86_model <= 0xf) > >+ fam15h_power_attrs[n++] = &dev_attr_power1_input.attr; > >+ > >+ data->fam15h_power_group.attrs = fam15h_power_attrs; > >+ > >+ return 0; > >+} > > > > static bool should_load_on_this_node(struct pci_dev *f4) > > { > >@@ -186,11 +193,12 @@ static int fam15h_power_resume(struct pci_dev *pdev) > > #define fam15h_power_resume NULL > > #endif > > > >-static void fam15h_power_init_data(struct pci_dev *f4, > >- struct fam15h_power_data *data) > >+static int fam15h_power_init_data(struct pci_dev *f4, > >+ struct fam15h_power_data *data) > > { > > u32 val, eax, ebx, ecx, edx; > > u64 tmp; > >+ int ret; > > > > pci_read_config_dword(f4, REG_PROCESSOR_TDP, &val); > > data->base_tdp = val >> 16; > >@@ -211,11 +219,15 @@ static void fam15h_power_init_data(struct pci_dev *f4, > > /* convert to microWatt */ > > data->processor_pwr_watts = (tmp * 15625) >> 10; > > > >+ ret = fam15h_power_init_attrs(f4, data); > >+ if (ret) > >+ return ret; > >+ > > cpuid(0x80000007, &eax, &ebx, &ecx, &edx); > > > > /* CPUID Fn8000_0007:EDX[12] indicates to support accumulated power */ > > if (!(edx & BIT(12))) > >- return; > >+ return 0; > > > > /* > > * determine the ratio of the compute unit power accumulator > >@@ -223,14 +235,17 @@ static void fam15h_power_init_data(struct pci_dev *f4, > > * Fn8000_0007:ECX > > */ > > data->cpu_pwr_sample_ratio = ecx; > >+ > >+ return 0; > > } > > > > static int fam15h_power_probe(struct pci_dev *pdev, > >- const struct pci_device_id *id) > >+ const struct pci_device_id *id) > > { > > struct fam15h_power_data *data; > > struct device *dev = &pdev->dev; > > struct device *hwmon_dev; > >+ int ret; > > > > /* > > * though we ignore every other northbridge, we still have to > >@@ -246,12 +261,17 @@ static int fam15h_power_probe(struct pci_dev *pdev, > > if (!data) > > return -ENOMEM; > > > >- fam15h_power_init_data(pdev, data); > >+ ret = fam15h_power_init_data(pdev, data); > >+ if (ret) > >+ return ret; > >+ > > data->pdev = pdev; > > > >+ data->fam15h_power_groups[0] = &data->fam15h_power_group; > >+ > > hwmon_dev = devm_hwmon_device_register_with_groups(dev, "fam15h_power", > > data, > >- fam15h_power_groups); > >+ (const struct attribute_group **)&data->fam15h_power_groups); > > Why this typecast ? It should not be needed. > That's because there will be a warning without the typecast. Expected 'const struct attribute_group **', but current type is 'const struct attribute_group * (*)[2]'. --- CC [M] /home/ray/tip/drivers/hwmon/fam15h_power.o /home/ray/tip/drivers/hwmon/fam15h_power.c: In function ¡®fam15h_power_probe¡¯: /home/ray/tip/drivers/hwmon/fam15h_power.c:481:11: warning: passing argument 4 of ¡®devm_hwmon_device_register_with_groups¡¯ from incompatible pointer type [enabled by default] &data->fam15h_power_groups); ^ In file included from /home/ray/tip/drivers/hwmon/fam15h_power.c:22:0: include/linux/hwmon.h:26:1: note: expected ¡®const struct attribute_group **¡¯ but argument is of type ¡®const struct attribute_group * (*)[2]¡¯ devm_hwmon_device_register_with_groups(struct device *dev, const char *name, ^ --- Thanks, Rui