On Wed, 28 Jun, 2023 03:16:43 +0200 Andrew Lunn [off-list ref] wrote:

On Tue, Jun 27, 2023 at 04:21:39PM -0700, Rahul Rameshbabu wrote:

quoted

The .adjphase operation is an operation that is implemented only by certain
PHCs. The sysfs device attribute node for querying the maximum phase
adjustment supported should not be exposed on devices that do not support
.adjphase.

Fixes: c3b60ab7a4df ("ptp: Add .getmaxphase callback to ptp_clock_info")
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Naresh Kamboju <redacted>
Reported-by: Linux Kernel Functional Testing <redacted>
Link: https://lore.kernel.org/netdev/20230627162146.GA114473@dev-arch.thelio-3990X/ (local)
Link: https://lore.kernel.org/all/CA+G9fYtKCZeAUTtwe69iK8Xcz1mOKQzwcy49wd+imZrfj6ifXA@mail.gmail.com/ (local)

I think Signed-off-by should be last.

I can agree with this. Follows the "real route" of the patch, reports
came in and then the patch was implemented and signed-off.

quoted

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 77219cdcd683..6e4d5456a885 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c

@@ -358,6 +358,9 @@ static umode_t ptp_is_attribute_visible(struct kobject *kobj,
 		   attr == &dev_attr_max_vclocks.attr) {
 		if (ptp->is_virtual_clock)
 			mode = 0;
+	} else if (attr == &dev_attr_max_phase_adjustment.attr) {
+		if (!info->adjphase || !info->getmaxphase)
+			mode = 0;

Maybe it is time to turn this into a switch statement?

I agree. However, I do not want to conflate two separate things being a
bugfix and a cleanup. I think we can do one of two options.

  1. We can take this patch as is, but I submit a subsequent cleanup patch for
  this.
  2. I can turn this into a two patch submission where the first patch
  does the conversion.

I am a fan of the first option personally.

I also wounder if this really is something for net. How do you think
this patch matches against the stable rules?

Apologize in advance but not sure I am following along. The commit for
the patch the introduces the problematic logic has made its way to net
and this patch is a fix. Therefore, isn't net the right tree to target?

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=c3b60ab7a4dff6e6e608e685b70ddc3d6b2aca81

    Andrew

Thanks,

Rahul Rameshbabu