| 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: <dcaaf0ee-2cfc-419e-8c4a-8834372f1948@ti.com>
Date: Tue, 26 Aug 2025 13:59:42 +0530
From: Yemike Abhilash Chandra <y-abhilashchandra@...com>
To: Krzysztof Kozlowski <krzk@...nel.org>, <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
Hi Krzysztof,
Thanks for the review.
On 16/07/25 19:39, Krzysztof Kozlowski wrote:
> 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.
>
I will fix this in v3.
>> + }
>> +
>> + 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".
>
Understood. I will avoid this in v3
Thanks and Regards,
Yemike Abhilash Chandra
>
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists