| Message ID | 20210224155802.13292-5-Viswas.G@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pm80xx updates | expand |
On Wed, Feb 24, 2021 at 4:48 PM Viswas G <Viswas.G@microchip.com> wrote: > > From: Vishakha Channapattan <vishakhavc@google.com> > > A new sysfs variable 'ctl_iop1_count' is being introduced that tells if > the controller is alive by indicating controller ticks. If on subsequent > run we see the ticks changing that indicates that controller is not > dead. > > Tested: Using 'ctl_iop1_count' sysfs variable we can see ticks > incrementing > mvae14:~# cat /sys/class/scsi_host/host*/ctl_iop1_count > IOP1TCNT=0x00000069 > IOP1TCNT=0x0000006b > IOP1TCNT=0x0000006d > IOP1TCNT=0x00000072 > > Signed-off-by: Vishakha Channapattan <vishakhavc@google.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > Signed-off-by: Ruksar Devadi <Ruksar.devadi@microchip.com> > Signed-off-by: Ashokkumar N <Ashokkumar.N@microchip.com> > Signed-off-by: Radha Ramachandran <radha@google.com> > --- > drivers/scsi/pm8001/pm8001_ctl.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c > index 8470bce2cee1..9bc9ef446801 100644 > --- a/drivers/scsi/pm8001/pm8001_ctl.c > +++ b/drivers/scsi/pm8001/pm8001_ctl.c > @@ -967,6 +967,30 @@ static ssize_t ctl_iop0_count_show(struct device *cdev, > } > static DEVICE_ATTR_RO(ctl_iop0_count); > > +/** > + * ctl_iop1_count_show - controller iop1 count check > + * @cdev: pointer to embedded class device > + * @buf: the buffer returned > + * > + * A sysfs 'read-only' shost attribute. > + */ > + > +static ssize_t ctl_iop1_count_show(struct device *cdev, > + struct device_attribute *attr, char *buf) > +{ > + struct Scsi_Host *shost = class_to_shost(cdev); > + struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost); > + struct pm8001_hba_info *pm8001_ha = sha->lldd_ha; > + unsigned int iop1cnt = 0; > + int c; > + > + pm8001_dbg(pm8001_ha, IOCTL, "%s\n", __func__); > + iop1cnt = pm8001_mr32(pm8001_ha->general_stat_tbl_addr, 20); > + c = sprintf(buf, "IOP1TCNT=0x%08x\n", iop1cnt); > + return c; > +} New file should use sysfs_emit instead of sprintf. > +static DEVICE_ATTR_RO(ctl_iop1_count); > + > struct device_attribute *pm8001_host_attrs[] = { > &dev_attr_interface_rev, > &dev_attr_controller_fatal_error, > @@ -993,6 +1017,7 @@ struct device_attribute *pm8001_host_attrs[] = { > &dev_attr_ctl_mpi_state, > &dev_attr_ctl_raae_count, > &dev_attr_ctl_iop0_count, > + &dev_attr_ctl_iop1_count, > NULL, > }; > > -- > 2.16.3 >
On 24/02/2021 15:57, Viswas G wrote: > From: Vishakha Channapattan <vishakhavc@google.com> > > A new sysfs variable 'ctl_iop1_count' is being introduced that tells if > the controller is alive by indicating controller ticks. If on subsequent > run we see the ticks changing that indicates that controller is not > dead. > Some comments, if you don't mind: > Tested: Using 'ctl_iop1_count' sysfs variable we can see ticks > incrementing > mvae14:~# cat /sys/class/scsi_host/host*/ctl_iop1_count > IOP1TCNT=0x00000069 why does this file not just hold the value, and rather print "IOP1TCNT=" as well? > IOP1TCNT=0x0000006b > IOP1TCNT=0x0000006d > IOP1TCNT=0x00000072 > > Signed-off-by: Vishakha Channapattan <vishakhavc@google.com> > Signed-off-by: Viswas G <Viswas.G@microchip.com> > Signed-off-by: Ruksar Devadi <Ruksar.devadi@microchip.com> > Signed-off-by: Ashokkumar N <Ashokkumar.N@microchip.com> > Signed-off-by: Radha Ramachandran <radha@google.com> > --- > drivers/scsi/pm8001/pm8001_ctl.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c > index 8470bce2cee1..9bc9ef446801 100644 > --- a/drivers/scsi/pm8001/pm8001_ctl.c > +++ b/drivers/scsi/pm8001/pm8001_ctl.c > @@ -967,6 +967,30 @@ static ssize_t ctl_iop0_count_show(struct device *cdev, > } > static DEVICE_ATTR_RO(ctl_iop0_count); > > +/** > + * ctl_iop1_count_show - controller iop1 count check > + * @cdev: pointer to embedded class device > + * @buf: the buffer returned > + * > + * A sysfs 'read-only' shost attribute. > + */ > + > +static ssize_t ctl_iop1_count_show(struct device *cdev, > + struct device_attribute *attr, char *buf) > +{ > + struct Scsi_Host *shost = class_to_shost(cdev); > + struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost); > + struct pm8001_hba_info *pm8001_ha = sha->lldd_ha; > + unsigned int iop1cnt = 0; no need to set an initial value > + int c; > + > + pm8001_dbg(pm8001_ha, IOCTL, "%s\n", __func__); strange that you require a debug message for something so simple > + iop1cnt = pm8001_mr32(pm8001_ha->general_stat_tbl_addr, 20); > + c = sprintf(buf, "IOP1TCNT=0x%08x\n", iop1cnt); > + return c; > +} > +static DEVICE_ATTR_RO(ctl_iop1_count); Seems like a lot of duplication in these functions > + > struct device_attribute *pm8001_host_attrs[] = { > &dev_attr_interface_rev, > &dev_attr_controller_fatal_error, > @@ -993,6 +1017,7 @@ struct device_attribute *pm8001_host_attrs[] = { > &dev_attr_ctl_mpi_state, > &dev_attr_ctl_raae_count, > &dev_attr_ctl_iop0_count, > + &dev_attr_ctl_iop1_count, > NULL, > }; > >
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c index 8470bce2cee1..9bc9ef446801 100644 --- a/drivers/scsi/pm8001/pm8001_ctl.c +++ b/drivers/scsi/pm8001/pm8001_ctl.c @@ -967,6 +967,30 @@ static ssize_t ctl_iop0_count_show(struct device *cdev, } static DEVICE_ATTR_RO(ctl_iop0_count); +/** + * ctl_iop1_count_show - controller iop1 count check + * @cdev: pointer to embedded class device + * @buf: the buffer returned + * + * A sysfs 'read-only' shost attribute. + */ + +static ssize_t ctl_iop1_count_show(struct device *cdev, + struct device_attribute *attr, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(cdev); + struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost); + struct pm8001_hba_info *pm8001_ha = sha->lldd_ha; + unsigned int iop1cnt = 0; + int c; + + pm8001_dbg(pm8001_ha, IOCTL, "%s\n", __func__); + iop1cnt = pm8001_mr32(pm8001_ha->general_stat_tbl_addr, 20); + c = sprintf(buf, "IOP1TCNT=0x%08x\n", iop1cnt); + return c; +} +static DEVICE_ATTR_RO(ctl_iop1_count); + struct device_attribute *pm8001_host_attrs[] = { &dev_attr_interface_rev, &dev_attr_controller_fatal_error, @@ -993,6 +1017,7 @@ struct device_attribute *pm8001_host_attrs[] = { &dev_attr_ctl_mpi_state, &dev_attr_ctl_raae_count, &dev_attr_ctl_iop0_count, + &dev_attr_ctl_iop1_count, NULL, };