Re: [PATCH net-next v2 8/9] ptp_ocp: implement DPLL ops
From: Simon Horman <horms@kernel.org>
Date: 2023-08-06 17:13:38
Also in:
intel-wired-lan, linux-arm-kernel, linux-clk
On Fri, Aug 04, 2023 at 08:04:53PM +0100, Vadim Fedorenko wrote:
Implement basic DPLL operations in ptp_ocp driver as the simplest example of using new subsystem. Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Signed-off-by: Jiri Pirko <redacted>
Hi Vadim, ...
quoted hunk ↗ jump to hunk
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 32dff1b4f891..e4da62ac9a9f 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig@@ -177,6 +177,7 @@ config PTP_1588_CLOCK_OCP depends on COMMON_CLK select NET_DEVLINK select CRC16 + 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
...
+static int ptp_ocp_dpll_frequency_set(const struct dpll_pin *pin,
+ void *pin_priv,
+ const struct dpll_device *dpll,
+ void *dpll_priv, u64 frequency,
+ struct netlink_ext_ack *extack)
+{
+ struct ptp_ocp_sma_connector *sma = pin_priv;
+ struct ptp_ocp *bp = dpll_priv;
+ const struct ocp_selector *tbl;
+ int sma_nr = (sma - bp->sma);
+ int val, i;
+
+ if (sma->fixed_fcn)
+ return -EOPNOTSUPP;
+
+ tbl = bp->sma_op->tbl[sma->mode];
+ for (i = 0; tbl[i].name; i++)
+ if (tbl[i].frequency == frequency)
+ return ptp_ocp_sma_store_val(bp, val, sma->mode, sma_nr);val appears to be used uninitialised here. As flagged by clang-16 W=1, and Smatch.
+ return -EINVAL; +}
...
quoted hunk ↗ jump to hunk
@@ -4233,8 +4437,40 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) ptp_ocp_info(bp); devlink_register(devlink); - return 0; + clkid = pci_get_dsn(pdev); + bp->dpll = dpll_device_get(clkid, 0, THIS_MODULE); + if (IS_ERR(bp->dpll)) { + dev_err(&pdev->dev, "dpll_device_alloc failed\n"); + goto out; + } + + err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp); + if (err) + goto out; + + for (i = 0; i < OCP_SMA_NUM; i++) { + bp->sma[i].dpll_pin = dpll_pin_get(clkid, i, THIS_MODULE, &bp->sma[i].dpll_prop); + if (IS_ERR(bp->sma[i].dpll_pin))
The function will return err. Should it be sett to an error value here? As flagged by Smatch.
+ goto out_dpll;
+
+ err = dpll_pin_register(bp->dpll, bp->sma[i].dpll_pin, &dpll_pins_ops,
+ &bp->sma[i]);
+ if (err) {
+ dpll_pin_put(bp->sma[i].dpll_pin);
+ goto out_dpll;
+ }
+ }
+ queue_delayed_work(system_power_efficient_wq, &bp->sync_work, HZ);
+
+ return 0;
+out_dpll:
+ while (i) {
+ --i;
+ dpll_pin_unregister(bp->dpll, bp->sma[i].dpll_pin, &dpll_pins_ops, &bp->sma[i]);
+ dpll_pin_put(bp->sma[i].dpll_pin);
+ }
+ dpll_device_put(bp->dpll);
out:
ptp_ocp_detach(bp);
out_disable:...