| 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: <87e5f31a-5c3d-4cc0-8146-1b6b5923136b@kernel.org>
Date: Fri, 17 Oct 2025 07:19:14 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Yemike Abhilash Chandra <y-abhilashchandra@...com>, mchehab@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
hverkuil+cisco@...nel.org
Cc: sakari.ailus@...ux.intel.com, bparrot@...com,
jai.luthra@...asonboard.com, dale@...nsworth.org,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, u-kumar1@...com,
Sukrut Bellary <sbellary@...libre.com>
Subject: Re: [PATCH V4 4/4] media: ti: vpe: Add the VIP driver
On 15/10/2025 07:40, Yemike Abhilash Chandra wrote:
> +static int vip_probe_complete(struct platform_device *pdev)
> +{
> + struct vip_shared *shared = platform_get_drvdata(pdev);
> + struct vip_ctrl_module *ctrl = NULL;
> + struct vip_port *port;
> + struct vip_dev *dev;
> + struct device_node *parent = pdev->dev.of_node;
> + struct fwnode_handle *ep = NULL;
> + struct of_phandle_args args;
> + int ret, i, slice_id, port_id, p;
> +
> + /* Allocate ctrl before using it */
> + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
> + if (!ctrl)
> + return -ENOMEM;
> +
> + if (parent && of_property_present(parent, "ti,ctrl-module")) {
> + ctrl->syscon_pol = syscon_regmap_lookup_by_phandle(parent,
> + "ti,ctrl-module");
> + if (IS_ERR(ctrl->syscon_pol))
> + return dev_err_probe(&pdev->dev, PTR_ERR(ctrl->syscon_pol),
> + "Failed to get ti,ctrl-module regmap\n");
> + }
> +
> + ret = of_parse_phandle_with_fixed_args(parent, "ti,ctrl-module",
> + 5, 0, &args);
1. You leak parent.
2. Why you can't just just take it from syscon call? This is not only
leaking, but actually duplicated and unnecessary.
> +
> + if (ret) {
> + dev_err(&pdev->dev, "failed to parse ti,ctrl-module\n");
> + return ret;
> + }
> +
> + ctrl->syscon_offset = args.args[0];
> +
> + for (i = 0; i < ARRAY_SIZE(ctrl->syscon_bit_field); i++)
> + ctrl->syscon_bit_field[i] = args.args[i + 1];
> +
> + for (p = 0; p < (VIP_NUM_PORTS * VIP_NUM_SLICES); p++) {
> + ep = fwnode_graph_get_next_endpoint_by_regs(of_fwnode_handle(parent),
> + p, 0);
> + if (!ep)
> + continue;
> +
> + switch (p) {
> + case 0:
> + slice_id = VIP_SLICE1; port_id = VIP_PORTA;
> + break;
> + case 1:
> + slice_id = VIP_SLICE2; port_id = VIP_PORTA;
> + break;
> + case 2:
> + slice_id = VIP_SLICE1; port_id = VIP_PORTB;
> + break;
> + case 3:
> + slice_id = VIP_SLICE2; port_id = VIP_PORTB;
> + break;
> + default:
> + dev_err(&pdev->dev, "Unknown port reg=<%d>\n", p);
> + continue;
> + }
> +
> + ret = alloc_port(shared->devs[slice_id], port_id);
> + if (ret < 0)
> + continue;
> +
> + dev = shared->devs[slice_id];
> + dev->syscon = ctrl;
> + port = dev->ports[port_id];
> +
> + vip_register_subdev_notif(port, ep);
> + fwnode_handle_put(ep);
> + }
> + return 0;
> +}
> +
> +static int vip_probe_slice(struct platform_device *pdev, int slice)
> +{
> + struct vip_shared *shared = platform_get_drvdata(pdev);
> + struct vip_dev *dev;
> + struct vip_parser_data *parser;
> + struct sc_data *sc;
> + struct csc_data *csc;
> + int ret;
> +
> + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + dev->irq = platform_get_irq(pdev, slice);
> + if (dev->irq < 0)
> + return dev->irq;
> +
> + ret = devm_request_irq(&pdev->dev, dev->irq, vip_irq,
> + 0, VIP_MODULE_NAME, dev);
> + if (ret < 0)
> + return -ENOMEM;
> +
> + spin_lock_init(&dev->slock);
> + mutex_init(&dev->mutex);
> +
> + dev->slice_id = slice;
> + dev->pdev = pdev;
> + dev->res = shared->res;
> + dev->base = shared->base;
> + dev->v4l2_dev = shared->v4l2_dev;
> +
> + dev->shared = shared;
> + shared->devs[slice] = dev;
> +
> + vip_top_reset(dev);
> + vip_set_slice_path(dev, VIP_MULTI_CHANNEL_DATA_SELECT, 1);
> +
> + parser = devm_kzalloc(&pdev->dev, sizeof(*dev->parser), GFP_KERNEL);
> + if (!parser)
> + return PTR_ERR(parser);
> +
> + parser->base = dev->base + (slice ? VIP_SLICE1_PARSER : VIP_SLICE0_PARSER);
> + if (IS_ERR(parser->base))
> + return PTR_ERR(parser->base);
> +
> + parser->pdev = pdev;
> + dev->parser = parser;
> +
> + dev->sc_assigned = VIP_NOT_ASSIGNED;
> + sc = devm_kzalloc(&pdev->dev, sizeof(*dev->sc), GFP_KERNEL);
> + if (!sc)
> + return PTR_ERR(sc);
> +
> + sc->base = dev->base + (slice ? VIP_SLICE1_SC : VIP_SLICE0_SC);
> + if (IS_ERR(sc->base))
> + return PTR_ERR(sc->base);
> +
> + sc->pdev = pdev;
> + dev->sc = sc;
> +
> + dev->csc_assigned = VIP_NOT_ASSIGNED;
> + csc = devm_kzalloc(&pdev->dev, sizeof(*dev->csc), GFP_KERNEL);
> + if (!csc)
> + return PTR_ERR(csc);
> +
> + csc->base = dev->base + (slice ? VIP_SLICE1_CSC : VIP_SLICE0_CSC);
> + if (IS_ERR(csc->base))
> + return PTR_ERR(csc->base);
> +
> + csc->pdev = pdev;
> + dev->csc = csc;
> +
> + return 0;
> +}
> +
> +static int vip_probe(struct platform_device *pdev)
> +{
> + struct vip_shared *shared;
> + struct pinctrl *pinctrl;
> + int ret, slice = VIP_SLICE1;
> + u32 tmp, pid;
> +
> + /* If there are no endpoint defined there is nothing to do */
> + if (!vip_endpoint_scan(pdev)) {
> + dev_err(&pdev->dev, "%s: No sensor", __func__);
> + return -ENODEV;
> + }
> +
> + ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_err(&pdev->dev,
> + "32-bit consistent DMA enable failed\n");
> + return ret;
> + }
> +
> + shared = devm_kzalloc(&pdev->dev, sizeof(*shared), GFP_KERNEL);
> + if (!shared)
> + return -ENOMEM;
> +
> + shared->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + shared->base = devm_ioremap_resource(&pdev->dev, shared->res);
Use proper wrapper for these two.
> + if (IS_ERR(shared->base))
> + return PTR_ERR(shared->base);
> +
> + vip_init_format_info(&pdev->dev);
> +
> + pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = pm_runtime_get_sync(&pdev->dev);
> + if (ret)
> + goto err_runtime_disable;
> +
> + /* Make sure H/W module has the right functionality */
> + pid = reg_read(shared, VIP_PID);
> + tmp = get_field(pid, VIP_PID_FUNC_MASK, VIP_PID_FUNC_SHIFT);
> +
> + if (tmp != VIP_PID_FUNC) {
> + dev_info(&pdev->dev, "vip: unexpected PID function: 0x%x\n",
> + tmp);
> + ret = -ENODEV;
> + goto err_runtime_put;
> + }
> +
> + ret = v4l2_device_register(&pdev->dev, &shared->v4l2_dev);
> + if (ret)
> + goto err_runtime_put;
> +
> + /* enable clocks, so the firmware will load properly */
> + vip_shared_set_clock_enable(shared, 1);
> + vip_top_vpdma_reset(shared);
> +
> + platform_set_drvdata(pdev, shared);
> +
> + v4l2_ctrl_handler_init(&shared->ctrl_handler, 11);
> + shared->v4l2_dev.ctrl_handler = &shared->ctrl_handler;
> +
> + for (slice = VIP_SLICE1; slice < VIP_NUM_SLICES; slice++) {
> + ret = vip_probe_slice(pdev, slice);
> + if (ret) {
> + dev_err(&pdev->dev, "Creating slice failed");
> + goto err_dev_unreg;
> + }
> + }
Best regards,
Krzysztof
Powered by blists - more mailing lists