| Message ID | 20220623005717.31040-4-vfedorenko@novek.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Create common DPLL/clock configuration API | expand |
On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: > From: Vadim Fedorenko <vadfed@fb.com> > > Implement DPLL operations in ptp_ocp driver. Please CC: me as well. > +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) > +{ > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > + int sync; > + > + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; > + return sync; > +} Please match existing code style. > +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) > +{ > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > + int ret; > + > + if (bp->sma[sma].mode != SMA_MODE_IN) > + return -1; > + > + switch (ptp_ocp_sma_get(bp, sma)) { > + case 0: > + ret = DPLL_TYPE_EXT_10MHZ; > + break; > + case 1: > + case 2: > + ret = DPLL_TYPE_EXT_1PPS; > + break; > + default: > + ret = DPLL_TYPE_INT_OSCILLATOR; > + } > + > + return ret; > +} These case statements switch on private bits. This needs to match on the selector name instead. > +static int ptp_ocp_dpll_get_output_type(struct dpll_device *dpll, int sma) > +{ > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > + int ret; > + > + if (bp->sma[sma].mode != SMA_MODE_OUT) > + return -1; > + > + switch (ptp_ocp_sma_get(bp, sma)) { > + case 0: > + ret = DPLL_TYPE_EXT_10MHZ; > + break; > + case 1: > + case 2: > + ret = DPLL_TYPE_INT_OSCILLATOR; > + break; > + case 4: > + case 8: > + ret = DPLL_TYPE_GNSS; > + break; > + default: > + ret = DPLL_TYPE_INT_OSCILLATOR; Missing break; > + } > + > + return ret; > +} > + > +static struct dpll_device_ops dpll_ops = { > + .get_status = ptp_ocp_dpll_get_status, > + .get_lock_status = ptp_ocp_dpll_get_lock_status, > + .get_source_type = ptp_ocp_dpll_get_source_type, > + .get_output_type = ptp_ocp_dpll_get_output_type, > +}; No 'set' statements here? Also, what happens if there is more than one GNSS receiver, how is this differentiated? > static int > ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > @@ -3768,6 +3846,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > ptp_ocp_info(bp); > devlink_register(devlink); > + > + bp->dpll = dpll_device_alloc(&dpll_ops, ARRAY_SIZE(bp->sma), ARRAY_SIZE(bp->sma), bp); > + if (!bp->dpll) { > + dev_err(&pdev->dev, "dpll_device_alloc failed\n"); > + return 0; > + } > + dpll_device_register(bp->dpll); > + How is the release/unregister path called when the module is unloaded?
On 23.06.2022 19:28, Jonathan Lemon wrote: > On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: >> From: Vadim Fedorenko <vadfed@fb.com> >> >> Implement DPLL operations in ptp_ocp driver. > > Please CC: me as well. > Sure, sorry for not doing it last. > >> +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int sync; >> + >> + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; >> + return sync; >> +} > > Please match existing code style. > > >> +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int ret; >> + >> + if (bp->sma[sma].mode != SMA_MODE_IN) >> + return -1; >> + >> + switch (ptp_ocp_sma_get(bp, sma)) { >> + case 0: >> + ret = DPLL_TYPE_EXT_10MHZ; >> + break; >> + case 1: >> + case 2: >> + ret = DPLL_TYPE_EXT_1PPS; >> + break; >> + default: >> + ret = DPLL_TYPE_INT_OSCILLATOR; >> + } >> + >> + return ret; >> +} > > These case statements switch on private bits. This needs to match > on the selector name instead. > Not sure that string comparison is a good idea. Maybe it's better to extend struct ocp_selector with netlink type id and fill it according to hardware? > >> +static int ptp_ocp_dpll_get_output_type(struct dpll_device *dpll, int sma) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int ret; >> + >> + if (bp->sma[sma].mode != SMA_MODE_OUT) >> + return -1; >> + >> + switch (ptp_ocp_sma_get(bp, sma)) { >> + case 0: >> + ret = DPLL_TYPE_EXT_10MHZ; >> + break; >> + case 1: >> + case 2: >> + ret = DPLL_TYPE_INT_OSCILLATOR; >> + break; >> + case 4: >> + case 8: >> + ret = DPLL_TYPE_GNSS; >> + break; >> + default: >> + ret = DPLL_TYPE_INT_OSCILLATOR; > > Missing break; > > >> + } >> + >> + return ret; >> +} >> + >> +static struct dpll_device_ops dpll_ops = { >> + .get_status = ptp_ocp_dpll_get_status, >> + .get_lock_status = ptp_ocp_dpll_get_lock_status, >> + .get_source_type = ptp_ocp_dpll_get_source_type, >> + .get_output_type = ptp_ocp_dpll_get_output_type, >> +}; > > No 'set' statements here? No, didn't implement it yet > Also, what happens if there is more than > one GNSS receiver, how is this differentiated? Good question. Haven't thought about such case, open for suggestions! >> static int >> ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> { >> @@ -3768,6 +3846,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> >> ptp_ocp_info(bp); >> devlink_register(devlink); >> + >> + bp->dpll = dpll_device_alloc(&dpll_ops, ARRAY_SIZE(bp->sma), ARRAY_SIZE(bp->sma), bp); >> + if (!bp->dpll) { >> + dev_err(&pdev->dev, "dpll_device_alloc failed\n"); >> + return 0; >> + } >> + dpll_device_register(bp->dpll); >> + > > How is the release/unregister path called when the module is unloaded? Thank you Jonathan for pointing it out. I've just realised that unregister path is messy in the subsystem, will polish it in the next version and add it to driver unload.
On Fri, Jun 24, 2022 at 12:11:43AM +0100, Vadim Fedorenko wrote: > On 23.06.2022 19:28, Jonathan Lemon wrote: > > On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: > > > From: Vadim Fedorenko <vadfed@fb.com> > > > +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) > > > +{ > > > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > > > + int ret; > > > + > > > + if (bp->sma[sma].mode != SMA_MODE_IN) > > > + return -1; > > > + > > > + switch (ptp_ocp_sma_get(bp, sma)) { > > > + case 0: > > > + ret = DPLL_TYPE_EXT_10MHZ; > > > + break; > > > + case 1: > > > + case 2: > > > + ret = DPLL_TYPE_EXT_1PPS; > > > + break; > > > + default: > > > + ret = DPLL_TYPE_INT_OSCILLATOR; > > > + } > > > + > > > + return ret; > > > +} > > > > These case statements switch on private bits. This needs to match > > on the selector name instead. > > > > Not sure that string comparison is a good idea. Maybe it's better to extend > struct ocp_selector with netlink type id and fill it according to hardware? Sure, that could be an option. But, as this is DPLL only, how does it handle things when a pin is used for non-clock IO? In the timecard, for example, we have the frequency counters for input, and the frequency generators/VCC/GND for output. Actually our HW has a multi-level input, where the DPLL selects its source from an internal mux - this isn't reflected here. The external pins feed into some complex HW logic, which performs its own priority calculations before presenting the end result as an available selection for the DPLL.
On 23.06.2022 19:28, Jonathan Lemon wrote: > On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: >> From: Vadim Fedorenko <vadfed@fb.com> >> >> +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int sync; >> + >> + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; >> + return sync; >> +} > > Please match existing code style. > Didn't get this point. The same code is used through out the driver. Could you please explain? > >> +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int ret; >> + >> + if (bp->sma[sma].mode != SMA_MODE_IN) >> + return -1; >> + >> + switch (ptp_ocp_sma_get(bp, sma)) { >> + case 0: >> + ret = DPLL_TYPE_EXT_10MHZ; >> + break; >> + case 1: >> + case 2: >> + ret = DPLL_TYPE_EXT_1PPS; >> + break; >> + default: >> + ret = DPLL_TYPE_INT_OSCILLATOR; >> + } >> + >> + return ret; >> +} > > These case statements switch on private bits. This needs to match > on the selector name instead. > Addressed this in v2 > >> +static int ptp_ocp_dpll_get_output_type(struct dpll_device *dpll, int sma) >> +{ >> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >> + int ret; >> + >> + if (bp->sma[sma].mode != SMA_MODE_OUT) >> + return -1; >> + >> + switch (ptp_ocp_sma_get(bp, sma)) { >> + case 0: >> + ret = DPLL_TYPE_EXT_10MHZ; >> + break; >> + case 1: >> + case 2: >> + ret = DPLL_TYPE_INT_OSCILLATOR; >> + break; >> + case 4: >> + case 8: >> + ret = DPLL_TYPE_GNSS; >> + break; >> + default: >> + ret = DPLL_TYPE_INT_OSCILLATOR; > > Missing break; > > >> + } >> + >> + return ret; >> +} >> + >> +static struct dpll_device_ops dpll_ops = { >> + .get_status = ptp_ocp_dpll_get_status, >> + .get_lock_status = ptp_ocp_dpll_get_lock_status, >> + .get_source_type = ptp_ocp_dpll_get_source_type, >> + .get_output_type = ptp_ocp_dpll_get_output_type, >> +}; > > No 'set' statements here? Also, what happens if there is more than > one GNSS receiver, how is this differentiated? >> static int >> ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> { >> @@ -3768,6 +3846,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> >> ptp_ocp_info(bp); >> devlink_register(devlink); >> + >> + bp->dpll = dpll_device_alloc(&dpll_ops, ARRAY_SIZE(bp->sma), ARRAY_SIZE(bp->sma), bp); >> + if (!bp->dpll) { >> + dev_err(&pdev->dev, "dpll_device_alloc failed\n"); >> + return 0; >> + } >> + dpll_device_register(bp->dpll); >> + > > How is the release/unregister path called when the module is unloaded? I re-implemented unregister/free in v2. Hope it fixes questions.
On 24.06.2022 00:36, Jonathan Lemon wrote: > On Fri, Jun 24, 2022 at 12:11:43AM +0100, Vadim Fedorenko wrote: >> On 23.06.2022 19:28, Jonathan Lemon wrote: >>> On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: >>>> From: Vadim Fedorenko <vadfed@fb.com> >>>> +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) >>>> +{ >>>> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >>>> + int ret; >>>> + >>>> + if (bp->sma[sma].mode != SMA_MODE_IN) >>>> + return -1; >>>> + >>>> + switch (ptp_ocp_sma_get(bp, sma)) { >>>> + case 0: >>>> + ret = DPLL_TYPE_EXT_10MHZ; >>>> + break; >>>> + case 1: >>>> + case 2: >>>> + ret = DPLL_TYPE_EXT_1PPS; >>>> + break; >>>> + default: >>>> + ret = DPLL_TYPE_INT_OSCILLATOR; >>>> + } >>>> + >>>> + return ret; >>>> +} >>> >>> These case statements switch on private bits. This needs to match >>> on the selector name instead. >>> >> >> Not sure that string comparison is a good idea. Maybe it's better to extend >> struct ocp_selector with netlink type id and fill it according to hardware? > > Sure, that could be an option. But, as this is DPLL only, how does it > handle things when a pin is used for non-clock IO? In the timecard, > for example, we have the frequency counters for input, and the frequency > generators/VCC/GND for output. > > Actually our HW has a multi-level input, where the DPLL selects its > source from an internal mux - this isn't reflected here. The external > pins feed into some complex HW logic, which performs its own priority > calculations before presenting the end result as an available selection > for the DPLL. I don't really know how to deal with such configuration. For now I simply added CUSTOM type but I'm not sure how to deal it 'set' functions. Do you have any suggestions?
On Sun, Jun 26, 2022 at 08:27:17PM +0100, Vadim Fedorenko wrote: > On 23.06.2022 19:28, Jonathan Lemon wrote: > > On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: > > > From: Vadim Fedorenko <vadfed@fb.com> > > > > > > +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) > > > +{ > > > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > > > + int sync; > > > + > > > + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; > > > + return sync; > > > +} > > > > Please match existing code style. > > > > Didn't get this point. The same code is used through out the driver. > Could you please explain? Match existing function definition style.
On Sun, Jun 26, 2022 at 08:28:34PM +0100, Vadim Fedorenko wrote: > On 24.06.2022 00:36, Jonathan Lemon wrote: > > On Fri, Jun 24, 2022 at 12:11:43AM +0100, Vadim Fedorenko wrote: > > > On 23.06.2022 19:28, Jonathan Lemon wrote: > > > > On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: > > > > > From: Vadim Fedorenko <vadfed@fb.com> > > > > > +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) > > > > > +{ > > > > > + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); > > > > > + int ret; > > > > > + > > > > > + if (bp->sma[sma].mode != SMA_MODE_IN) > > > > > + return -1; > > > > > + > > > > > + switch (ptp_ocp_sma_get(bp, sma)) { > > > > > + case 0: > > > > > + ret = DPLL_TYPE_EXT_10MHZ; > > > > > + break; > > > > > + case 1: > > > > > + case 2: > > > > > + ret = DPLL_TYPE_EXT_1PPS; > > > > > + break; > > > > > + default: > > > > > + ret = DPLL_TYPE_INT_OSCILLATOR; > > > > > + } > > > > > + > > > > > + return ret; > > > > > +} > > > > > > > > These case statements switch on private bits. This needs to match > > > > on the selector name instead. > > > > > > > > > > Not sure that string comparison is a good idea. Maybe it's better to extend > > > struct ocp_selector with netlink type id and fill it according to hardware? > > > > Sure, that could be an option. But, as this is DPLL only, how does it > > handle things when a pin is used for non-clock IO? In the timecard, > > for example, we have the frequency counters for input, and the frequency > > generators/VCC/GND for output. > > > > Actually our HW has a multi-level input, where the DPLL selects its > > source from an internal mux - this isn't reflected here. The external > > pins feed into some complex HW logic, which performs its own priority > > calculations before presenting the end result as an available selection > > for the DPLL. > > I don't really know how to deal with such configuration. For now I simply added > CUSTOM type but I'm not sure how to deal it 'set' functions. Do you have any > suggestions? No suggestions other than the API should be able to handle things? Also, should there be a notifier if the source changes?
On 27.06.2022 20:23, Jonathan Lemon wrote: > On Sun, Jun 26, 2022 at 08:27:17PM +0100, Vadim Fedorenko wrote: >> On 23.06.2022 19:28, Jonathan Lemon wrote: >>> On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: >>>> From: Vadim Fedorenko <vadfed@fb.com> >>>> >>>> +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) >>>> +{ >>>> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >>>> + int sync; >>>> + >>>> + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; >>>> + return sync; >>>> +} >>> >>> Please match existing code style. >>> >> >> Didn't get this point. The same code is used through out the driver. >> Could you please explain? > > Match existing function definition style. Got it. Will address in the next version, thanks!
On 27.06.2022 20:27, Jonathan Lemon wrote: > On Sun, Jun 26, 2022 at 08:28:34PM +0100, Vadim Fedorenko wrote: >> On 24.06.2022 00:36, Jonathan Lemon wrote: >>> On Fri, Jun 24, 2022 at 12:11:43AM +0100, Vadim Fedorenko wrote: >>>> On 23.06.2022 19:28, Jonathan Lemon wrote: >>>>> On Thu, Jun 23, 2022 at 03:57:17AM +0300, Vadim Fedorenko wrote: >>>>>> From: Vadim Fedorenko <vadfed@fb.com> >>>>>> +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) >>>>>> +{ >>>>>> + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >>>>>> + int ret; >>>>>> + >>>>>> + if (bp->sma[sma].mode != SMA_MODE_IN) >>>>>> + return -1; >>>>>> + >>>>>> + switch (ptp_ocp_sma_get(bp, sma)) { >>>>>> + case 0: >>>>>> + ret = DPLL_TYPE_EXT_10MHZ; >>>>>> + break; >>>>>> + case 1: >>>>>> + case 2: >>>>>> + ret = DPLL_TYPE_EXT_1PPS; >>>>>> + break; >>>>>> + default: >>>>>> + ret = DPLL_TYPE_INT_OSCILLATOR; >>>>>> + } >>>>>> + >>>>>> + return ret; >>>>>> +} >>>>> >>>>> These case statements switch on private bits. This needs to match >>>>> on the selector name instead. >>>>> >>>> >>>> Not sure that string comparison is a good idea. Maybe it's better to extend >>>> struct ocp_selector with netlink type id and fill it according to hardware? >>> >>> Sure, that could be an option. But, as this is DPLL only, how does it >>> handle things when a pin is used for non-clock IO? In the timecard, >>> for example, we have the frequency counters for input, and the frequency >>> generators/VCC/GND for output. >>> >>> Actually our HW has a multi-level input, where the DPLL selects its >>> source from an internal mux - this isn't reflected here. The external >>> pins feed into some complex HW logic, which performs its own priority >>> calculations before presenting the end result as an available selection >>> for the DPLL. >> >> I don't really know how to deal with such configuration. For now I simply added >> CUSTOM type but I'm not sure how to deal it 'set' functions. Do you have any >> suggestions? > > No suggestions other than the API should be able to handle things? Ok, I will try to combine this with Arkadiusz's suggestion with priorities. > > Also, should there be a notifier if the source changes? Definitely. I'm working on *set operations and will add notifiers at the same time.
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 458218f88c5e..f74846ebc177 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig @@ -176,6 +176,7 @@ config PTP_1588_CLOCK_OCP depends on !S390 depends on COMMON_CLK select NET_DEVLINK + select DPLL help This driver adds support for an OpenCompute time card. diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c index e59ea2173aac..693168fdda6c 100644 --- a/drivers/ptp/ptp_ocp.c +++ b/drivers/ptp/ptp_ocp.c @@ -21,6 +21,7 @@ #include <linux/mtd/mtd.h> #include <linux/nvmem-consumer.h> #include <linux/crc16.h> +#include <uapi/linux/dpll.h> #define PCI_VENDOR_ID_FACEBOOK 0x1d9b #define PCI_DEVICE_ID_FACEBOOK_TIMECARD 0x0400 @@ -336,6 +337,7 @@ struct ptp_ocp { struct ptp_ocp_signal signal[4]; struct ptp_ocp_sma_connector sma[4]; const struct ocp_sma_op *sma_op; + struct dpll_device *dpll; }; #define OCP_REQ_TIMESTAMP BIT(0) @@ -3713,6 +3715,82 @@ ptp_ocp_detach(struct ptp_ocp *bp) device_unregister(&bp->dev); } +static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) +{ + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); + int sync; + + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; + return sync; +} + +static int ptp_ocp_dpll_get_lock_status(struct dpll_device *dpll) +{ + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); + int sync; + + sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; + return sync; +} + +static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) +{ + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); + int ret; + + if (bp->sma[sma].mode != SMA_MODE_IN) + return -1; + + switch (ptp_ocp_sma_get(bp, sma)) { + case 0: + ret = DPLL_TYPE_EXT_10MHZ; + break; + case 1: + case 2: + ret = DPLL_TYPE_EXT_1PPS; + break; + default: + ret = DPLL_TYPE_INT_OSCILLATOR; + } + + return ret; +} + + +static int ptp_ocp_dpll_get_output_type(struct dpll_device *dpll, int sma) +{ + struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); + int ret; + + if (bp->sma[sma].mode != SMA_MODE_OUT) + return -1; + + switch (ptp_ocp_sma_get(bp, sma)) { + case 0: + ret = DPLL_TYPE_EXT_10MHZ; + break; + case 1: + case 2: + ret = DPLL_TYPE_INT_OSCILLATOR; + break; + case 4: + case 8: + ret = DPLL_TYPE_GNSS; + break; + default: + ret = DPLL_TYPE_INT_OSCILLATOR; + } + + return ret; +} + +static struct dpll_device_ops dpll_ops = { + .get_status = ptp_ocp_dpll_get_status, + .get_lock_status = ptp_ocp_dpll_get_lock_status, + .get_source_type = ptp_ocp_dpll_get_source_type, + .get_output_type = ptp_ocp_dpll_get_output_type, +}; + static int ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) { @@ -3768,6 +3846,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) ptp_ocp_info(bp); devlink_register(devlink); + + bp->dpll = dpll_device_alloc(&dpll_ops, ARRAY_SIZE(bp->sma), ARRAY_SIZE(bp->sma), bp); + if (!bp->dpll) { + dev_err(&pdev->dev, "dpll_device_alloc failed\n"); + return 0; + } + dpll_device_register(bp->dpll); + return 0; out: