[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] hwclock: add get/set parameters option
From: Andrej Picej <andrej.picej () norik ! com>
Date: 2023-07-06 5:05:42
Message-ID: 1a05eb10-96db-ee6f-e0ca-80ac11ddc0f6 () norik ! com
[Download RAW message or body]
Hi David,
On 6. 07. 23 05:58, David Leonard wrote:
> On Mon, 3 Jul 2023, Andrej Picej wrote:
>> Gentle ping.
>>
>> On 5. 06. 23 10:57, Andrej Picej wrote:
>>> In kernel 5.16 special ioctls were introduced to get/set RTC parameters.
>>> Add option to get/set parameters into busybox version of hwclock.
>>> Implementation is similar to the one already used in linux-utils hwclock
>>> tool.
>>>
>>> Example of parameter get use:
>>> $ hwclock -g 2
>>> The RTC parameter 0x2 is set to 0x2.
>>> $ hwclock --param-get bsm
>>> The RTC parameter 0x2 is set to 0x2.
>>>
>>> Example of parameter set use:
>>> $ hwclock -p 2=1
>>> The RTC parameter 0x2 will be set to 0x1.
>>> $ hwclock -p bsm=2
>>> The RTC parameter 0x2 will be set to 0x2.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>> ---
>>> include/rtc_.h | 23 +++++++++
>>> util-linux/hwclock.c | 119 +++++++++++++++++++++++++++++++++++++++++--
>>> 2 files changed, 137 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/rtc_.h b/include/rtc_.h
>>> index 24ff536..4b8d9fc 100644
>>> --- a/include/rtc_.h
>>> +++ b/include/rtc_.h
>>> @@ -22,6 +22,11 @@ int rtc_xopen(const char **default_rtc, int flags)
>>> FAST_FUNC;
>>> void rtc_read_tm(struct tm *ptm, int fd) FAST_FUNC;
>>> time_t rtc_tm2time(struct tm *ptm, int utc) FAST_FUNC;
>>> +struct hwclock_param {
>>> + int id;
>>> + const char *name;
>>> + const char *help;
>>> +};
>>> /*
>>> * Everything below this point has been copied from linux/rtc.h
>>> @@ -46,6 +51,17 @@ struct linux_rtc_wkalrm {
>>> struct linux_rtc_time time; /* time the alarm is set to */
>>> };
>>> +struct rtc_param {
>>> + __u64 param;
>>> + union {
>>> + __u64 uvalue;
>>> + __s64 svalue;
>>> + __u64 ptr;
>>> + };
>>> + __u32 index;
>>> + __u32 __pad;
>>> +};
>>> +
>>> /*
>>> * ioctl calls that are permitted to the /dev/rtc interface, if
>>> * any of the RTC drivers are enabled.
>>> @@ -71,12 +87,19 @@ struct linux_rtc_wkalrm {
>>> #define RTC_WKALM_SET _IOW('p', 0x0f, struct linux_rtc_wkalrm)/*
>>> Set wakeup alarm*/
>>> #define RTC_WKALM_RD _IOR('p', 0x10, struct linux_rtc_wkalrm)/*
>>> Get wakeup alarm*/
>>> +#define RTC_PARAM_GET _IOW('p', 0x13, struct rtc_param) /* Get
>>> parameter */
>>> +#define RTC_PARAM_SET _IOW('p', 0x14, struct rtc_param) /* Set
>>> parameter */
>>> +
>>> /* interrupt flags */
>>> #define RTC_IRQF 0x80 /* any of the following is active */
>>> #define RTC_PF 0x40
>>> #define RTC_AF 0x20
>>> #define RTC_UF 0x10
>>> +# define RTC_PARAM_FEATURES 0
>>> +# define RTC_PARAM_CORRECTION 1
>>> +# define RTC_PARAM_BACKUP_SWITCH_MODE 2
>>> +
>>> POP_SAVED_FUNCTION_VISIBILITY
>>> #endif
>>> diff --git a/util-linux/hwclock.c b/util-linux/hwclock.c
>>> index 723b095..0a49671 100644
>>> --- a/util-linux/hwclock.c
>>> +++ b/util-linux/hwclock.c
>>> @@ -48,6 +48,19 @@
>>> # define LIBC_IS_MUSL 0
>>> #endif
>>> +static const struct hwclock_param hwclock_params[] =
>>> +{
>>> + { RTC_PARAM_FEATURES, "features", "supported features" },
>>> + { RTC_PARAM_CORRECTION, "correction", "time correction" },
>>> + { RTC_PARAM_BACKUP_SWITCH_MODE, "bsm", "backup switch mode" },
>>> + { }
>>> +};
>>> +
>>> +const struct hwclock_param *get_hwclock_params(void)
>>> +{
>>> + return hwclock_params;
>>> +}
>>> +
>>> /* diff code is disabled: it's not sys/hw clock diff, it's some
>>> useless
>>> * "time between hwclock was started and we saw CMOS tick" quantity.
>>> @@ -320,6 +333,89 @@ static void from_sys_clock(const char
>>> **pp_rtcname, int utc)
>>> close(rtc);
>>> }
>>> +static int resolve_rtc_param_alias(const char *alias, __u64 *value)
>>> +{
>>> + const struct hwclock_param *param = &hwclock_params[0];
>>> +
>>> + while (param->name) {
>>> + if (!strcmp(alias, param->name)) {
>
> Per docs/style-guide.txt, please use strcmp() == 0
>
Sorry, was not aware of that, will change in v2.
>>> + *value = param->id;
>>> + return 0;
>>> + }
>>> + param++;
>>> + }
>>> +
>>> + return 1;
>>> +}
>>> +
>>> +static void get_rtc_param(const char **pp_rtcname, const char
>>> *rtc_param)
>>> +{
>>> + int rtc;
>>> + struct rtc_param param = { .param = 0 };
>>> +
>>> + if (resolve_rtc_param_alias(rtc_param, ¶m.param) != 0) {
>>> + param.param = bb_strtoull(rtc_param, NULL, 0);
>>> + if (errno)
>>> + bb_error_msg_and_die("could not convert parameter name:
>>> '%s' to number", rtc_param);
>>> + }
>>> +
>>> + rtc = rtc_xopen(pp_rtcname, O_RDONLY);
>>> +
>>> + xioctl(rtc, RTC_PARAM_GET, ¶m);
>>> +
>>> + printf("The RTC parameter 0x%jx is set to 0x%jx.\n",
>>> + (uintmax_t) param.param, (uintmax_t) param.uvalue);
>>> +
>>> + if (ENABLE_FEATURE_CLEAN_UP)
>>> + close(rtc);
>>> +}
>>> +
>>> +static void set_rtc_param(const char **pp_rtcname, const char
>>> *rtc_param)
>>> +{
>>> + int rtc;
>>> + int ret = 1;
>>> + struct rtc_param param = { .param = 0 };
>>> + char *tok, *opt = xstrdup(rtc_param);
>>> +
>>> + /* handle param name */
>>> + tok = strtok(opt, "=");
>>> + if (resolve_rtc_param_alias(tok, ¶m.param) != 0) {
>>> + param.param = bb_strtoull(tok, NULL, 0);
>>> + if (errno) {
>>> + bb_error_msg("could not convert parameter name: '%s' to
>>> number", tok);
>>> + goto done;
>>> + }
>>> + }
>>> +
>>> + /* handle param value */
>>> + tok = strtok(NULL, "=");
>>> + if (!tok) {
>>> + bb_error_msg("expected <param>=<value>");
>>> + goto done;
>>> + }
>>> + param.uvalue = bb_strtoull(tok, NULL, 0);
>>> + if (errno) {
>>> + bb_error_msg("could not convert parameter value to number");
>>> + goto done;
>
> Could possibly replace all these {bb_error_msg();goto done;} with
> bb_error_msg_and_die(), then kill ret?
>
Yeah sure, resources are freed with process termination. Will change in v2.
>>> + }
>>> +
>>> + rtc = rtc_xopen(pp_rtcname, O_WRONLY);
>>> +
>>> + printf("The RTC parameter 0x%jx will be set to 0x%jx.\n",
>>> + (uintmax_t) param.param, (uintmax_t) param.uvalue);
>>> +
>>> + xioctl(rtc, RTC_PARAM_SET, ¶m);
>>> +
>>> + if (ENABLE_FEATURE_CLEAN_UP)
>>> + close(rtc);
>>> +
>>> + ret = 0;
>>> +done:
>>> + free(opt);
>>> + if (ret)
>>> + bb_error_msg_and_die("error: %d", ret);
>>> +}
>>> +
>>> // hwclock from util-linux 2.36.1
>>> // hwclock [function] [option...]
>>> //Functions:
>>> @@ -346,10 +442,10 @@ static void from_sys_clock(const char
>>> **pp_rtcname, int utc)
>>> //usage:#define hwclock_trivial_usage
>>> //usage: IF_LONG_OPTS(
>>> -//usage: "[-swul] [--systz] [-f DEV]"
>>> +//usage: "[-swul] [--systz] [--param-get PARAM] [--param-set
>>> PARAM=VAL] [-f DEV]"
>>> //usage: )
>>> //usage: IF_NOT_LONG_OPTS(
>>> -//usage: "[-swult] [-f DEV]"
>>> +//usage: "[-swult] [-g PARAM] [-p PARAM=VAL] [-f DEV]"
>>> //usage: )
>>> //usage:#define hwclock_full_usage "\n\n"
>>> //usage: "Show or set hardware clock (RTC)\n"
>>> @@ -360,6 +456,8 @@ static void from_sys_clock(const char
>>> **pp_rtcname, int utc)
>>> //usage: IF_LONG_OPTS(
>>> //usage: "\n --systz Set in-kernel timezone, correct
>>> system time"
>>> //usage: "\n if RTC is kept in local time"
>>> +//usage: "\n --param-get PARAM Get RTC parameter"
>>> +//usage: "\n --param-set PARAM=VAL Set RTC parameter"
>>> //usage: )
>>> //usage: "\n -f DEV Use specified device (e.g. /dev/rtc2)"
>>> //usage: "\n -u Assume RTC is kept in UTC"
>>> @@ -375,11 +473,14 @@ static void from_sys_clock(const char
>>> **pp_rtcname, int utc)
>>> #define HWCLOCK_OPT_SYSTOHC 0x10
>>> #define HWCLOCK_OPT_SYSTZ 0x20
>>> #define HWCLOCK_OPT_RTCFILE 0x40
>>> +#define HWCLOCK_OPT_PARAM_GET 0x80
>>> +#define HWCLOCK_OPT_PARAM_SET 0x100
>>> int hwclock_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>>> int hwclock_main(int argc UNUSED_PARAM, char **argv)
>>> {
>>> const char *rtcname = NULL;
>>> + const char *param = NULL;
>>> unsigned opt;
>>> int utc;
>>> #if ENABLE_LONG_OPTS
>>> @@ -391,14 +492,18 @@ int hwclock_main(int argc UNUSED_PARAM, char
>>> **argv)
>>> "systohc\0" No_argument "w"
>>> "systz\0" No_argument "t" /* short opt is non-standard */
>>> "rtc\0" Required_argument "f"
>>> + "param-get\0" Required_argument "g" /* short opt is
>>> non-standard */
>>> + "param-set\0" Required_argument "p" /* short opt is
>>> non-standard */
>
> Could this be big enough to warrant a feature guard?
> e.g. FEATURE_HWCLOCK_SUPPORT_PARAM
>
I don't think this is necessary since this should be a default, and the
code for this is really not that big. Do you agree?
Thanks for the review,
Andrej
>>> ;
>>> #endif
>>> opt = getopt32long(argv,
>>> - "^""lurswtf:v" /* -v is accepted and ignored */
>>> + "^""lurswtf:g:p:v" /* -v is accepted and ignored */
>>> "\0"
>>> - "r--wst:w--rst:s--wrt:t--rsw:l--u:u--l",
>>> + "r--wstgp:w--rstgp:s--wrtgp:t--rswgp:g--rswtp:p--rswtg:l--u:u--l",
>>> hwclock_longopts,
>>> - &rtcname
>>> + &rtcname,
>>> + ¶m,
>>> + ¶m
>>> );
>>> /* If -u or -l wasn't given, check if we are using utc */
>>> @@ -413,6 +518,10 @@ int hwclock_main(int argc UNUSED_PARAM, char
>>> **argv)
>>> from_sys_clock(&rtcname, utc);
>>> else if (opt & HWCLOCK_OPT_SYSTZ)
>>> set_kernel_timezone_and_clock(utc, NULL);
>>> + else if (opt & HWCLOCK_OPT_PARAM_GET)
>>> + get_rtc_param(&rtcname, param);
>>> + else if (opt & HWCLOCK_OPT_PARAM_SET)
>>> + set_rtc_param(&rtcname, param);
>>> else
>>> /* default HWCLOCK_OPT_SHOW */
>>> show_clock(&rtcname, utc);
>> _______________________________________________
>> busybox mailing list
>> busybox@busybox.net
>> http://lists.busybox.net/mailman/listinfo/busybox
>>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic