| Message ID | 20250206-vmclock-probe-v1-0-17a3ea07be34@linutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | ptp: vmclock: bugfixes and cleanups for error handling | expand |
On Thu, Feb 06, 2025 at 06:45:00PM +0100, 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> For the series: Acked-by: Richard Cochran <richardcochran@gmail.com>
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. thanks
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); }
On Fri, Feb 07, 2025 at 09:10:42AM +0000, David Woodhouse wrote: > 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: Ack. > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> Thanks. > I'm about to post a fifth which adds a .owner to vmclock_miscdev_fops. I assume you want me to include this in my series. > 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); > } > >
On Fri, 2025-02-07 at 10:25 +0100, Thomas Weißschuh wrote: > > I'm about to post a fifth which adds a .owner to vmclock_miscdev_fops. > > I assume you want me to include this in my series. If you do need to repost the series, yes please. Otherwise I've posted it as [PATCH 5/4] so hopefully it'll get swept up with the original set.
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,