From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05AB6823DE for ; Wed, 7 Aug 2024 14:58:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723042733; cv=none; b=sBKgu8OF8UwHrQ6lNvNaG+52ZYO5q60TN8W/MEHdYmNvZyKcNo4I8INE+NIlSN5LV7Jz5idr47LYRH6YKTGR6MSanxhvldNNl0BsWggT3iSBzENlzncbqtpL7+ucV+dekrrr1L2EI+SCgpbwlkm54gZWq3aT8otOhzj/Z4opyLA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723042733; c=relaxed/simple; bh=2fItWewwxFGqhmsqb2jSiwgO6+kgU3T+Q2F51ZJQ8+I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=L2j9bU7emBls4AEswO9mVTM8OXUYVlznBt1ArD09vQLzQqXq4NuEveoaW6wK6Q1pr2HZHh7/EWx//HuP+ZK750eVDtIEEVl1+J5ZGom2J6KrW0pggr0T3nMXaIWc0TiLz6SimzP3iEt+qHnGuxrx7uzoKGopGz70NZbg4qDSv+w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=JDBgWqbA; arc=none smtp.client-ip=192.198.163.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="JDBgWqbA" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1723042731; x=1754578731; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=2fItWewwxFGqhmsqb2jSiwgO6+kgU3T+Q2F51ZJQ8+I=; b=JDBgWqbAp23Rxe73VNHCfI1z0G+hiOOG35Ekhvv37JNfXTNasP7//s6M BgLc1WQowWVwyqgFdX59c03moPXgocakVrKcELsu1K3ckS95Y1yvV549H uj9Qv1xTtVei3kxRs+Kh+oqwFNiqBtL02CDcFLg4DRcY9ivJzbtT3aLhb HY92Ev3rRnD6pYfJt98FeldPk2MY5fm/FJOWuIwvQS+qdQn9a3f1sNws+ p9/kV1fDKhy9Ruf9Ik1grglA9j92fuAVwoEwsjlSkwk4yZNweWTE+IgtN Df+WECM3wOAQn481nZeDW+ncAp9rlYYNGe1hzWRWQjF/Eo2Ws5/A1kdem g==; X-CSE-ConnectionGUID: Eb9wzdnOQnqxCmcGgqSnNA== X-CSE-MsgGUID: js2t5FqHQoyM6aeMie5d6Q== X-IronPort-AV: E=McAfee;i="6700,10204,11157"; a="32520886" X-IronPort-AV: E=Sophos;i="6.09,270,1716274800"; d="scan'208";a="32520886" Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Aug 2024 07:58:50 -0700 X-CSE-ConnectionGUID: VlmhSiYLTeeDTDHJqqa1uA== X-CSE-MsgGUID: 0gbQH6ABSa+3Udj6lGs5ug== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,270,1716274800"; d="scan'208";a="56970199" Received: from oandoniu-mobl3.ger.corp.intel.com (HELO intel.com) ([10.245.245.8]) by fmviesa010-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Aug 2024 07:58:45 -0700 Date: Wed, 7 Aug 2024 15:58:30 +0100 From: Andi Shyti To: "Cavitt, Jonathan" Cc: Thorsten Blum , "jani.nikula@linux.intel.com" , "joonas.lahtinen@linux.intel.com" , "Vivi, Rodrigo" , "tursulin@ursulin.net" , "airlied@gmail.com" , "daniel@ffwll.ch" , "intel-gfx@lists.freedesktop.org" , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2] drm/i915: Explicitly cast divisor and use div_u64() Message-ID: References: <20240802160323.46518-2-thorsten.blum@toblux.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi Thorsten, > > /* This check is primarily to ensure that oa_period <= > > - * UINT32_MAX (before passing to do_div which only > > + * UINT32_MAX (before passing it to div_u64 which only > > * accepts a u32 denominator), but we can also skip > > * checking anything < 1Hz which implicitly can't be > > * limited via an integer oa_max_sample_rate. > > */ > > if (oa_period <= NSEC_PER_SEC) { > > - u64 tmp = NSEC_PER_SEC; > > - do_div(tmp, oa_period); > > - oa_freq_hz = tmp; > > + oa_freq_hz = div_u64(NSEC_PER_SEC, (u32)oa_period); > > } else > > oa_freq_hz = 0; > > Non-blocking suggestion: this looks like it can be inlined. And if the > inline route is taken, it might be best to invert the conditional check > like such: > > oa_freq_hz = oa_period > NSEC_PER_SEC ? 0 : > div_u64(NSEC_PER_SEC, (u32)oa_period); > > I think this is just a matter of preference, though. The explicit if-else > block is definitely clearer. It's also stylistically wrong given that now the if/else don't need the brackets anymore, triggering a checkpatch error. Thorsten do you mind resending it either following Jonathan's suggestion (my favourite, as well) or fix the bracket issue following the kernel style. Andi