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 72EF7C04FFE for ; Mon, 20 May 2024 13:06:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PE4Wv3VoTYrE48TT9YcM0vGOhmIdqSKGjQNw0hSfApE=; b=KjYQytq3rcDt43 WrY0IYMGNs+0p2JmcvD7jPR3YMNNXjunsqpmewBeKh+x3v+ObAVrnmuR2/1Sut+JMa+CWKN+A1scL C3R707T2g+DlADnFkKqvEpchKu/tDb4ngH8bMnW3zFMDT9E0U6DqogBqBl8qNVbRyZKUXu3Twjv7Q KverWHbOPzafQb22gYQkagC9SlswavEimofK9X7pCpx0d/TVQcQLy+/8woKMT73TIjKhstL3Q/LMf hBLZtYQeXjNkSpX7YB37Q24T7WkZr0Z3th/53DSvKIKc8gonre04hFeXP6yErG97WnDpXaNCcA2HJ k69LWYsjNyg9BYsXTNcA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s92im-0000000EXFC-3pZZ; Mon, 20 May 2024 13:06:32 +0000 Received: from mgamail.intel.com ([192.198.163.15]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s92ij-0000000EXEh-2Djm for linux-arm-kernel@lists.infradead.org; Mon, 20 May 2024 13:06:31 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1716210389; x=1747746389; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=QNxMxsp5f2IJVZm6nz8esw+/sZvJ7kDRqA+fh+kKJJQ=; b=axzHVrbZ06HWFd2NmAHGuOzWvuY51kqBuITZ0Voy18v+/h5vlgO8R513 epewbLtolBghKiHQw+jTeK4XTRjYi+J+s87hysTmONJ5sguStQKus4oIM UMfZpzlxSofFW6ntGV4f4+nWzo/bKm8EPXN2jBDp8QxUW2fNqIdWvgTv5 l5J0w+t/Dz0mGrXG5LC92NxW6mOgJL5PlWRHVmA+5210xGjwLfWc+pIzF 1+09gMoU8pp7MmU7SqvX1lhR9+n2BMFInQa8fPLRrqCHaW4SlnjKXFLoI e+g5FJcCflIAAoJBBwYlWTLIVA43qZDBWymp1yl3oV++xyIfKP+QsxLA8 g==; X-CSE-ConnectionGUID: a+YaadhdQRuEGy5ckm3LHw== X-CSE-MsgGUID: 3ddu0erqT0SHi/qlguArWw== X-IronPort-AV: E=McAfee;i="6600,9927,11078"; a="12526297" X-IronPort-AV: E=Sophos;i="6.08,175,1712646000"; d="scan'208";a="12526297" Received: from fmviesa009.fm.intel.com ([10.60.135.149]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2024 06:06:27 -0700 X-CSE-ConnectionGUID: F6+f0sAaTM632rEMs4GzRw== X-CSE-MsgGUID: aBJJoIvDS6qStyLHllYAIA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,175,1712646000"; d="scan'208";a="32554370" Received: from unknown (HELO localhost) ([10.245.246.99]) by fmviesa009-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2024 06:06:22 -0700 From: Jani Nikula To: Dmitry Baryshkov Cc: dri-devel@lists.freedesktop.org, Philipp Zabel , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 10/11] drm/imx/tve: convert to struct drm_edid In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <2f59a7ad10c29c21f08223ef19221cef48623cc8.1715691257.git.jani.nikula@intel.com> Date: Mon, 20 May 2024 16:06:19 +0300 Message-ID: <87v838skdw.fsf@intel.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240520_060629_878263_3E6D09FC X-CRM114-Status: GOOD ( 19.82 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 20 May 2024, Dmitry Baryshkov wrote: > On Tue, May 14, 2024 at 03:55:16PM +0300, Jani Nikula wrote: >> Prefer the struct drm_edid based functions for reading the EDID and >> updating the connector. >> >> Signed-off-by: Jani Nikula >> >> --- >> >> Cc: Philipp Zabel >> Cc: Maarten Lankhorst >> Cc: Maxime Ripard >> Cc: Thomas Zimmermann >> Cc: Shawn Guo >> Cc: Sascha Hauer >> Cc: Pengutronix Kernel Team >> Cc: Fabio Estevam >> Cc: imx@lists.linux.dev >> Cc: linux-arm-kernel@lists.infradead.org >> --- >> drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 ++++++-------- >> 1 file changed, 6 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-tve.c b/drivers/gpu/drm/imx/ipuv3/imx-tve.c >> index b49bddb85535..29f494bfff67 100644 >> --- a/drivers/gpu/drm/imx/ipuv3/imx-tve.c >> +++ b/drivers/gpu/drm/imx/ipuv3/imx-tve.c >> @@ -201,18 +201,16 @@ static int tve_setup_vga(struct imx_tve *tve) >> static int imx_tve_connector_get_modes(struct drm_connector *connector) >> { >> struct imx_tve *tve = con_to_tve(connector); >> - struct edid *edid; >> - int ret = 0; >> + const struct drm_edid *drm_edid; >> + int ret; >> >> if (!tve->ddc) >> return 0; >> >> - edid = drm_get_edid(connector, tve->ddc); >> - if (edid) { >> - drm_connector_update_edid_property(connector, edid); >> - ret = drm_add_edid_modes(connector, edid); >> - kfree(edid); >> - } >> + drm_edid = drm_edid_read_ddc(connector, tve->ddc); >> + drm_edid_connector_update(connector, drm_edid); >> + ret = drm_edid_connector_add_modes(connector); >> + drm_edid_free(drm_edid); > > Reviewed-by: Dmitry Baryshkov > > Nit: if you change two last lines, you can drop ret= assignment and use > return drm_edid_connector_add_modes(connector). > > Maybe we shoud add cocci rule for such cases. I think there was a cocci rule like that, but a lot of people (including yours truly) preferred to keep the assignment, and the patches ended up in bikeshedding, so the cocci was removed. My argument is that it's not uncommon to keep adding and removing stuff while the code evolves, and having to change the return statement is boring and makes the diff harder to follow. It's a bit like that extra comma at the end of initialization lists or enumeration definitions. Others think the code should reflect current state and not prepare for scenarios that might never arrive. Both are correct, so it's perfect for never ending bikeshedding. ;) BR, Jani. > >> >> return ret; >> } >> -- >> 2.39.2 >> -- Jani Nikula, Intel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel