Re: [PATCH v2 2/2] s390/time: Add PtP driver
From: Heiko Carstens
Date: Wed Oct 16 2024 - 14:38:45 EST
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@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("S390 Physical/STCKE Clock PtP Driver");
"s390 Physical/STCKE Clock PtP Driver"