RE: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On Mon, 2020-03-02 at 10:42 +1100, Alastair D'Silva wrote:
> On Fri, 2020-02-28 at 08:15 +0100, Greg Kroah-Hartman wrote:
> > On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> > > On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > > > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > > > +{
> > > > + int i, rc;
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > > > + rc = device_create_file(>dev,
> > > > [i]);
> > > > + if (rc) {
> > > > + for (; --i >= 0;)
> > > > + device_remove_file(
> > > > >dev,
> > > > [i]);
> > >
> > > I'd rather avoid weird for loop constructs if possible.
> > >
> > > Is it actually dangerous to call device_remove_file() on an attr
> > > that hasn't
> > > been added? If not then I'd rather define an err: label and loop
> > > over the
> > > whole array there.
> >
> > None of this should be used at all, just use attribute groups
> > properly
> > and the driver core will handle this all for you.
> >
> > device_create/remove_file should never be called by anyone anymore
> > if
> > at all
> > possible.
> >
> > thanks,
> >
> > greg k-h
>
> Thanks, I'll rework it to use the .groups member of struct
> pci_driver.
>
I ended up making these available as DIMM attributes instead.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
RE: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On Fri, 2020-02-28 at 08:15 +0100, Greg Kroah-Hartman wrote:
> On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> > On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > > +{
> > > + int i, rc;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > > + rc = device_create_file(>dev, [i]);
> > > + if (rc) {
> > > + for (; --i >= 0;)
> > > + device_remove_file(>dev,
> > > [i]);
> >
> > I'd rather avoid weird for loop constructs if possible.
> >
> > Is it actually dangerous to call device_remove_file() on an attr
> > that hasn't
> > been added? If not then I'd rather define an err: label and loop
> > over the
> > whole array there.
>
> None of this should be used at all, just use attribute groups
> properly
> and the driver core will handle this all for you.
>
> device_create/remove_file should never be called by anyone anymore if
> at all
> possible.
>
> thanks,
>
> greg k-h
Thanks, I'll rework it to use the .groups member of struct pci_driver.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > +{
> > + int i, rc;
> > +
> > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > + rc = device_create_file(>dev, [i]);
> > + if (rc) {
> > + for (; --i >= 0;)
> > + device_remove_file(>dev, [i]);
>
> I'd rather avoid weird for loop constructs if possible.
>
> Is it actually dangerous to call device_remove_file() on an attr that hasn't
> been added? If not then I'd rather define an err: label and loop over the
> whole array there.
None of this should be used at all, just use attribute groups properly
and the driver core will handle this all for you.
device_create/remove_file should never be called by anyone anymore if at all
possible.
thanks,
greg k-h
Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
+int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
+{
+ int i, rc;
+
+ for (i = 0; i < ARRAY_SIZE(attrs); i++) {
+ rc = device_create_file(>dev, [i]);
+ if (rc) {
+ for (; --i >= 0;)
+ device_remove_file(>dev, [i]);
I'd rather avoid weird for loop constructs if possible.
Is it actually dangerous to call device_remove_file() on an attr that
hasn't been added? If not then I'd rather define an err: label and loop
over the whole array there.
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
[PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
From: Alastair D'Silva
This information will be used by ndctl in userspace to help users identify
the device.
Signed-off-by: Alastair D'Silva
---
arch/powerpc/platforms/powernv/pmem/Makefile | 2 +-
arch/powerpc/platforms/powernv/pmem/ocxl.c| 5 +++
.../platforms/powernv/pmem/ocxl_internal.h| 6 +++
.../platforms/powernv/pmem/ocxl_sysfs.c | 37 +++
4 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl_sysfs.c
diff --git a/arch/powerpc/platforms/powernv/pmem/Makefile
b/arch/powerpc/platforms/powernv/pmem/Makefile
index 4ceda25907d4..d02870806f30 100644
--- a/arch/powerpc/platforms/powernv/pmem/Makefile
+++ b/arch/powerpc/platforms/powernv/pmem/Makefile
@@ -4,4 +4,4 @@ ccflags-$(CONFIG_PPC_WERROR)+= -Werror
obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
-ocxlpmem-y := ocxl.o ocxl_internal.o
+ocxlpmem-y := ocxl.o ocxl_internal.o ocxl_sysfs.o
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
b/arch/powerpc/platforms/powernv/pmem/ocxl.c
index 5cd1b6d78dd6..ec73713d05ad 100644
--- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
@@ -1878,6 +1878,11 @@ static int probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
goto err;
}
+ if (ocxlpmem_sysfs_add(ocxlpmem)) {
+ dev_err(>dev, "Could not create sysfs entries\n");
+ goto err;
+ }
+
elapsed = 0;
timeout = ocxlpmem->readiness_timeout +
ocxlpmem->memory_available_timeout;
while (!is_usable(ocxlpmem, false)) {
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
index 0eb7a35d24ae..12304ceace61 100644
--- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
@@ -246,3 +246,9 @@ int ns_response_handled(const struct ocxlpmem *ocxlpmem);
*/
void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
u8 status);
+
+/**
+ * ocxlpmem_sysfs_add() - Create sysfs entries for an OpenCAPI persistent
memory device
+ * @ocxlpmem: the device metadata
+ */
+int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem);
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_sysfs.c
b/arch/powerpc/platforms/powernv/pmem/ocxl_sysfs.c
new file mode 100644
index ..7829e4bc887d
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl_sysfs.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2018 IBM Corp.
+
+#include
+#include
+#include
+#include
+#include "ocxl_internal.h"
+
+static ssize_t serial_show(struct device *device, struct device_attribute
*attr,
+ char *buf)
+{
+ struct ocxlpmem *ocxlpmem = container_of(device, struct ocxlpmem, dev);
+ const struct ocxl_fn_config *fn_config =
ocxl_function_config(ocxlpmem->ocxl_fn);
+
+ return scnprintf(buf, PAGE_SIZE, "%llu\n", fn_config->serial);
+}
+
+static struct device_attribute attrs[] = {
+ __ATTR_RO(serial),
+};
+
+int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
+{
+ int i, rc;
+
+ for (i = 0; i < ARRAY_SIZE(attrs); i++) {
+ rc = device_create_file(>dev, [i]);
+ if (rc) {
+ for (; --i >= 0;)
+ device_remove_file(>dev, [i]);
+
+ return rc;
+ }
+ }
+ return 0;
+}
--
2.24.1
