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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 E0352E7D246 for ; Tue, 26 Sep 2023 07:44:29 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8635C10E35D; Tue, 26 Sep 2023 07:44:29 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9356D10E35D; Tue, 26 Sep 2023 07:44:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695714268; x=1727250268; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=FgG1KY5ZAcKGmpGkHLr3kBgobBPcTYLfeCJ7S7PzGhs=; b=AXFUHGX03K/pRipWtf7Iif3mEtLCO+cp0sJqDsdan/cjfb0J0Kj4zIpz J/WbCyamjCRfVA5J5wpAwOLZYSxCU9ISGj3v+gqiAgYxjcFvG638XQ4F/ jakZJ2S2MyCXQ4Xjd1zPWplw2tmM93BbHSBXbxSpZi6am+y0Oy/ITzDeH bPIOwKIiNCulWDLBL+2XszcBOa09J5q1F6tsH598Zr07rnXXveZd1V96w dZtT7W1RKDcOyBLd296v9E3JR49wuL9m6SY98t+amaizs3YVBFJfWoLxf aFkJyoxbiRh4UHbF37+M1FXCyVfahkaYbT9Rin1++cVMUcz3d33Cnv08o A==; X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="448006652" X-IronPort-AV: E=Sophos;i="6.03,177,1694761200"; d="scan'208";a="448006652" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2023 00:44:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="752066130" X-IronPort-AV: E=Sophos;i="6.03,177,1694761200"; d="scan'208";a="752066130" Received: from wagnert-mobl2.ger.corp.intel.com (HELO localhost) ([10.252.52.202]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2023 00:44:26 -0700 From: Jani Nikula To: "Golani, Mitulkumar Ajitkumar" , "dri-devel@lists.freedesktop.org" In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <6692fbce07fbc03ad8785e6e6fe81fad4354e657.1694078430.git.jani.nikula@intel.com> Date: Tue, 26 Sep 2023 10:44:23 +0300 Message-ID: <871qelml9k.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Intel-gfx] [PATCH 4/6] drm/edid: use a temp variable for sads to drop one level of dereferences X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "intel-gfx@lists.freedesktop.org" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, 25 Sep 2023, "Golani, Mitulkumar Ajitkumar" wrote: > Hi Jani, > > added comments in-line. > >> -----Original Message----- >> From: Nikula, Jani >> Sent: Thursday, September 7, 2023 2:58 PM >> To: dri-devel@lists.freedesktop.org >> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani ; >> Golani, Mitulkumar Ajitkumar >> Subject: [PATCH 4/6] drm/edid: use a temp variable for sads to drop one level >> of dereferences >> >> It's arguably easier on the eyes, and drops a set of parenthesis too. > > Please consider providing a bit more context in the commit message for better clarity. > >> >> Cc: Mitul Golani >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/drm_edid.c | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index >> 2025970816c9..fcdc2c314cde 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -5583,7 +5583,7 @@ static void drm_edid_to_eld(struct drm_connector >> *connector, } >> >> static int _drm_edid_to_sad(const struct drm_edid *drm_edid, >> - struct cea_sad **sads) >> + struct cea_sad **psads) >> { >> const struct cea_db *db; >> struct cea_db_iter iter; >> @@ -5592,19 +5592,21 @@ static int _drm_edid_to_sad(const struct >> drm_edid *drm_edid, >> cea_db_iter_edid_begin(drm_edid, &iter); >> cea_db_iter_for_each(db, &iter) { >> if (cea_db_tag(db) == CTA_DB_AUDIO) { >> + struct cea_sad *sads; >> int j; >> >> count = cea_db_payload_len(db) / 3; /* SAD is 3B */ >> - *sads = kcalloc(count, sizeof(**sads), GFP_KERNEL); >> - if (!*sads) >> + sads = kcalloc(count, sizeof(*sads), GFP_KERNEL); >> + *psads = sads; >> + if (!sads) >> return -ENOMEM; >> for (j = 0; j < count; j++) { >> const u8 *sad = &db->data[j * 3]; >> >> - (*sads)[j].format = (sad[0] & 0x78) >> 3; >> - (*sads)[j].channels = sad[0] & 0x7; >> - (*sads)[j].freq = sad[1] & 0x7F; >> - (*sads)[j].byte2 = sad[2]; >> + sads[j].format = (sad[0] & 0x78) >> 3; >> + sads[j].channels = sad[0] & 0x7; >> + sads[j].freq = sad[1] & 0x7F; >> + sads[j].byte2 = sad[2]; > > Thanks for the code update. I noticed the use of magic values in this section, which can make the code less clear > and harder to maintain. Would it be possible to define constants or use descriptive names instead of these magic > values? Yes, but that would be for a separate patch. The magic values aren't added here. BR, Jani. > > Regards, > Mitul >> } >> break; >> } >> -- >> 2.39.2 > -- Jani Nikula, Intel