Nice patch Gabriel! Some questions/comments below:
On 03/29/2016 11:02 AM, Gabriel Krisman Bertazi wrote:
> Some new external drawers like ESLS have an internal clock, used by the
> internal microcode. This patch enables iprinit to configure the clock
> at boot time, and allow users to fetch and override this configuration
> in iprconfig.
>
> Signed-off-by: Gabriel Krisman Bertazi <kris...@linux.vnet.ibm.com>
> ---
> iprconfig.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> iprlib.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> iprlib.h | 21 +++++++++++++++++++++
> 3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/iprconfig.c b/iprconfig.c
> index a6b1372..89b6f4a 100644
> --- a/iprconfig.c
> +++ b/iprconfig.c
> @@ -27,7 +27,7 @@
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> -
> +#include <time.h>
> #include <math.h>
>
> char *tool_name = "iprconfig";
> @@ -19149,6 +19149,54 @@ static int ssd_report(char **args, int num_args)
> return 0;
> }
>
> +static int get_ses_time(char **args, int num_args)
> +{
> + struct ipr_dev *dev = find_dev(args[0]);
> +
> + time_t timestamp;
> + int origin;
> + char* origin_str[] = { "Not Configured",
> + "Configured" };
> + if (!dev) {
> + fprintf(stderr, "Cannot find %s\n", args[0]);
> + return -EINVAL;
> + }
> +
> + if (ipr_ses_get_time(dev, ×tamp, &origin))
> + scsi_err(dev, "%s, Can't read enclosure time\n", dev->gen_name);
> +
> + /* ipr_ses_get_time returns its value in miliseconds, while epoch is
> + measured in seconds. */
> + timestamp = timestamp/1000;
> +
> + printf ("%s(%s)\n", ctime(×tamp), origin_str[origin]);
> +}
Just curious here: the function is declared as int, so shouldn't it
return an int value in case it succeeds?
> +
> +static int set_ses_time(char **args, int num_args)
> +{
> + struct ipr_dev *dev = find_dev(args[0]);
> + struct tm tm;
> + time_t timestamp;
> +
> +
> + if (!dev) {
> + fprintf(stderr, "Cannot find %s\n", args[0]);
> + return -EINVAL;
> + }
> +
> + if (!strptime (args[1], "%F", &tm) || !strptime (args[2], "%T", &tm)) {
> + fprintf(stderr, "Date in the wrong format. Use \"YYYY-MM-DD
> HH:MM:SS\"\n");
> + return -EINVAL;
> + }
> +
> + timestamp = mktime(&tm) * 1000;
> + if (ipr_ses_set_time(dev, timestamp)) {
> + scsi_err(dev, "%s, Can't set enclosure time\n", dev->gen_name);
> + return -EIO;
> + }
> + return 0;
> +}
> +
> static const struct {
> char *cmd;
> int min_args;
> @@ -19270,6 +19318,8 @@ static const struct {
> { "resume-disk-enclosure", 1, 0, 1, resume_disk_enclosure,
> "sg8 "},
> { "show-perf", 1, 0, 1, show_perf, "sg8"},
> { "dump", 0, 0, 0, dump, "" },
> + { "get-ses-time", 1, 0, 1, get_ses_time, "sg8"},
> + { "set-ses-time", 3, 0, 3, set_ses_time, "sg8
> YYYY-MM-DD HH:MM:SS"},
> };
>
> /**
> diff --git a/iprlib.c b/iprlib.c
> index 70e2924..8534274 100644
> --- a/iprlib.c
> +++ b/iprlib.c
> @@ -9535,6 +9535,15 @@ static void init_ioa_dev(struct ipr_dev *dev)
> return;
> }
>
> +int init_ses_dev(struct ipr_dev *dev)
> +{
> + time_t t = time(NULL);
> + if (t != ((time_t) -1)) {
> + t *= 1000;
> + ipr_ses_set_time(dev, t);
> + }
> +}
Same question as above: why not returning? Perhaps worth to declare it
as void?
> +
> /**
> * ipr_init_dev -
> * @dev: ipr dev struct
> @@ -9561,6 +9570,9 @@ int ipr_init_dev(struct ipr_dev *dev)
> if (&dev->ioa->ioa == dev)
> init_ioa_dev(dev);
> break;
> + case TYPE_ENCLOSURE:
> + init_ses_dev(dev);
> + break;
> default:
> break;
> };
> @@ -10076,3 +10088,33 @@ int ipr_jbod_sysfs_bind(struct ipr_dev *dev, u8 op)
> close(fd);
> return 0;
> }
> +
> +int ipr_ses_get_time(struct ipr_dev *dev, u64* timestamp, int *origin)
> +{
> + struct ipr_ses_diag_page12 get_time;
> + int err;
> +
> + err = ipr_receive_diagnostics(dev, 0x12, &get_time, sizeof(get_time));
> + if (err)
> + return -EIO;
> +
> + *origin = !!get_time.timestamp_origin;
> + *timestamp = be64toh(*((u64*) get_time.timestamp)) >> 16;
> + return 0;
> +}
> +
> +int ipr_ses_set_time(struct ipr_dev *dev, u64 timestamp)
> +{
> + struct ipr_ses_diag_ctrl_page13 set_time;
> +
> + memset(&set_time, '\0', sizeof(set_time));
> +
> + set_time.page_code = 0x13;
> + set_time.page_length[1] = 8;
> +
> + timestamp = htobe64(timestamp << 16);
> + memcpy(set_time.timestamp, (char*) &
> + timestamp, 6);
> +
> + return ipr_send_diagnostics(dev, &set_time, sizeof(set_time));
> +}
> diff --git a/iprlib.h b/iprlib.h
> index b2a46c9..eccd116 100644
> --- a/iprlib.h
> +++ b/iprlib.h
> @@ -2646,6 +2646,25 @@ struct ipr_ses_inquiry_pageC3 {
> u8 reserved2;
> };
>
> +/* Page 12h - Get Time */
> +struct ipr_ses_diag_page12 {
> + u8 page_code;
> + u8 reserved1;
> + u8 page_length[2];
> + u8 timestamp_origin;
> + u8 reserved2;
> + u8 timestamp[6];
> +};
> +
> +/* Page 13h - Set Time */
> +struct ipr_ses_diag_ctrl_page13 {
> + u8 page_code;
> + u8 reserved1;
> + u8 page_length[2];
> + u8 timestamp[6];
> + u8 reserved[2];
> +};
> +
> struct ipr_sas_inquiry_pageC4 {
> u8 peri_qual_dev_type;
> u8 page_code;
> @@ -2909,6 +2928,8 @@ int ipr_improper_device_type(struct ipr_dev *);
> int ipr_get_fw_version(struct ipr_dev *, u8 release_level[4]);
> int ipr_get_live_dump(struct ipr_ioa *);
> int ipr_jbod_sysfs_bind(struct ipr_dev *, u8);
> +int ipr_ses_get_time(struct ipr_dev *dev, u64* timestamp, int *origin);
> +int ipr_ses_set_time(struct ipr_dev *dev, u64 timestamp);
Indentation seems weird here, but it's possibly my email client heheh
>
> static inline u32 ipr_get_dev_res_handle(struct ipr_ioa *ioa, struct
> ipr_dev_record *dev_rcd)
> {
>
Thanks,
Guilherme
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
