> -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Sent: Wednesday, March 22, 2023 11:04 PM > To: Zhang, Tianfei <tianfei.zhang@xxxxxxxxx> > Cc: richardcochran@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux- > fpga@xxxxxxxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; Gomes, Vinicius > <vinicius.gomes@xxxxxxxxx>; pierre-louis.bossart@xxxxxxxxxxxxxxx; Pagani, Marco > <marpagan@xxxxxxxxxx>; Weight, Russell H <russell.h.weight@xxxxxxxxx>; > matthew.gerlach@xxxxxxxxxxxxxxx; nico@xxxxxxxxxxx; Khadatare, RaghavendraX > Anand <raghavendrax.anand.khadatare@xxxxxxxxx> > Subject: Re: [PATCH v2] ptp: add ToD device driver for Intel FPGA cards > > On Wed, Mar 22, 2023 at 02:59:21PM +0000, Zhang, Tianfei wrote: > > > -----Original Message----- > > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > > Sent: Wednesday, March 22, 2023 10:49 PM > > > To: Zhang, Tianfei <tianfei.zhang@xxxxxxxxx> > > > Cc: richardcochran@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux- > > > fpga@xxxxxxxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; Gomes, Vinicius > > > <vinicius.gomes@xxxxxxxxx>; pierre-louis.bossart@xxxxxxxxxxxxxxx; > > > Pagani, Marco <marpagan@xxxxxxxxxx>; Weight, Russell H > > > <russell.h.weight@xxxxxxxxx>; matthew.gerlach@xxxxxxxxxxxxxxx; > > > nico@xxxxxxxxxxx; Khadatare, RaghavendraX Anand > > > <raghavendrax.anand.khadatare@xxxxxxxxx> > > > Subject: Re: [PATCH v2] ptp: add ToD device driver for Intel FPGA > > > cards > > > > > > On Wed, Mar 22, 2023 at 10:35:47AM -0400, Tianfei Zhang wrote: > > > > Adding a DFL (Device Feature List) device driver of ToD device for > > > > Intel FPGA cards. > > > > > > > > The Intel FPGA Time of Day(ToD) IP within the FPGA DFL bus is > > > > exposed as PTP Hardware clock(PHC) device to the Linux PTP stack > > > > to synchronize the system clock to its ToD information using > > > > phc2sys utility of the Linux PTP stack. The DFL is a hardware List > > > > within FPGA, which defines a linked list of feature headers within > > > > the device MMIO space to provide an extensible way of adding subdevice > features. > > ... > > > > > + dt->ptp_clock = ptp_clock_register(&dt->ptp_clock_ops, dev); > > > > + if (IS_ERR_OR_NULL(dt->ptp_clock)) > > > > + return dev_err_probe(dt->dev, PTR_ERR_OR_ZERO(dt->ptp_clock), > > > > + "Unable to register PTP clock\n"); > > > > + > > > > + return 0; > > > > > > Can be as simple as: > > > > > > ret = PTR_ERR_OR_ZERO(dt->ptp_clock); > > > return dev_err_probe(dt->dev, ret, "Unable to register PTP > > > clock\n"); > > > > This should be : > > ret = PTR_ERR_OR_ZERO(dt->ptp_clock); > > if (ret) > > return dev_err_probe(dt->dev, ret, "Unable to register PTP clock\n"); > > return 0; > > It depends how you treat the NULL from ptp_clock_register() and why driver will be > still bound to the device even if it doesn't provide PTP facility. For this ToD DFL driver, it depends on CONFIG_PTP_1588_CLOCK, I will add this dependency in Kconfig file later as Ilpo pointed out. It will not occurred the NULL case for ptp_clock_register() in this driver, so it only handle the error case is enough.
