| Message ID | 20211008202353.1448570-7-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Register SCSI sysfs attributes earlier | expand |
Hey Bart, just a small nitpick here. On Fri, Oct 08, 2021 at 01:23:13PM -0700, Bart Van Assche wrote: > struct device supports attribute groups directly but does not support > struct device_attribute directly. Hence switch to attribute groups. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/s390/scsi/zfcp_ext.h | 4 +-- > drivers/s390/scsi/zfcp_scsi.c | 4 +-- > drivers/s390/scsi/zfcp_sysfs.c | 52 +++++++++++++++++++++++----------- > 3 files changed, 39 insertions(+), 21 deletions(-) > > diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h > index 6bc96d70254d..7b7b567186d6 100644 > --- a/drivers/s390/scsi/zfcp_ext.h > +++ b/drivers/s390/scsi/zfcp_ext.h > @@ -184,8 +184,8 @@ extern const struct attribute_group *zfcp_sysfs_adapter_attr_groups[]; > extern const struct attribute_group *zfcp_unit_attr_groups[]; > extern const struct attribute_group *zfcp_port_attr_groups[]; > extern struct mutex zfcp_sysfs_port_units_mutex; > -extern struct device_attribute *zfcp_sysfs_sdev_attrs[]; > -extern struct device_attribute *zfcp_sysfs_shost_attrs[]; > +extern const struct attribute_group *zfcp_sdev_attr_groups[]; > +extern const struct attribute_group *zfcp_shost_attr_groups[]; I'd prefer it, if you leave the `zfcp_sysfs_` intact; while not universally used in the whole driver, its a convention we've been trying to follow regarding the symbols declared by the driver in the recent time (`zfcp_sysfs.o` is the compilation unit).
On 10/11/21 6:26 AM, Benjamin Block wrote: > On Fri, Oct 08, 2021 at 01:23:13PM -0700, Bart Van Assche wrote: >> -extern struct device_attribute *zfcp_sysfs_sdev_attrs[]; >> -extern struct device_attribute *zfcp_sysfs_shost_attrs[]; >> +extern const struct attribute_group *zfcp_sdev_attr_groups[]; >> +extern const struct attribute_group *zfcp_shost_attr_groups[]; > > I'd prefer it, if you leave the `zfcp_sysfs_` intact; while not > universally used in the whole driver, its a convention we've been trying > to follow regarding the symbols declared by the driver in the recent > time (`zfcp_sysfs.o` is the compilation unit). Hi Benjamin, I will restore these prefixes and post a fourth version of this patch series. Thanks, Bart.
diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h index 6bc96d70254d..7b7b567186d6 100644 --- a/drivers/s390/scsi/zfcp_ext.h +++ b/drivers/s390/scsi/zfcp_ext.h @@ -184,8 +184,8 @@ extern const struct attribute_group *zfcp_sysfs_adapter_attr_groups[]; extern const struct attribute_group *zfcp_unit_attr_groups[]; extern const struct attribute_group *zfcp_port_attr_groups[]; extern struct mutex zfcp_sysfs_port_units_mutex; -extern struct device_attribute *zfcp_sysfs_sdev_attrs[]; -extern struct device_attribute *zfcp_sysfs_shost_attrs[]; +extern const struct attribute_group *zfcp_sdev_attr_groups[]; +extern const struct attribute_group *zfcp_shost_attr_groups[]; bool zfcp_sysfs_port_is_removing(const struct zfcp_port *const port); /* zfcp_unit.c */ diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c index 9da9b2b2a580..0b40e2331f97 100644 --- a/drivers/s390/scsi/zfcp_scsi.c +++ b/drivers/s390/scsi/zfcp_scsi.c @@ -444,8 +444,8 @@ static struct scsi_host_template zfcp_scsi_host_template = { /* report size limit per scatter-gather segment */ .max_segment_size = ZFCP_QDIO_SBALE_LEN, .dma_boundary = ZFCP_QDIO_SBALE_LEN - 1, - .shost_attrs = zfcp_sysfs_shost_attrs, - .sdev_attrs = zfcp_sysfs_sdev_attrs, + .shost_groups = zfcp_shost_attr_groups, + .sdev_groups = zfcp_sdev_attr_groups, .track_queue_depth = 1, .supported_mode = MODE_INITIATOR, }; diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c index b8cd75a872ee..fe1974658d82 100644 --- a/drivers/s390/scsi/zfcp_sysfs.c +++ b/drivers/s390/scsi/zfcp_sysfs.c @@ -672,17 +672,26 @@ ZFCP_DEFINE_SCSI_ATTR(zfcp_in_recovery, "%d\n", ZFCP_DEFINE_SCSI_ATTR(zfcp_status, "0x%08x\n", atomic_read(&zfcp_sdev->status)); -struct device_attribute *zfcp_sysfs_sdev_attrs[] = { - &dev_attr_fcp_lun, - &dev_attr_wwpn, - &dev_attr_hba_id, - &dev_attr_read_latency, - &dev_attr_write_latency, - &dev_attr_cmd_latency, - &dev_attr_zfcp_access_denied, - &dev_attr_zfcp_failed, - &dev_attr_zfcp_in_recovery, - &dev_attr_zfcp_status, +struct attribute *zfcp_sdev_attrs[] = { + &dev_attr_fcp_lun.attr, + &dev_attr_wwpn.attr, + &dev_attr_hba_id.attr, + &dev_attr_read_latency.attr, + &dev_attr_write_latency.attr, + &dev_attr_cmd_latency.attr, + &dev_attr_zfcp_access_denied.attr, + &dev_attr_zfcp_failed.attr, + &dev_attr_zfcp_in_recovery.attr, + &dev_attr_zfcp_status.attr, + NULL +}; + +static const struct attribute_group zfcp_sdev_attr_group = { + .attrs = zfcp_sdev_attrs +}; + +const struct attribute_group *zfcp_sdev_attr_groups[] = { + &zfcp_sdev_attr_group, NULL }; @@ -783,12 +792,21 @@ static ssize_t zfcp_sysfs_adapter_q_full_show(struct device *dev, } static DEVICE_ATTR(queue_full, S_IRUGO, zfcp_sysfs_adapter_q_full_show, NULL); -struct device_attribute *zfcp_sysfs_shost_attrs[] = { - &dev_attr_utilization, - &dev_attr_requests, - &dev_attr_megabytes, - &dev_attr_seconds_active, - &dev_attr_queue_full, +static struct attribute *zfcp_sysfs_shost_attrs[] = { + &dev_attr_utilization.attr, + &dev_attr_requests.attr, + &dev_attr_megabytes.attr, + &dev_attr_seconds_active.attr, + &dev_attr_queue_full.attr, + NULL +}; + +static const struct attribute_group zfcp_shost_attr_group = { + .attrs = zfcp_sysfs_shost_attrs +}; + +const struct attribute_group *zfcp_shost_attr_groups[] = { + &zfcp_shost_attr_group, NULL };
struct device supports attribute groups directly but does not support struct device_attribute directly. Hence switch to attribute groups. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/s390/scsi/zfcp_ext.h | 4 +-- drivers/s390/scsi/zfcp_scsi.c | 4 +-- drivers/s390/scsi/zfcp_sysfs.c | 52 +++++++++++++++++++++++----------- 3 files changed, 39 insertions(+), 21 deletions(-)