netdev - Re: [PATCH v2 2/2] s390/time: Add PtP driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241016183746.25478-A-hca@linux.ibm.com>
Date: Wed, 16 Oct 2024 20:37:46 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Sven Schnelle <svens@...ux.ibm.com>
Cc: Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Richard Cochran <richardcochran@...il.com>,
        "Ricardo B. Marliere" <ricardo@...liere.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH v2 2/2] s390/time: Add PtP driver

On Wed, Oct 16, 2024 at 01:53:00PM +0200, Sven Schnelle wrote:
> Add a small PtP driver which allows user space to get
> the values of the physical and tod clock. This allows
> programs like chrony to use STP as clock source and
> steer the kernel clock. The physical clock can be used
> as a debugging aid to get the clock without any additional
> offsets like STP steering or LPAR offset.
> 
> Signed-off-by: Sven Schnelle <svens@...ux.ibm.com>
> ---
>  MAINTAINERS                   |   6 ++
>  arch/s390/include/asm/stp.h   |   1 +
>  arch/s390/include/asm/timex.h |   6 ++
>  arch/s390/kernel/time.c       |   6 ++
>  drivers/ptp/Kconfig           |  11 +++
>  drivers/ptp/Makefile          |   1 +
>  drivers/ptp/ptp_s390.c        | 129 ++++++++++++++++++++++++++++++++++
>  7 files changed, 160 insertions(+)
>  create mode 100644 drivers/ptp/ptp_s390.c

...

> +static __always_inline unsigned long eitod_to_ns(u128 todval)
> +{
> +	return (todval * 125) >> 9;
> +}

This should return u128 so the caller gets a non-truncated return value.

> +static struct timespec64 eitod_to_timespec64(union tod_clock *clk)
> +{
> +	return ns_to_timespec64(eitod_to_ns(clk->eitod) - TOD_UNIX_EPOCH);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Misplaced braces. I guess you want:

	return ns_to_timespec64(eitod_to_ns(clk->eitod - TOD_UNIX_EPOCH));

> +static struct ptp_clock_info ptp_s390_stcke_info = {
> +	.owner		= THIS_MODULE,
> +	.name		= "IBM s390 STCKE Clock",

Please, as written before make this simply "s390 STCKE Clock".

> +static struct ptp_clock_info ptp_s390_qpt_info = {
> +	.owner		= THIS_MODULE,
> +	.name		= "IBM s390 Physical Clock",

"s390 Physical Clock"

> +MODULE_AUTHOR("Sven Schnelle <svens@...ux.ibm.com>");
> +MODULE_DESCRIPTION("S390 Physical/STCKE Clock PtP Driver");

"s390 Physical/STCKE Clock PtP Driver"