Re: [PATCH net-next v1 4/5] net: lan743x: reset the device on TX timeout

[Chris Babroski]

Hi Nicolai,

> Is it ok to re-register the PTP clock on a reset the driver starts by 
> itself?
> 
> ptp_clock_unregister() makes the open /dev/ptpN return -ENODEV, and
> since the old index stays taken while ptp4l holds it, the reopen comes
> back as a different /dev/ptpN. So ptp4l has to be restarted, which is
> more than "set up timestamping again"...
> 
> How about moving clock registration to probe instead, so it survives
> the reset? Then only the hw timestamping config would need to be put
> back.

It may be best to get Microchip's opinion about how this should be
handled. Woojung, can you answer this?

Makes sense that moving clock registration to probe and remove would
keep the PTP device valid across TX hang reset, but I'm not sure if
there are other consequences to PTP still being disabled and re-enabled
during reset.

I'll update the commit message to be more clear about ptp4l needing to
be restarted and try to run some tests to see what the effect is of
moving clock registration.

> > +	ret = lan743x_netdev_open(netdev);
> > +	if (ret) {
> > +		netdev_err(netdev, "failed to reopen after reset: %d\n", ret);
> > +		goto close;
> > +	}
> 
> Unrelated to this series, but lan743x_pm_resume() seems to ignore this
> return value.

This issue has been flagged by the Sashiko AI review bot twice as well
now. It would not be difficult to add similar error handling to
lan743x_pm_resume() so I can add that as a separate small patch when I
send the v2 series.

Thanks,
Chris