lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <2b7e0024-2a92-58c8-fbeb-d42beebedb03@linux.dev> Date: Mon, 7 Aug 2023 22:42:41 +0100 From: Vadim Fedorenko <vadim.fedorenko@...ux.dev> To: Simon Horman <horms@...nel.org> Cc: Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>, Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>, Jonathan Lemon <jonathan.lemon@...il.com>, Paolo Abeni <pabeni@...hat.com>, Milena Olech <milena.olech@...el.com>, Michal Michalik <michal.michalik@...el.com>, linux-arm-kernel@...ts.infradead.org, poros@...hat.com, mschmidt@...hat.com, netdev@...r.kernel.org, linux-clk@...r.kernel.org, Bart Van Assche <bvanassche@....org>, intel-wired-lan@...ts.osuosl.org, Jiri Pirko <jiri@...dia.com> Subject: Re: [PATCH net-next v2 8/9] ptp_ocp: implement DPLL ops On 06.08.2023 18:13, Simon Horman wrote: > 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@...ux.dev> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com> >> Signed-off-by: Jiri Pirko <jiri@...dia.com> > > Hi Vadim, > > ... > >> 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. > Ahh, looks like it's not needed at all, thanks! >> + return -EINVAL; >> +} > > ... > >> @@ -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? Yeah, you are right. And I have spotted one more place where err is not properly set, just above this one. Thanks for the review. > 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: > > ...
Powered by blists - more mailing lists