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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8149d7d-65ca-47c1-9338-45a0db614e77@kernel.org>
Date: Wed, 16 Jul 2025 16:09:12 +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
Cc: linux@...linux.org.uk, ardb@...nel.org, ebiggers@...nel.org,
 geert+renesas@...der.be, claudiu.beznea@...on.dev, bparrot@...com,
 andre.draszik@...aro.org, kuninori.morimoto.gx@...esas.com,
 prabhakar.mahadev-lad.rj@...renesas.com, heikki.krogerus@...ux.intel.com,
 kory.maincent@...tlin.com, florian.fainelli@...adcom.com, lumag@...nel.org,
 dale@...nsworth.org, sbellary@...libre.com, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, dagriego@...lakesoftware.com,
 u-kumar1@...com
Subject: Re: [PATCH V2 4/4] media: ti-vpe: Add the VIP driver

On 16/07/2025 13:19, Yemike Abhilash Chandra wrote:

> +static int vip_probe_complete(struct platform_device *pdev)
> +{
> +	struct vip_shared *shared = platform_get_drvdata(pdev);
> +	struct regmap *syscon_pol = NULL;
> +	u32 syscon_pol_offset = 0;
> +	struct vip_port *port;
> +	struct vip_dev *dev;
> +	struct device_node *parent = pdev->dev.of_node;
> +	struct fwnode_handle *ep = NULL;
> +	int ret, slice_id, port_id, p;
> +
> +	if (parent && of_property_read_bool(parent, "ti,vip-clk-polarity")) {
> +		syscon_pol = syscon_regmap_lookup_by_phandle(parent,
> +							     "ti,vip-clk-polarity");
> +		if (IS_ERR(syscon_pol)) {
> +			dev_err(&pdev->dev, "failed to get ti,vip-clk-polarity regmap\n");
> +			return PTR_ERR(syscon_pol);

Syntax is return dev_err_probe. If this is not probe path, then this has
to be fixed.

> +		}
> +
> +		if (of_property_read_u32_index(parent, "ti,vip-clk-polarity",
> +					       1, &syscon_pol_offset)) {
> +			dev_err(&pdev->dev, "failed to get ti,vip-clk-polarity offset\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	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_pol = syscon_pol;
> +		dev->syscon_pol_offset = syscon_pol_offset;
> +		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, int instance_id)
> +{
> +	struct vip_shared *shared = platform_get_drvdata(pdev);
> +	struct vip_dev *dev;
> +	struct vip_parser_data *parser;
> +	u32 vin_id;
> +	int ret;
> +
> +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	dev->instance_id = instance_id;
> +	vin_id = 1 + ((dev->instance_id - 1) * 2) + slice;
> +	snprintf(dev->name, sizeof(dev->name), "vin%d", vin_id);
> +
> +	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, dev->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->res = platform_get_resource_byname(pdev,
> +						   IORESOURCE_MEM,
> +						   (slice == 0) ?
> +						   "parser0" :
> +						   "parser1");
> +	parser->base = devm_ioremap_resource(&pdev->dev, parser->res);
> +	if (IS_ERR(parser->base))
> +		return PTR_ERR(parser->base);
> +
> +	parser->pdev = pdev;
> +	dev->parser = parser;
> +
> +	dev->sc_assigned = VIP_NOT_ASSIGNED;
> +	dev->sc = sc_create(pdev, (slice == 0) ? "sc0" : "sc1");
> +	if (IS_ERR(dev->sc))
> +		return PTR_ERR(dev->sc);
> +
> +	dev->csc_assigned = VIP_NOT_ASSIGNED;
> +	dev->csc = csc_create(pdev, (slice == 0) ? "csc0" : "csc1");
> +	if (IS_ERR(dev->sc))
> +		return PTR_ERR(dev->sc);
> +
> +	return 0;
> +}
> +
> +static int vip_probe(struct platform_device *pdev)
> +{
> +	struct vip_shared *shared;
> +	struct pinctrl *pinctrl;
> +	int ret, slice = VIP_SLICE1;
> +	int instance_id;
> +	u32 tmp, pid;
> +	const char *label;
> +
> +	if (!of_property_read_string(pdev->dev.of_node, "label", &label)) {
> +		if (strcmp(label, "vip1") == 0)
> +			instance_id = 1;
> +		else if (strcmp(label, "vip2") == 0)
> +			instance_id = 2;
> +		else if (strcmp(label, "vip3") == 0)


Heh, nice try. You cannot encode instance ID as different property (and
instance ID is not allowed, see writing bindings in next).

And how does it work with label called "krzk"? Your binding said that
"krzk" is a perfectly correct label.

You need to think about such cases.


> +			instance_id = 3;

And past here you use uninitialized instance_id, because you did not
consider "krzk".



Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ