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 68299C71148 for ; Wed, 28 Aug 2024 21:20:13 +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:In-Reply-To:Content-Type: MIME-Version:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=YzuKZCo6MN+88vx6TtEXGpZ61/mvO2X1C8KPnkyGeZw=; b=LwZXT0RzhKfyPF BByceZZXluBk3VTgTpkwlSJ0mD/MNVPwNGuGfxX2O5POfsZfFBx5PtWFZMrnqzgVq7ulxpTmLCskB wRfJQeVikXufqvwYhjhOv9O+1624i9Ph+pubfwn9+C/WL45dJlCSfVDrwxpA1fNYSbCCLdNXfDUcL W+Rjc6+XLnsJiG6C8cLZ167DXpX17NLGXP7S8Aze/99TRaFfDeh73coOR2pmRopAel4Q88wOC5RUR yCyPRAUUWhtY63vEKxis9t4QpjEKNdagGE+6VArV1BzEBApikkizCKCD7sIx8EMXyeFZ40Mx3rOE8 +VOlyefQ/ORtix0GSTkQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sjQ5D-0000000H3En-3KRb; Wed, 28 Aug 2024 21:20:03 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sjQ4N-0000000H35B-0OPB for linux-arm-kernel@lists.infradead.org; Wed, 28 Aug 2024 21:19:12 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 52BA6A4365F; Wed, 28 Aug 2024 21:19:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A9E37C4CEC3; Wed, 28 Aug 2024 21:19:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1724879949; bh=49PHB8qyunhNwkqPu8maqVRtY1UPnUcFp4zqIf0kXcY=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=pVFp21+BRU63IIpfRMbslg+8XpR0EjCfJpCQik0ChLei1TqYjx+Huu9B+GgR30ELW FDqVR2GpORk5bS++6AD4hEBeA14/Br+lrgnv7rfsp2HnPJcxwG+sJmd2mhWEHjFWtd e8h38p7fw54LCX1XFpK/lzWHBTCTzw1jr46hC21QyuasPUC50Xq2F5jDXbGh1bqnQL S8/NSn1O2jHXHY0mUd+cD8vX2tauqBnjYQQIVaD0Wv11BQnJ2ygq7UlIj9e+zdI3sw tWjZB3RwR7tOTO+LYTqQNBDxpUUbCK3FbBguUI/FPvFN3a/RSMMIigUe8Xxq8AaYvT E76oUpgeyQPoQ== Date: Wed, 28 Aug 2024 16:19:06 -0500 From: Bjorn Helgaas To: Siddharth Vadapalli Cc: bhelgaas@google.com, lpieralisi@kernel.org, kw@linux.com, manivannan.sadhasivam@linaro.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, vigneshr@ti.com, kishon@kernel.org, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, srk@ti.com Subject: Re: [PATCH v3 2/2] PCI: j721e: Enable ACSPCIE Refclk if "ti,syscon-acspcie-proxy-ctrl" exists Message-ID: <20240828211906.GA38267@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240827055548.901285-3-s-vadapalli@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240828_141911_288567_A7BD9F40 X-CRM114-Status: GOOD ( 31.75 ) 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 On Tue, Aug 27, 2024 at 11:25:48AM +0530, Siddharth Vadapalli wrote: > The ACSPCIE module is capable of driving the reference clock required by > the PCIe Endpoint device. It is an alternative to on-board and external > reference clock generators. Enabling the output from the ACSPCIE module's > PAD IO Buffers requires clearing the "PAD IO disable" bits of the > ACSPCIE_PROXY_CTRL register in the CTRL_MMR register space. > > Add support to enable the ACSPCIE reference clock output using the optional > device-tree property "ti,syscon-acspcie-proxy-ctrl". > > Signed-off-by: Siddharth Vadapalli > --- > > v2: > https://lore.kernel.org/r/20240729092855.1945700-3-s-vadapalli@ti.com/ > Changes since v2: > - Rebased patch on next-20240826. > > v1: > https://lore.kernel.org/r/20240715120936.1150314-4-s-vadapalli@ti.com/ > Changes since v1: > - Addressed Bjorn's feedback at: > https://lore.kernel.org/r/20240725211841.GA859405@bhelgaas/ > with the following changes: > 1) Updated $subject and commit message to indicate that this patch > enables ACSPCIE reference clock output if the DT property is present. > 2) Updated macro and comments to indicate that the BITS correspond to > disabling ACSPCIE output, due to which clearing them enables the > reference clock output. > 3) Replaced "PAD" with "refclk" both in the function name and in the > error prints. > 4) Wrapped lines to be within the 80 character limit to match the rest > of the driver. > > drivers/pci/controller/cadence/pci-j721e.c | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 85718246016b..ed42b2229483 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -44,6 +44,7 @@ enum link_status { > #define J721E_MODE_RC BIT(7) > #define LANE_COUNT(n) ((n) << 8) > > +#define ACSPCIE_PAD_DISABLE_MASK GENMASK(1, 0) > #define GENERATION_SEL_MASK GENMASK(1, 0) > > struct j721e_pcie { > @@ -220,6 +221,34 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie, > return ret; > } > > +static int j721e_enable_acspcie_refclk(struct j721e_pcie *pcie, > + struct regmap *syscon) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + struct device_node *node = dev->of_node; > + u32 mask = ACSPCIE_PAD_DISABLE_MASK; > + struct of_phandle_args args; > + u32 val; > + int ret; > + > + ret = of_parse_phandle_with_fixed_args(node, > + "ti,syscon-acspcie-proxy-ctrl", > + 1, 0, &args); > + if (!ret) { > + /* Clear PAD IO disable bits to enable refclk output */ > + val = ~(args.args[0]); > + ret = regmap_update_bits(syscon, 0, mask, val); > + if (ret) > + dev_err(dev, "failed to enable ACSPCIE refclk: %d\n", > + ret); > + } else { > + dev_err(dev, > + "ti,syscon-acspcie-proxy-ctrl has invalid arguments\n"); > + } I should have mentioned this the first time, but this would be easier to read if structured as: ret = of_parse_phandle_with_fixed_args(...); if (ret) { dev_err(...); return ret; } /* Clear PAD IO disable bits to enable refclk output */ val = ~(args.args[0]); ret = regmap_update_bits(syscon, 0, mask, val); if (ret) { dev_err(...); return ret; } return 0; > + return ret; > +} > + > static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) > { > struct device *dev = pcie->cdns_pcie->dev; > @@ -259,6 +288,15 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) > return ret; > } > > + /* Enable ACSPCIE refclk output if the optional property exists */ > + syscon = syscon_regmap_lookup_by_phandle_optional(node, > + "ti,syscon-acspcie-proxy-ctrl"); > + if (syscon) { > + ret = j721e_enable_acspcie_refclk(pcie, syscon); > + if (ret) > + return ret; > + } > + > return 0; Not as dramatic here, but I think the following would be a little simpler since the final "return" isn't used for two purposes ((1) syscon property absent, (2) syscon present and refclk successfully enabled): syscon = syscon_regmap_lookup_by_phandle_optional(...); if (!syscon) return 0; return j721e_enable_acspcie_refclk(...); > } > > -- > 2.40.1 >