On 2/6/2025 6:45 PM, Thomas Weißschuh wrote:

If vmlock_ptp_register() fails during probing, vmclock_remove() is

Typo: you missed 'c' in function name - vmclock_ptp_register

called to clean up the ptp clock and misc device. It uses dev_get_drvdata() to access the vmclock state. However the driver data is not yet set at this point.

Assign the driver data earlier.

Fixes: 205032724226 ("ptp: Add support for the AMZNC10C 'vmclock' device") Cc: stable@vger.kernel.org Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de

---

drivers/ptp/ptp_vmclock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c index 0a2cfc8ad3c5408a87fd8fedeff274ab895de3dd..1920698ae6eba6abfff5b61afae1b047910026fd 100644 --- a/drivers/ptp/ptp_vmclock.c +++ b/drivers/ptp/ptp_vmclock.c @@ -524,6 +524,8 @@ static int vmclock_probe(struct platform_device *pdev) goto out; }

• dev_set_drvdata(dev, st);
• if (le32_to_cpu(st->clk->magic) != VMCLOCK_MAGIC || le32_to_cpu(st->clk->size) > resource_size(&st->res) || le16_to_cpu(st->clk->version) != 1) {

@@ -587,8 +589,6 @@ static int vmclock_probe(struct platform_device *pdev) (st->miscdev.minor && st->ptp_clock) ? ", " : "", st->ptp_clock ? "PTP" : "");

• dev_set_drvdata(dev, st);
• out: return ret; }

Nice catch! The code looks good but please fix the typo in the commit msg, as this may be misleading. When applied please add my:

Reviewed-by: Mateusz Polchlopek mateusz.polchlopek@intel.com

On Fri, 2025-02-07 at 08:13 +0100, Mateusz Polchlopek wrote:

On 2/6/2025 6:45 PM, Thomas Weißschuh wrote:

...

Some error handling issues I noticed while looking at the code.

Only compile-tested.

Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de

Thomas Weißschuh (4):        ptp: vmclock: Set driver data before its usage        ptp: vmclock: Don't unregister misc device if it was not registered        ptp: vmclock: Clean up miscdev and ptp clock through devres        ptp: vmclock: Remove goto-based cleanup logic

drivers/ptp/ptp_vmclock.c | 46 ++++++++++++++++++++--------------------------   1 file changed, 20 insertions(+), 26 deletions(-)

---

base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20250206-vmclock-probe-57cbcb770925

Best regards,

As those all are fixes and cleanups then I think it should be tagged to net not net-next.

Agreed. Thanks, Thomas. For all four:

Reviewed-by: David Woodhouse dwmw@amazon.co.uk

I'm about to post a fifth which adds a .owner to vmclock_miscdev_fops.

Tested with the existing '-device vmclock' support in QEMU, plus this hack to actually expose a PTP clock to the guest (which we haven't worked out how to do *properly* from the timekeeping subsystem of a Linux host yet; qv).

--- a/hw/acpi/vmclock.c +++ b/hw/acpi/vmclock.c @@ -151,6 +151,18 @@ static void vmclock_realize(DeviceState *dev, Error **errp)

qemu_register_reset(vmclock_handle_reset, vms);

+ vms->clk->time_type = VMCLOCK_TIME_TAI; + vms->clk->flags = VMCLOCK_FLAG_TAI_OFFSET_VALID; + vms->clk->tai_offset_sec = -3600; + vms->clk->clock_status = VMCLOCK_STATUS_SYNCHRONIZED; + vms->clk->counter_value = 0; + vms->clk->counter_id = VMCLOCK_COUNTER_X86_TSC; + vms->clk->time_sec = 1704067200; + vms->clk->time_frac_sec = 0x8000000000000000ULL; + vms->clk->counter_period_frac_sec = 0x1a6e39b3e0ULL; + vms->clk->counter_period_shift = 4; + //vms->clk->counter_period_frac_sec = 0x1934c67f9b2ce6ULL; + vmclock_update_guest(vms); }