[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <861942e6-d1e9-4e6a-8d4c-d8fa4584ed45@linux.dev>
Date: Sat, 21 Jun 2025 21:24:52 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Thomas Gleixner <tglx@...utronix.de>, LKML <linux-kernel@...r.kernel.org>
Cc: Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org
Subject: Re: [patch 03/13] ptp: Split out PTP_PEROUT_REQUEST ioctl code
On 20/06/2025 14:24, Thomas Gleixner wrote:
> Continue the ptp_ioctl() cleanup by splitting out the PTP_PEROUT_REQUEST
> ioctl code into a helper function. Convert to a lock guard while at it.
>
> No functional change intended.
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> drivers/ptp/ptp_chardev.c | 129 ++++++++++++++++++++--------------------------
> 1 file changed, 58 insertions(+), 71 deletions(-)
>
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -223,6 +223,61 @@ static long ptp_extts_request(struct ptp
> return ops->enable(ops, &req, req.extts.flags & PTP_ENABLE_FEATURE ? 1 : 0);
> }
>
> +static long ptp_perout_request(struct ptp_clock *ptp, unsigned int cmd, void __user *arg)
> +{
> + struct ptp_clock_request req = { .type = PTP_CLK_REQ_PEROUT };
> + struct ptp_perout_request *perout = &req.perout;
> + struct ptp_clock_info *ops = ptp->info;
> +
> + if (copy_from_user(perout, arg, sizeof(*perout)))
> + return -EFAULT;
> +
> + if (cmd == PTP_PEROUT_REQUEST2) {
> + if (perout->flags & ~PTP_PEROUT_VALID_FLAGS)
> + return -EINVAL;
> +
> + /*
> + * The "on" field has undefined meaning if
> + * PTP_PEROUT_DUTY_CYCLE isn't set, we must still treat it
> + * as reserved, which must be set to zero.
> + */
> + if (!(perout->flags & PTP_PEROUT_DUTY_CYCLE) &&
> + !mem_is_zero(perout->rsv, sizeof(perout->rsv)))
> + return -EINVAL;
> +
> + if (perout->flags & PTP_PEROUT_DUTY_CYCLE) {
> + /* The duty cycle must be subunitary. */
> + if (perout->on.sec > perout->period.sec ||
> + (perout->on.sec == perout->period.sec &&
> + perout->on.nsec > perout->period.nsec))
> + return -ERANGE;
> + }
> +
> + if (perout->flags & PTP_PEROUT_PHASE) {
> + /*
> + * The phase should be specified modulo the period,
> + * therefore anything equal or larger than 1 period
> + * is invalid.
> + */
> + if (perout->phase.sec > perout->period.sec ||
> + (perout->phase.sec == perout->period.sec &&
> + perout->phase.nsec >= perout->period.nsec))
> + return -ERANGE;
> + }
> + } else {
> + perout->flags &= PTP_PEROUT_V1_VALID_FLAGS;
> + memset(perout->rsv, 0, sizeof(perout->rsv));
> + }
> +
> + if (perout->index >= ops->n_per_out)
> + return -EINVAL;
> + if (perout->flags & ~ops->supported_perout_flags)
> + return -EOPNOTSUPP;
> +
> + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &ptp->pincfg_mux)
> + return ops->enable(ops, &req, perout->period.sec || perout->period.nsec);
> +}
> +
> long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
> unsigned long arg)
> {
> @@ -262,77 +317,9 @@ long ptp_ioctl(struct posix_clock_contex
>
> case PTP_PEROUT_REQUEST:
> case PTP_PEROUT_REQUEST2:
> - if ((pccontext->fp->f_mode & FMODE_WRITE) == 0) {
> - err = -EACCES;
> - break;
> - }
> - memset(&req, 0, sizeof(req));
> -
> - if (copy_from_user(&req.perout, (void __user *)arg,
> - sizeof(req.perout))) {
> - err = -EFAULT;
> - break;
> - }
> - if (cmd == PTP_PEROUT_REQUEST2) {
> - struct ptp_perout_request *perout = &req.perout;
> -
> - if (perout->flags & ~PTP_PEROUT_VALID_FLAGS) {
> - err = -EINVAL;
> - break;
> - }
> - /*
> - * The "on" field has undefined meaning if
> - * PTP_PEROUT_DUTY_CYCLE isn't set, we must still treat
> - * it as reserved, which must be set to zero.
> - */
> - if (!(perout->flags & PTP_PEROUT_DUTY_CYCLE) &&
> - (perout->rsv[0] || perout->rsv[1] ||
> - perout->rsv[2] || perout->rsv[3])) {
> - err = -EINVAL;
> - break;
> - }
> - if (perout->flags & PTP_PEROUT_DUTY_CYCLE) {
> - /* The duty cycle must be subunitary. */
> - if (perout->on.sec > perout->period.sec ||
> - (perout->on.sec == perout->period.sec &&
> - perout->on.nsec > perout->period.nsec)) {
> - err = -ERANGE;
> - break;
> - }
> - }
> - if (perout->flags & PTP_PEROUT_PHASE) {
> - /*
> - * The phase should be specified modulo the
> - * period, therefore anything equal or larger
> - * than 1 period is invalid.
> - */
> - if (perout->phase.sec > perout->period.sec ||
> - (perout->phase.sec == perout->period.sec &&
> - perout->phase.nsec >= perout->period.nsec)) {
> - err = -ERANGE;
> - break;
> - }
> - }
> - } else if (cmd == PTP_PEROUT_REQUEST) {
> - req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS;
> - req.perout.rsv[0] = 0;
> - req.perout.rsv[1] = 0;
> - req.perout.rsv[2] = 0;
> - req.perout.rsv[3] = 0;
> - }
> - if (req.perout.index >= ops->n_per_out) {
> - err = -EINVAL;
> - break;
> - }
> - if (req.perout.flags & ~ptp->info->supported_perout_flags)
> - return -EOPNOTSUPP;
> - req.type = PTP_CLK_REQ_PEROUT;
> - enable = req.perout.period.sec || req.perout.period.nsec;
> - if (mutex_lock_interruptible(&ptp->pincfg_mux))
> - return -ERESTARTSYS;
> - err = ops->enable(ops, &req, enable);
> - mutex_unlock(&ptp->pincfg_mux);
> - break;
> + if ((pccontext->fp->f_mode & FMODE_WRITE) == 0)
> + return -EACCES;
> + return ptp_perout_request(ptp, cmd, argptr);
>
> case PTP_ENABLE_PPS:
> case PTP_ENABLE_PPS2:
>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Powered by blists - more mailing lists