[PATCH 0/3] Enclosure sysfs refactor

[PATCH 0/3] Enclosure sysfs refactor

[Mariusz Tkaczyk]

Hi Bjorn,
I agreed with Stuart to take over the NPEM implementation[1].
First part I want to share is a small refactor around enclosure interface.

The one sysfs change introduced is changing active LED to write-only.
get_active() callback is not implemented for SES which is the
only one enclosure API consumer now.

[1] https://lore.kernel.org/linux-pci/cover.1643822289.git.stuart.w.hayes@gmail.com/

Mariusz Tkaczyk (3):
  misc: enclosure: remove get_active() callback
  misc: enclosure, ses: simplify some get callbacks
  misc: enclosure: update sysfs api

 drivers/misc/enclosure.c  | 96 ++++++++++++++++-----------------------
 drivers/scsi/ses.c        | 33 ++++++++------
 include/linux/enclosure.h | 14 ++----
 3 files changed, 61 insertions(+), 82 deletions(-)

-- 
2.26.2

[PATCH 1/3] misc: enclosure: remove get_active() callback

[Mariusz Tkaczyk]

The callback is not used, remove it.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 drivers/misc/enclosure.c  | 14 +-------------
 include/linux/enclosure.h |  2 --
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 1b010d9267c9..fd0707a8ed79 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -531,17 +531,6 @@ static ssize_t set_component_status(struct device *cdev,
 		return -EINVAL;
 }
 
-static ssize_t get_component_active(struct device *cdev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
-	struct enclosure_component *ecomp = to_enclosure_component(cdev);
-
-	if (edev->cb->get_active)
-		edev->cb->get_active(edev, ecomp);
-	return sysfs_emit(buf, "%d\n", ecomp->active);
-}
-
 static ssize_t set_component_active(struct device *cdev,
 				    struct device_attribute *attr,
 				    const char *buf, size_t count)
@@ -645,8 +634,7 @@ static DEVICE_ATTR(fault, S_IRUGO | S_IWUSR, get_component_fault,
 		    set_component_fault);
 static DEVICE_ATTR(status, S_IRUGO | S_IWUSR, get_component_status,
 		   set_component_status);
-static DEVICE_ATTR(active, S_IRUGO | S_IWUSR, get_component_active,
-		   set_component_active);
+static DEVICE_ATTR(active, S_IWUSR, NULL, set_component_active);
 static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
 		   set_component_locate);
 static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR, get_component_power_status,
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 1c630e2c2756..8d09c6d07bf1 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -62,8 +62,6 @@ struct enclosure_component_callbacks {
 	int (*set_fault)(struct enclosure_device *,
 			 struct enclosure_component *,
 			 enum enclosure_component_setting);
-	void (*get_active)(struct enclosure_device *,
-			   struct enclosure_component *);
 	int (*set_active)(struct enclosure_device *,
 			  struct enclosure_component *,
 			  enum enclosure_component_setting);
-- 
2.26.2

RE: [PATCH 1/3] misc: enclosure: remove get_active() callback

[Dan Williams]

Mariusz Tkaczyk wrote:
> The callback is not used, remove it.

While the callback is not used, userspace might still be dependent upon
on the "active" attribute being readable. So I think the change would be
limited to:

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 4ba966529458..0d7225dc5d45 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -534,11 +534,8 @@ static ssize_t set_component_status(struct device *cdev,
 static ssize_t get_component_active(struct device *cdev,
                                    struct device_attribute *attr, char *buf)
 {
-       struct enclosure_device *edev = to_enclosure_device(cdev->parent);
        struct enclosure_component *ecomp = to_enclosure_component(cdev);
 
-       if (edev->cb->get_active)
-               edev->cb->get_active(edev, ecomp);
        return sysfs_emit(buf, "%d\n", ecomp->active);
 }
 
...unless you can prove that userspace never reads "active", but I
expect it is too late to make a breaking change like that.

[PATCH 2/3] misc: enclosure, ses: simplify some get callbacks

[Mariusz Tkaczyk]

Remove active, status, fault and locate variables from
enclosure_component struct. Return then directly.
No functional changes intended.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 drivers/misc/enclosure.c  | 15 +++++++++------
 drivers/scsi/ses.c        | 33 ++++++++++++++++++---------------
 include/linux/enclosure.h | 12 ++++--------
 3 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index fd0707a8ed79..00f50fd0cc85 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -478,10 +478,11 @@ static ssize_t get_component_fault(struct device *cdev,
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
+	int status = 0;
 
 	if (edev->cb->get_fault)
-		edev->cb->get_fault(edev, ecomp);
-	return sysfs_emit(buf, "%d\n", ecomp->fault);
+		status = edev->cb->get_fault(edev, ecomp);
+	return sysfs_emit(buf, "%d\n", status);
 }
 
 static ssize_t set_component_fault(struct device *cdev,
@@ -502,10 +503,11 @@ static ssize_t get_component_status(struct device *cdev,
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
+	enum enclosure_status status = 0;
 
 	if (edev->cb->get_status)
-		edev->cb->get_status(edev, ecomp);
-	return sysfs_emit(buf, "%s\n", enclosure_status[ecomp->status]);
+		status = edev->cb->get_status(edev, ecomp);
+	return sysfs_emit(buf, "%s\n", enclosure_status[status]);
 }
 
 static ssize_t set_component_status(struct device *cdev,
@@ -549,10 +551,11 @@ static ssize_t get_component_locate(struct device *cdev,
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
+	int status = 0;
 
 	if (edev->cb->get_locate)
-		edev->cb->get_locate(edev, ecomp);
-	return sysfs_emit(buf, "%d\n", ecomp->locate);
+		status = edev->cb->get_locate(edev, ecomp);
+	return sysfs_emit(buf, "%d\n", status);
 }
 
 static ssize_t set_component_locate(struct device *cdev,
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 0a1734f34587..901dc94e5aeb 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -209,13 +209,14 @@ static void ses_get_fault(struct enclosure_device *edev,
 {
 	unsigned char *desc;
 
-	if (!ses_page2_supported(edev)) {
-		ecomp->fault = 0;
-		return;
-	}
+	if (!ses_page2_supported(edev))
+		return 0;
+
 	desc = ses_get_page2_descriptor(edev, ecomp);
 	if (desc)
-		ecomp->fault = (desc[3] & 0x60) >> 4;
+		return (desc[3] & 0x60) >> 4;
+
+	return 0;
 }
 
 static int ses_set_fault(struct enclosure_device *edev,
@@ -255,13 +256,14 @@ static void ses_get_status(struct enclosure_device *edev,
 {
 	unsigned char *desc;
 
-	if (!ses_page2_supported(edev)) {
-		ecomp->status = 0;
-		return;
-	}
+	if (!ses_page2_supported(edev))
+		return 0;
+
 	desc = ses_get_page2_descriptor(edev, ecomp);
 	if (desc)
-		ecomp->status = (desc[0] & 0x0f);
+		return (desc[0] & 0x0f);
+
+	return 0;
 }
 
 static void ses_get_locate(struct enclosure_device *edev,
@@ -269,13 +271,14 @@ static void ses_get_locate(struct enclosure_device *edev,
 {
 	unsigned char *desc;
 
-	if (!ses_page2_supported(edev)) {
-		ecomp->locate = 0;
-		return;
-	}
+	if (!ses_page2_supported(edev))
+		return 0;
+
 	desc = ses_get_page2_descriptor(edev, ecomp);
 	if (desc)
-		ecomp->locate = (desc[2] & 0x02) ? 1 : 0;
+		return (desc[2] & 0x02) ? 1 : 0;
+
+	return 0;
 }
 
 static int ses_set_locate(struct enclosure_device *edev,
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 8d09c6d07bf1..b70e9deef3bc 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -52,12 +52,12 @@ enum enclosure_component_setting {
 struct enclosure_device;
 struct enclosure_component;
 struct enclosure_component_callbacks {
-	void (*get_status)(struct enclosure_device *,
+	int (*get_status)(struct enclosure_device *,
 			     struct enclosure_component *);
 	int (*set_status)(struct enclosure_device *,
 			  struct enclosure_component *,
 			  enum enclosure_status);
-	void (*get_fault)(struct enclosure_device *,
+	int (*get_fault)(struct enclosure_device *,
 			  struct enclosure_component *);
 	int (*set_fault)(struct enclosure_device *,
 			 struct enclosure_component *,
@@ -65,8 +65,8 @@ struct enclosure_component_callbacks {
 	int (*set_active)(struct enclosure_device *,
 			  struct enclosure_component *,
 			  enum enclosure_component_setting);
-	void (*get_locate)(struct enclosure_device *,
-			   struct enclosure_component *);
+	int (*get_locate)(struct enclosure_device *,
+			  struct enclosure_component *);
 	int (*set_locate)(struct enclosure_device *,
 			  struct enclosure_component *,
 			  enum enclosure_component_setting);
@@ -85,11 +85,7 @@ struct enclosure_component {
 	struct device *dev;
 	enum enclosure_component_type type;
 	int number;
-	int fault;
-	int active;
-	int locate;
 	int slot;
-	enum enclosure_status status;
 	int power_status;
 };
 
-- 
2.26.2

RE: [PATCH 2/3] misc: enclosure, ses: simplify some get callbacks

[Dan Williams]

Mariusz Tkaczyk wrote:
> Remove active, status, fault and locate variables from
> enclosure_component struct. Return then directly.
> No functional changes intended.

This looks ok although it's not a clear win on the diffstat. Does this
make the NPEM implementation easier to remove the indirection through
"struct enclosure_component" for reading fresh values? That would help
make the case.

Re: [PATCH 2/3] misc: enclosure, ses: simplify some get callbacks

[Mariusz Tkaczyk]

On Thu, 4 May 2023 16:58:35 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Mariusz Tkaczyk wrote:
> > Remove active, status, fault and locate variables from
> > enclosure_component struct. Return then directly.
> > No functional changes intended.  
> 
> This looks ok although it's not a clear win on the diffstat. Does this
> make the NPEM implementation easier to remove the indirection through
> "struct enclosure_component" for reading fresh values? That would help
> make the case.

I did that to familiarize better with this API. I determined that those values
can be just returned. They are refreshed every time on read in get_*led*(). I
believed that it makes implementation simpler for reader.

It could save me from some questions, "why not to reuse existing active,
fault, status variables" but it is no clear benefit.
It saves some memory because those variable probably won't be used in
NPEM/_DSM implementation (at least draft I left doesn't not use them).

If you don't see this valuable let me know, I can drop it.

Thanks,
Mariusz

Re: [PATCH 2/3] misc: enclosure, ses: simplify some get callbacks

[Dan Williams]

Mariusz Tkaczyk wrote:
> On Thu, 4 May 2023 16:58:35 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Mariusz Tkaczyk wrote:
> > > Remove active, status, fault and locate variables from
> > > enclosure_component struct. Return then directly.
> > > No functional changes intended.  
> > 
> > This looks ok although it's not a clear win on the diffstat. Does this
> > make the NPEM implementation easier to remove the indirection through
> > "struct enclosure_component" for reading fresh values? That would help
> > make the case.
> 
> I did that to familiarize better with this API. I determined that those values
> can be just returned. They are refreshed every time on read in get_*led*(). I
> believed that it makes implementation simpler for reader.
> 
> It could save me from some questions, "why not to reuse existing active,
> fault, status variables" but it is no clear benefit.
> It saves some memory because those variable probably won't be used in
> NPEM/_DSM implementation (at least draft I left doesn't not use them).
> 
> If you don't see this valuable let me know, I can drop it.

I think the problem with it is that it now requires a get_active()
implementation where one was not needed before, see my comment on
patch3. The attributes to cache the values allows a 'get' method to be
skipped when 'set'+caching is sufficient.

[PATCH 3/3] misc: enclosure: update sysfs api

[Mariusz Tkaczyk]

Use DEVICE_ATTR RW, RO and WO macros. Update function names
accordingly.
No functional changes intended.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 drivers/misc/enclosure.c | 69 +++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 00f50fd0cc85..ab2fd918ecf6 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -473,8 +473,8 @@ static const char *const enclosure_type[] = {
 	[ENCLOSURE_COMPONENT_ARRAY_DEVICE] = "array device",
 };
 
-static ssize_t get_component_fault(struct device *cdev,
-				   struct device_attribute *attr, char *buf)
+static ssize_t fault_show(struct device *cdev, struct device_attribute *attr,
+			  char *buf)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -485,9 +485,8 @@ static ssize_t get_component_fault(struct device *cdev,
 	return sysfs_emit(buf, "%d\n", status);
 }
 
-static ssize_t set_component_fault(struct device *cdev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t count)
+static ssize_t fault_store(struct device *cdev, struct device_attribute *attr,
+			   const char *buf, size_t count)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -498,8 +497,8 @@ static ssize_t set_component_fault(struct device *cdev,
 	return count;
 }
 
-static ssize_t get_component_status(struct device *cdev,
-				    struct device_attribute *attr,char *buf)
+static ssize_t status_show(struct device *cdev, struct device_attribute *attr,
+			   char *buf)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -510,9 +509,8 @@ static ssize_t get_component_status(struct device *cdev,
 	return sysfs_emit(buf, "%s\n", enclosure_status[status]);
 }
 
-static ssize_t set_component_status(struct device *cdev,
-				    struct device_attribute *attr,
-				    const char *buf, size_t count)
+static ssize_t status_store(struct device *cdev, struct device_attribute *attr,
+			    const char *buf, size_t count)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -533,9 +531,8 @@ static ssize_t set_component_status(struct device *cdev,
 		return -EINVAL;
 }
 
-static ssize_t set_component_active(struct device *cdev,
-				    struct device_attribute *attr,
-				    const char *buf, size_t count)
+static ssize_t active_store(struct device *cdev, struct device_attribute *attr,
+			    const char *buf, size_t count)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -546,8 +543,8 @@ static ssize_t set_component_active(struct device *cdev,
 	return count;
 }
 
-static ssize_t get_component_locate(struct device *cdev,
-				    struct device_attribute *attr, char *buf)
+static ssize_t locate_show(struct device *cdev, struct device_attribute *attr,
+			   char *buf)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -558,9 +555,8 @@ static ssize_t get_component_locate(struct device *cdev,
 	return sysfs_emit(buf, "%d\n", status);
 }
 
-static ssize_t set_component_locate(struct device *cdev,
-				    struct device_attribute *attr,
-				    const char *buf, size_t count)
+static ssize_t locate_store(struct device *cdev, struct device_attribute *attr,
+			    const char *buf, size_t count)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -571,9 +567,8 @@ static ssize_t set_component_locate(struct device *cdev,
 	return count;
 }
 
-static ssize_t get_component_power_status(struct device *cdev,
-					  struct device_attribute *attr,
-					  char *buf)
+static ssize_t power_status_show(struct device *cdev,
+				 struct device_attribute *attr, char *buf)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -588,9 +583,9 @@ static ssize_t get_component_power_status(struct device *cdev,
 	return sysfs_emit(buf, "%s\n", ecomp->power_status ? "on" : "off");
 }
 
-static ssize_t set_component_power_status(struct device *cdev,
-					  struct device_attribute *attr,
-					  const char *buf, size_t count)
+static ssize_t power_status_store(struct device *cdev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
 {
 	struct enclosure_device *edev = to_enclosure_device(cdev->parent);
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
@@ -610,16 +605,16 @@ static ssize_t set_component_power_status(struct device *cdev,
 	return count;
 }
 
-static ssize_t get_component_type(struct device *cdev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t type_show(struct device *cdev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
 
 	return sysfs_emit(buf, "%s\n", enclosure_type[ecomp->type]);
 }
 
-static ssize_t get_component_slot(struct device *cdev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t slot_show(struct device *cdev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct enclosure_component *ecomp = to_enclosure_component(cdev);
 	int slot;
@@ -633,17 +628,13 @@ static ssize_t get_component_slot(struct device *cdev,
 	return sysfs_emit(buf, "%d\n", slot);
 }
 
-static DEVICE_ATTR(fault, S_IRUGO | S_IWUSR, get_component_fault,
-		    set_component_fault);
-static DEVICE_ATTR(status, S_IRUGO | S_IWUSR, get_component_status,
-		   set_component_status);
-static DEVICE_ATTR(active, S_IWUSR, NULL, set_component_active);
-static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
-		   set_component_locate);
-static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR, get_component_power_status,
-		   set_component_power_status);
-static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL);
-static DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
+static DEVICE_ATTR_RW(fault);
+static DEVICE_ATTR_RW(status);
+static DEVICE_ATTR_WO(active);
+static DEVICE_ATTR_RW(locate);
+static DEVICE_ATTR_RW(power_status);
+static DEVICE_ATTR_RO(type);
+static DEVICE_ATTR_RO(slot);
 
 static struct attribute *enclosure_component_attrs[] = {
 	&dev_attr_fault.attr,
-- 
2.26.2

RE: [PATCH 3/3] misc: enclosure: update sysfs api

[Dan Williams]

Mariusz Tkaczyk wrote:
> Use DEVICE_ATTR RW, RO and WO macros. Update function names
> accordingly.
> No functional changes intended.

This looks like a nice cleanup to me, but I think you missed that
ses_set_active() updates ecomp->active. So even though
ses_enclosure_callbacks() does not publish get_active() it is still the
case that active needs to be DEVICE_ATTR_RW(). In fact with your change
in patch2 it now *requires* that ses_enclosure_callbacks() adds a
"get_active()" implementation. In that case it's going to need to cache
the last active setting which basically means dropping patch2 because it
seems more awkward to require "get" callbacks when cached value from the
last "set" is sufficient.

Re: [PATCH 0/3] Enclosure sysfs refactor

[Mariusz Tkaczyk]

On Thu, 17 Nov 2022 17:34:04 +0100
Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote:

> Hi Bjorn,
> I agreed with Stuart to take over the NPEM implementation[1].
> First part I want to share is a small refactor around enclosure interface.
> 
> The one sysfs change introduced is changing active LED to write-only.
> get_active() callback is not implemented for SES which is the
> only one enclosure API consumer now.
> 
> [1]
> https://lore.kernel.org/linux-pci/cover.1643822289.git.stuart.w.hayes@gmail.com/
> 
> Mariusz Tkaczyk (3):
>   misc: enclosure: remove get_active() callback
>   misc: enclosure, ses: simplify some get callbacks
>   misc: enclosure: update sysfs api
> 
>  drivers/misc/enclosure.c  | 96 ++++++++++++++++-----------------------
>  drivers/scsi/ses.c        | 33 ++++++++------
>  include/linux/enclosure.h | 14 ++----
>  3 files changed, 61 insertions(+), 82 deletions(-)
> 

Hi Bjorn,
Could you please take a look? Let me know if you against this cleanup.
I would like to get back to NPEM, I based my patches on top of it.

Thanks,
Mariusz

Re: [PATCH 0/3] Enclosure sysfs refactor

[Dan Williams]

Mariusz Tkaczyk wrote:
> On Thu, 17 Nov 2022 17:34:04 +0100
> Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote:
> 
> > Hi Bjorn,
> > I agreed with Stuart to take over the NPEM implementation[1].
> > First part I want to share is a small refactor around enclosure interface.
> > 
> > The one sysfs change introduced is changing active LED to write-only.
> > get_active() callback is not implemented for SES which is the
> > only one enclosure API consumer now.
> > 
> > [1]
> > https://lore.kernel.org/linux-pci/cover.1643822289.git.stuart.w.hayes@gmail.com/
> > 
> > Mariusz Tkaczyk (3):
> >   misc: enclosure: remove get_active() callback
> >   misc: enclosure, ses: simplify some get callbacks
> >   misc: enclosure: update sysfs api
> > 
> >  drivers/misc/enclosure.c  | 96 ++++++++++++++++-----------------------
> >  drivers/scsi/ses.c        | 33 ++++++++------
> >  include/linux/enclosure.h | 14 ++----
> >  3 files changed, 61 insertions(+), 82 deletions(-)
> > 
> 
> Hi Bjorn,
> Could you please take a look? Let me know if you against this cleanup.
> I would like to get back to NPEM, I based my patches on top of it.

Hi Mariusz,

I don't expect Bjorn to handle enclosure updates. I expect the upstream
path for these patches is through the SCSI tree. Going forward you can
use get_maintainer.pl for this routing information.

$ ./scripts/get_maintainer.pl drivers/scsi/ses.c 
"James E.J. Bottomley" <jejb@linux.ibm.com> (maintainer:SCSI SUBSYSTEM)
"Martin K. Petersen" <martin.petersen@oracle.com> (maintainer:SCSI SUBSYSTEM)
linux-scsi@vger.kernel.org (open list:SCSI SUBSYSTEM)
linux-kernel@vger.kernel.org (open list)

I'll take a look and then you can resend to linux-scsi. I do like this
stat line:

3 files changed, 61 insertions(+), 82 deletions(-)

...makes it compelling to take a look.

Re: [PATCH 0/3] Enclosure sysfs refactor

[Mariusz Tkaczyk]

On Thu, 4 May 2023 10:16:36 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Mariusz Tkaczyk wrote:
> > On Thu, 17 Nov 2022 17:34:04 +0100
> > Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote:
> >   
> > > Hi Bjorn,
> > > I agreed with Stuart to take over the NPEM implementation[1].
> > > First part I want to share is a small refactor around enclosure interface.
> > > 
> > > The one sysfs change introduced is changing active LED to write-only.
> > > get_active() callback is not implemented for SES which is the
> > > only one enclosure API consumer now.
> > > 
> > > [1]
> > > https://lore.kernel.org/linux-pci/cover.1643822289.git.stuart.w.hayes@gmail.com/
> > > 
> > > Mariusz Tkaczyk (3):
> > >   misc: enclosure: remove get_active() callback
> > >   misc: enclosure, ses: simplify some get callbacks
> > >   misc: enclosure: update sysfs api
> > > 
> > >  drivers/misc/enclosure.c  | 96 ++++++++++++++++-----------------------
> > >  drivers/scsi/ses.c        | 33 ++++++++------
> > >  include/linux/enclosure.h | 14 ++----
> > >  3 files changed, 61 insertions(+), 82 deletions(-)
> > >   
> > 
> > Hi Bjorn,
> > Could you please take a look? Let me know if you against this cleanup.
> > I would like to get back to NPEM, I based my patches on top of it.  
> 
> Hi Mariusz,
> 
> I don't expect Bjorn to handle enclosure updates. I expect the upstream
> path for these patches is through the SCSI tree. Going forward you can
> use get_maintainer.pl for this routing information.
> 
> $ ./scripts/get_maintainer.pl drivers/scsi/ses.c 
> "James E.J. Bottomley" <jejb@linux.ibm.com> (maintainer:SCSI SUBSYSTEM)
> "Martin K. Petersen" <martin.petersen@oracle.com> (maintainer:SCSI SUBSYSTEM)
> linux-scsi@vger.kernel.org (open list:SCSI SUBSYSTEM)
> linux-kernel@vger.kernel.org (open list)
> 
> I'll take a look and then you can resend to linux-scsi. I do like this
> stat line:
> 
> 3 files changed, 61 insertions(+), 82 deletions(-)
> 
> ...makes it compelling to take a look.

Ohh...my bad. Apologize for noise. Thanks Dan for clarification.

Mariusz