Received-SPF: pass (google.com: domain of linux-kernel+bounces-203303-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Thomas Gleixner <tglx@linutronix.de>
To: lakshmi.sowjanya.d@intel.com, giometti@enneenne.com, corbet@lwn.net,
 linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Cc: gregkh@linuxfoundation.org, andriy.shevchenko@linux.intel.com,
 eddie.dong@intel.com, christopher.s.hall@intel.com, pandith.n@intel.com,
 subramanian.mohan@intel.com, thejesh.reddy.t.r@intel.com,
 lakshmi.sowjanya.d@intel.com
Subject: Re: [PATCH v9 1/3] pps: generators: Add PPS Generator TIO Driver
In-Reply-To: <20240605153554.11584-2-lakshmi.sowjanya.d@intel.com>
References: <20240605153554.11584-1-lakshmi.sowjanya.d@intel.com>
 <20240605153554.11584-2-lakshmi.sowjanya.d@intel.com>
Date: Wed, 05 Jun 2024 23:34:28 +0200
Message-ID: <871q5b3wgr.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On Wed, Jun 05 2024 at 21:05, lakshmi.sowjanya.d@intel.com wrote:
> +static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
> +			    size_t count)
> +{
> +	struct pps_tio *tio = dev_get_drvdata(dev);
> +	bool enable;
> +	int err;
> +
> +	if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> +		dev_err(dev, "PPS cannot be used as clock is not related to ART");

                dev_err_once() if at all

> +		return -EPERM;

Why -EPERM? This has nothing to do with permissions.

ENODEV or ENOTSUPPORTED perhaps.

> +static ssize_t enable_show(struct device *dev, struct device_attribute *devattr, char *buf)
> +{
> +	struct pps_tio *tio = dev_get_drvdata(dev);
> +	u32 ctrl;
> +
> +	ctrl = pps_tio_read(tio, TIOCTL);
> +	ctrl &= TIOCTL_EN;

Why reading the hardware instead of simply using tio->enabled?

> +
> +	return sysfs_emit(buf, "%u\n", ctrl);
> +}

Thanks,

        tglx