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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E70CC6FD18 for ; Wed, 29 Mar 2023 17:00:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230007AbjC2RAD (ORCPT ); Wed, 29 Mar 2023 13:00:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45300 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229495AbjC2RAC (ORCPT ); Wed, 29 Mar 2023 13:00:02 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B189619B5; Wed, 29 Mar 2023 10:00:01 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 4DD82B820CA; Wed, 29 Mar 2023 17:00:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9D4DDC433EF; Wed, 29 Mar 2023 16:59:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680109198; bh=VdnuvhIM2cRnRHD6OIlrF1vM7Xa/zS4dOr/IjqNNj7k=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=WGmVkGJxklNiVx1FInWIqUPg+a4lgK5x2N65ZVa820iEqX8AIhS7d7atd8Myt23wC X2Q0weysAdn2Lbmulawv6SRWEHTu7yONn0ZcgcOp0SRZAvuGDjweZcjPz2VcVse/aP jSdZwBRk93rjHWyWyALcl5jVeV0FahOn/vPHKlBKTtwj0l20pXyoKcibpoI5VgSLyA VNPS5MuyCqgBNAk6ydScLCI8EyYtX+zVGiP3U4/ztRdsOMB6bSo+QCZ91+LOpWuWL5 p8YVMnS4tdEHgrPfYrxOyaWfJVbveOrlrj4QubImSg1dOQP65Bu5IQyT3W8cZYj64h X8PZMTiWVlYdg== Date: Wed, 29 Mar 2023 11:59:57 -0500 From: Bjorn Helgaas To: Sumit Gupta Cc: treding@nvidia.com, krzysztof.kozlowski@linaro.org, dmitry.osipenko@collabora.com, viresh.kumar@linaro.org, rafael@kernel.org, jonathanh@nvidia.com, robh+dt@kernel.org, lpieralisi@kernel.org, linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, mmaddireddy@nvidia.com, kw@linux.com, bhelgaas@google.com, vidyas@nvidia.com, sanjayc@nvidia.com, ksitaraman@nvidia.com, ishah@nvidia.com, bbasu@nvidia.com Subject: Re: [Patch v4 10/10] PCI: tegra194: add interconnect support in Tegra234 Message-ID: <20230329165957.GA3066317@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8d0e4e2f-a131-ca19-e5ae-ef2349623b39@nvidia.com> Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On Wed, Mar 29, 2023 at 02:44:34PM +0530, Sumit Gupta wrote: > On 28/03/23 23:23, Bjorn Helgaas wrote: > > > +static void tegra_pcie_icc_set(struct tegra_pcie_dw *pcie) > > > +{ > > > + struct dw_pcie *pci = &pcie->pci; > > > + u32 val, speed, width; > > > + > > > + val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA); > > > + > > > + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val); > > > + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, val); > > > + > > > + val = width * (PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]) / BITS_PER_BYTE); > > > + > > > + if (icc_set_bw(pcie->icc_path, MBps_to_icc(val), 0)) > > > + dev_err(pcie->dev, "can't set bw[%u]\n", val); > > > + > > > + clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > > > > Array bounds violation; PCI_EXP_LNKSTA_CLS is 0x000f, so possible > > speed (CLS) values are 0..0xf and "speed - 1" values are -1..0xe. > > > > pcie_gen_freq[] is of size 4 (valid indices 0..3). > > > > I see that you're just *moving* this code, but might as well fix it. > > > Thank you for the review. > Will include the below change in the same patch. Please let me know if any > issue. > > - clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > + if (speed && (speed <= ARRAY_SIZE(pcie_gen_freq))) > + clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > + else > + clk_set_rate(pcie->core_clk, pcie_gen_freq[0]); I didn't notice that speed is a u32, so -1 is not a possible value. Also, it's used earlier for PCIE_SPEED2MBS_ENC(), so you could do something like this: speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val) - 1; if (speed >= ARRAY_SIZE(pcie_gen_freq)) speed = 0; val = width * (PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]) / BITS_PER_BYTE); ... clk_set_rate(pcie->core_clk, pcie_gen_freq[speed]);