From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (193.142.43.55:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 24 Sep 2019 23:30:41 -0000 Received: from mga07.intel.com ([134.134.136.100]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1iCuGV-0000pN-TK for speck@linutronix.de; Wed, 25 Sep 2019 01:30:40 +0200 Date: Tue, 24 Sep 2019 16:25:03 -0700 From: Pawan Gupta Subject: [MODERATED] Re: [PATCH v4 04/10] TAAv4 4 Message-ID: <20190924232503.GE2473@guptapadev.amr> References: <20190904060028.GD7212@kroah.com> <20190906072835.GD13480@guptapadev.amr> <20190906092727.GA16843@kroah.com> <20190910184223.GA7543@guptapadev.amr> <20190910223334.GA21301@kroah.com> <20190911023223.GA8305@guptapadev.amr> <20190923191312.GB161280@kroah.com> <20190923222553.GA2473@guptapadev.amr> <20190924050459.GA3705@kroah.com> MIME-Version: 1.0 In-Reply-To: <20190924050459.GA3705@kroah.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Tue, Sep 24, 2019 at 07:04:59AM +0200, speck for Greg KH wrote: > Either way, my original comment of "do not use sysfs_create_group() when > you have a 'struct device'" still stands, that needs to be fixed up no > matter what here. Thanks for your inputs. As we have the 'struct device' are you suggesting to use device_create_file()? Approach 1 - sysfs_create_file() => device_create_file() ----------------- diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c index 40fec8ab82b1..4799c081198c 100644 --- a/arch/x86/kernel/cpu/tsx.c +++ b/arch/x86/kernel/cpu/tsx.c @@ -162,7 +162,7 @@ static void tsx_update_on_each_cpu(bool val) put_online_cpus(); } -static ssize_t tsx_show(struct device *cdev, +static ssize_t htm_show(struct device *cdev, struct device_attribute *attr, char *buf) { @@ -171,7 +171,7 @@ static ssize_t tsx_show(struct device *cdev, static DEFINE_MUTEX(tsx_mutex); -static ssize_t tsx_store(struct device *cdev, +static ssize_t htm_store(struct device *cdev, struct device_attribute *attr, const char *buf, size_t count) { @@ -197,15 +197,14 @@ static ssize_t tsx_store(struct device *cdev, return count; } -static DEVICE_ATTR_RW(tsx); +static const DEVICE_ATTR_RW(htm); static int __init tsx_sysfs_init(void) { if (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED) return -ENODEV; - return sysfs_create_file(&cpu_subsys.dev_root->kobj, - &dev_attr_tsx.attr); + return device_create_file(cpu_subsys.dev_root, &dev_attr_htm); } -late_initcall(tsx_sysfs_init); +device_initcall(tsx_sysfs_init); ------------------------------------------------------------------- If the first approach is not what you are expecting, is the below patch to add the attribute to the cpu_root_attrs list okay? Approach 2 - Add tsx attribute to cpu_root_attrs in drivers/base/cpu.c -------------------- diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c index da75021daa1d..631996e129d5 100644 --- a/arch/x86/kernel/cpu/tsx.c +++ b/arch/x86/kernel/cpu/tsx.c @@ -158,17 +158,15 @@ static void tsx_update_on_each_cpu(bool val) put_online_cpus(); } -static ssize_t tsx_show(struct device *cdev, - struct device_attribute *attr, - char *buf) +ssize_t htm_show(struct device *dev, struct device_attribute *attr, char *buf) { return sprintf(buf, "%d\n", tsx_ctrl_state == TSX_CTRL_ENABLE ? 1 : 0); } static DEFINE_MUTEX(tsx_mutex); -static ssize_t tsx_store(struct device *cdev, struct device_attribute *attr, - const char *buf, size_t count) +ssize_t htm_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) { enum tsx_ctrl_states requested_state; ssize_t ret; @@ -229,32 +227,10 @@ static ssize_t tsx_store(struct device *cdev, struct device_attribute *attr, return count; } -static DEVICE_ATTR_RW(tsx); - -static struct attribute *tsx_ctrl_attrs[] = { - &dev_attr_tsx.attr, - NULL -}; - -static umode_t tsx_ctrl_attr_is_visible(struct kobject *kobj, - struct attribute *attr, int index) +umode_t htm_is_visible(void) { - if ((attr == &dev_attr_tsx.attr) && - (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED)) + if (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED) return 0; - return attr->mode; + return 0644; } - -static struct attribute_group tsx_ctrl_attr_group = { - .attrs = tsx_ctrl_attrs, - .is_visible = tsx_ctrl_attr_is_visible, -}; - -static int __init tsx_sysfs_init(void) -{ - return sysfs_create_group(&cpu_subsys.dev_root->kobj, - &tsx_ctrl_attr_group); -} - -late_initcall(tsx_sysfs_init); diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index 80d43ff9a7ec..2b1579c7f22d 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -460,6 +460,34 @@ struct device *cpu_device_create(struct device *parent, void *drvdata, } EXPORT_SYMBOL_GPL(cpu_device_create); +ssize_t __weak htm_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + return -ENODEV; +} + +ssize_t __weak htm_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + return -ENODEV; +} + +DEVICE_ATTR_RW(htm); + +umode_t __weak htm_is_visible(void) +{ + return 0; +} + +static umode_t cpu_root_attrs_is_visible(struct kobject *kobj, + struct attribute *attr, int index) +{ + if (attr == &dev_attr_htm.attr) + return htm_is_visible(); + + return attr->mode; +} + #ifdef CONFIG_GENERIC_CPU_AUTOPROBE static DEVICE_ATTR(modalias, 0444, print_cpu_modalias, NULL); #endif @@ -481,11 +509,13 @@ static struct attribute *cpu_root_attrs[] = { #ifdef CONFIG_GENERIC_CPU_AUTOPROBE &dev_attr_modalias.attr, #endif + &dev_attr_htm.attr, NULL }; static struct attribute_group cpu_root_attr_group = { - .attrs = cpu_root_attrs, + .attrs = cpu_root_attrs, + .is_visible = cpu_root_attrs_is_visible, }; static const struct attribute_group *cpu_root_attr_groups[] = {