From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 97599CA0FE9 for ; Tue, 26 Aug 2025 08:34:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:CC:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=qA4QtszB3lhYVxil4FY85NorKM9gDEhmhJP9fmqDZM0=; b=c05uxW+2UFH1jYEWFPYberZ4hC gcZ/O1J3dmT4y14OCAJ4XZdkeMlRZ86ANDPEuhDjmla4VQ6LfbKorR5Vu9WBYk4bLrFKbqrOcaeSC 4OGnGq2P5Fp/B7l4whopGOboKVbfkrtsxBDyzz9MxhN3T/ar0gQiAMtgEiEqnMmIbYgsCGrQWKx25 PVrrOsWpSZgLVhvxO9uUW+XqLt7fLs/3EN6nTs7F/u7yEBoZbr9YLMBZ+QNmWvXuIxi7Z4tp8DXEi xH5u5WqXEypS5BtG1DKnG60uKfs+thTCapBWzOMIBAJEt0z7m8btENZ5bk+RjasRAnXqWKoYNecCp 7o32bNTw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uqp8N-0000000B25b-0RSi; Tue, 26 Aug 2025 08:34:27 +0000 Received: from fllvem-ot04.ext.ti.com ([198.47.19.246]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uqp4O-0000000B1O8-1aPP for linux-arm-kernel@lists.infradead.org; Tue, 26 Aug 2025 08:30:21 +0000 Received: from fllvem-sh04.itg.ti.com ([10.64.41.54]) by fllvem-ot04.ext.ti.com (8.15.2/8.15.2) with ESMTP id 57Q8TpuD1474791; Tue, 26 Aug 2025 03:29:51 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1756196991; bh=qA4QtszB3lhYVxil4FY85NorKM9gDEhmhJP9fmqDZM0=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=RNXZ4RDQOHKqg91jIce38BPyes6xfCyL41zxjE5Fxuronah7zNCrbTzB9WI8J4FBq 6qEH2XOFxkaoa1kIlykoBp/G4OpGIjfZYRjsa0Fo59pkc2Z2fwOIV793HHwW2DJsZl qH/s0rpWYp1eSTxm6oSnOj7zn+JLxeDZo9WQDTLA= Received: from DFLE109.ent.ti.com (dfle109.ent.ti.com [10.64.6.30]) by fllvem-sh04.itg.ti.com (8.18.1/8.18.1) with ESMTPS id 57Q8ToHr2261537 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA256 bits=128 verify=FAIL); Tue, 26 Aug 2025 03:29:50 -0500 Received: from DFLE115.ent.ti.com (10.64.6.36) by DFLE109.ent.ti.com (10.64.6.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.55; Tue, 26 Aug 2025 03:29:50 -0500 Received: from lelvem-mr05.itg.ti.com (10.180.75.9) by DFLE115.ent.ti.com (10.64.6.36) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.55 via Frontend Transport; Tue, 26 Aug 2025 03:29:49 -0500 Received: from [10.24.68.198] (abhilash-hp.dhcp.ti.com [10.24.68.198]) by lelvem-mr05.itg.ti.com (8.18.1/8.18.1) with ESMTP id 57Q8Thoe1135709; Tue, 26 Aug 2025 03:29:43 -0500 Message-ID: Date: Tue, 26 Aug 2025 13:59:42 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V2 4/4] media: ti-vpe: Add the VIP driver To: Krzysztof Kozlowski , , , , CC: , , , , , , , , , , , , , , , , , , , , References: <20250716111912.235157-1-y-abhilashchandra@ti.com> <20250716111912.235157-5-y-abhilashchandra@ti.com> Content-Language: en-US From: Yemike Abhilash Chandra In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-C2ProcessedOrg: 333ef613-75bf-4e12-a4b1-8e3623f5dcea X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250826_013020_524437_21347955 X-CRM114-Status: GOOD ( 20.32 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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