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 EDF30C52D6F for ; Wed, 21 Aug 2024 07: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:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=BjeaDQ8CqSlUX+6KQTLvorw0PworcJX4Z9bMHJPFVyo=; b=KbtMbHoM5XgmbYG5VXwFX3bzDo APKNuYSs30wRzRsvZDayfv47n6LJ/WCmHMNtfKk/OQ0WOFVM1VsZYqRou8V5ez3Ww2ePuCJefFvM0 NeSCTATma9YQLCGCsX5lfVdeJfYWLxb5f0yadK/19FOenrihSeTCNEVY33Nams/X4fZZh2XcWgEhZ dF6sAOMNOU27mwRcckr+sUa+sPD7EyRR4dRuLlYVmsSVeeM4iLEygqZ9Hp5L0rEPf2aPPvfQBuYnY EbYdZ3wP8SSBXEqgyHcQqyd/r9wcZlssQodpfVfZ6+XbJSnprUHy610Cik/0I+du/NAwXGkEMrsf0 L9z4nUxA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgfQK-00000007osB-0t5o; Wed, 21 Aug 2024 07:06:28 +0000 Received: from mail-pf1-x42d.google.com ([2607:f8b0:4864:20::42d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sgfPa-00000007oiE-1zMl for linux-arm-kernel@lists.infradead.org; Wed, 21 Aug 2024 07:05:44 +0000 Received: by mail-pf1-x42d.google.com with SMTP id d2e1a72fcca58-712603f7ba5so5178175b3a.3 for ; Wed, 21 Aug 2024 00:05:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1724223941; x=1724828741; darn=lists.infradead.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=BjeaDQ8CqSlUX+6KQTLvorw0PworcJX4Z9bMHJPFVyo=; b=SjzlZciavodQX9/h2EVXbQ+QQDrg8eK2DtP6ojAVlHvAFDgTVT/+BBedAIBIyi6ABy uLbKeYhcbj4IshxwecwVN2L9IzxF+NOU6EHSNJnZbUMtnsENwtfor5+tx2mbqKEv3kv4 93sebsxxuUGHfx0/Se5CCb877WiqDH7Y8z1G27YOyLgMWBA8/pne5H8fIao9qCk+4fDm +SyPaAtcRHLMz0qz+TvP1I+2yUuWlKidJ1Dm9P0IPUgkc5uZoYTFKbF5ssrrqBpjs3jQ lV/DXQggjJwwtVODnDVVo8v2WoGLmiAqifFtqoV2EdnIQgiQvClk8gxzmiOrmlB8q//Z 5YhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724223941; x=1724828741; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BjeaDQ8CqSlUX+6KQTLvorw0PworcJX4Z9bMHJPFVyo=; b=mJ2HbW8BMmFBqqlumzuCP5DCcZZRCeLzbyMpccSEQr2FQtoTLqCHJBvBMsc9heFXDl c6AsILc/VzCF8RefOUOHTsusFcA3HXhZT0WOV3Wp5Z/fYHLs9s9nVs6GZuPE/8qYjqqz iby7KgVIySa9BSk5xG7XXCaGMSbM35v+eTYcw5a+H+0Rq0avdC5THfheUp7ODQjKgwat W54yIEt6BSz5PkzPWIOnPqFM3caEfz2VcnpG0OvQoIqJ6BTtKQMUsiYih1h9MZYI8H26 wupSt6Mvh1wZn0zBnthDhMe/uMeE5Ufap5LvbcdxTFruak7KZ66MTkwtJYxrwFBnbsM9 MM2g== X-Forwarded-Encrypted: i=1; AJvYcCV0G5fgrw4dk9CtCl79d8ZSRE7w+LPAuaFjBP//3Sy4RP1kdOXTH2wWlfe+KC4rmQX8TqrczPAwx2YkqKZLQqIs@lists.infradead.org X-Gm-Message-State: AOJu0YyhrbA4JjGtwtwZuzQ7FOjBkZrzRpdqTM0FQTnNsSTRst4UMAMr f+aoaY69qEfVO6i4yP95uavBmWcUZKbjhSG1jajqAS+C4WwPpeS16CEN26umNA== X-Google-Smtp-Source: AGHT+IGTTYXfSAyUkjeZvaungm1XIw/2ET1TYmDIvuu0czZ/vNdPJ8c0pxkRctUuVn27J8snAv+4pg== X-Received: by 2002:a05:6a20:3b84:b0:1ca:da51:6635 with SMTP id adf61e73a8af0-1cada516753mr1150687637.1.1724223941057; Wed, 21 Aug 2024 00:05:41 -0700 (PDT) Received: from thinkpad ([120.60.137.118]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7cd8ce64ef3sm360604a12.59.2024.08.21.00.05.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Aug 2024 00:05:40 -0700 (PDT) Date: Wed, 21 Aug 2024 12:35:33 +0530 From: Manivannan Sadhasivam To: Anand Moon Cc: Shawn Lin , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Bjorn Helgaas , Heiko Stuebner , Philipp Zabel , linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 2/3] PCI: rockchip: Simplify reset control handling by using reset_control_bulk*() function Message-ID: <20240821070533.wztutnvlpghzso6v@thinkpad> References: <20240625104039.48311-1-linux.amoon@gmail.com> <20240625104039.48311-2-linux.amoon@gmail.com> <20240815162004.GF2562@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240821_000542_568349_240C9D9F X-CRM114-Status: GOOD ( 32.98 ) 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 Sat, Aug 17, 2024 at 06:52:47PM +0530, Anand Moon wrote: > Hi Manivannan, > > On Thu, 15 Aug 2024 at 21:50, Manivannan Sadhasivam > wrote: > > > > On Tue, Jun 25, 2024 at 04:10:33PM +0530, Anand Moon wrote: > > > Refactor the reset control handling in the Rockchip PCIe driver, > > > introducing a more robust and efficient method for assert and > > > deassert reset controller using reset_control_bulk*() API. Using the > > > reset_control_bulk APIs, the reset handling for the core clocks reset > > > unit becomes much simpler. > > > > > > As per rockchip rk3399 TRM SOFTRST_CON8 soft reset controller > > > have clock reset unit value set to 0x1 for example "pcie_pipe", > > > "pcie_mgmt_sticky", "pcie_mgmt" and "pci_core", hence group then under > > > one reset bulk controller. > > > > > > Where as "pcie_pm", "presetn_pcie", "aresetn_pcie" have reset value > > > set to 0x0, hence group them under reset control bulk controller. > > > > > > Signed-off-by: Anand Moon > > > --- > > > v4: use dev_err_probe in error path. > > > v3: Fix typo in commit message, dropped reported by. > > > v2: Fix compilation error reported by Intel test robot > > > fixed checkpatch warning > > > --- > > > drivers/pci/controller/pcie-rockchip.c | 149 +++++-------------------- > > > drivers/pci/controller/pcie-rockchip.h | 25 +++-- > > > 2 files changed, 47 insertions(+), 127 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c > > > index 804135511528..024308bb6ac8 100644 > > > --- a/drivers/pci/controller/pcie-rockchip.c > > > +++ b/drivers/pci/controller/pcie-rockchip.c > > > @@ -69,55 +69,23 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) > > > if (rockchip->link_gen < 0 || rockchip->link_gen > 2) > > > rockchip->link_gen = 2; > > > > > > - rockchip->core_rst = devm_reset_control_get_exclusive(dev, "core"); > > > - if (IS_ERR(rockchip->core_rst)) { > > > - if (PTR_ERR(rockchip->core_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing core reset property in node\n"); > > > - return PTR_ERR(rockchip->core_rst); > > > - } > > > - > > > - rockchip->mgmt_rst = devm_reset_control_get_exclusive(dev, "mgmt"); > > > - if (IS_ERR(rockchip->mgmt_rst)) { > > > - if (PTR_ERR(rockchip->mgmt_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing mgmt reset property in node\n"); > > > - return PTR_ERR(rockchip->mgmt_rst); > > > - } > > > - > > > - rockchip->mgmt_sticky_rst = devm_reset_control_get_exclusive(dev, > > > - "mgmt-sticky"); > > > - if (IS_ERR(rockchip->mgmt_sticky_rst)) { > > > - if (PTR_ERR(rockchip->mgmt_sticky_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing mgmt-sticky reset property in node\n"); > > > - return PTR_ERR(rockchip->mgmt_sticky_rst); > > > - } > > > - > > > - rockchip->pipe_rst = devm_reset_control_get_exclusive(dev, "pipe"); > > > - if (IS_ERR(rockchip->pipe_rst)) { > > > - if (PTR_ERR(rockchip->pipe_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing pipe reset property in node\n"); > > > - return PTR_ERR(rockchip->pipe_rst); > > > - } > > > + for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++) > > > + rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i]; > > > > > > - rockchip->pm_rst = devm_reset_control_get_exclusive(dev, "pm"); > > > - if (IS_ERR(rockchip->pm_rst)) { > > > - if (PTR_ERR(rockchip->pm_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing pm reset property in node\n"); > > > - return PTR_ERR(rockchip->pm_rst); > > > - } > > > + err = devm_reset_control_bulk_get_optional_exclusive(dev, > > > + ROCKCHIP_NUM_PM_RSTS, > > > + rockchip->pm_rsts); > > > + if (err) > > > + return dev_err_probe(dev, err, "cannot get the reset control\n"); > > > > > > - rockchip->pclk_rst = devm_reset_control_get_exclusive(dev, "pclk"); > > > - if (IS_ERR(rockchip->pclk_rst)) { > > > - if (PTR_ERR(rockchip->pclk_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing pclk reset property in node\n"); > > > - return PTR_ERR(rockchip->pclk_rst); > > > - } > > > + for (i = 0; i < ROCKCHIP_NUM_CORE_RSTS; i++) > > > + rockchip->core_rsts[i].id = rockchip_pci_core_rsts[i]; > > > > > > - rockchip->aclk_rst = devm_reset_control_get_exclusive(dev, "aclk"); > > > - if (IS_ERR(rockchip->aclk_rst)) { > > > - if (PTR_ERR(rockchip->aclk_rst) != -EPROBE_DEFER) > > > - dev_err(dev, "missing aclk reset property in node\n"); > > > - return PTR_ERR(rockchip->aclk_rst); > > > - } > > > + err = devm_reset_control_bulk_get_optional_exclusive(dev, > > > + ROCKCHIP_NUM_CORE_RSTS, > > > + rockchip->core_rsts); > > > + if (err) > > > + return dev_err_probe(dev, err, "cannot get the reset control\n"); > > > > > > if (rockchip->is_rc) { > > > rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep", > > > @@ -150,23 +118,10 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) > > > int err, i; > > > u32 regs; > > > > > > - err = reset_control_assert(rockchip->aclk_rst); > > > - if (err) { > > > - dev_err(dev, "assert aclk_rst err %d\n", err); > > > - return err; > > > - } > > > - > > > - err = reset_control_assert(rockchip->pclk_rst); > > > - if (err) { > > > - dev_err(dev, "assert pclk_rst err %d\n", err); > > > - return err; > > > - } > > > - > > > - err = reset_control_assert(rockchip->pm_rst); > > > - if (err) { > > > - dev_err(dev, "assert pm_rst err %d\n", err); > > > - return err; > > > - } > > > + err = reset_control_bulk_assert(ROCKCHIP_NUM_PM_RSTS, > > > + rockchip->pm_rsts); > > > + if (err) > > > + return dev_err_probe(dev, err, "reset bulk assert pm reset\n"); > > > > > > for (i = 0; i < MAX_LANE_NUM; i++) { > > > err = phy_init(rockchip->phys[i]); > > > @@ -176,47 +131,17 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) > > > } > > > } > > > > > > - err = reset_control_assert(rockchip->core_rst); > > > - if (err) { > > > - dev_err(dev, "assert core_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > - > > > - err = reset_control_assert(rockchip->mgmt_rst); > > > - if (err) { > > > - dev_err(dev, "assert mgmt_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > - > > > - err = reset_control_assert(rockchip->mgmt_sticky_rst); > > > - if (err) { > > > - dev_err(dev, "assert mgmt_sticky_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > - > > > - err = reset_control_assert(rockchip->pipe_rst); > > > - if (err) { > > > - dev_err(dev, "assert pipe_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > + err = reset_control_bulk_assert(ROCKCHIP_NUM_CORE_RSTS, > > > + rockchip->core_rsts); > > > + if (err) > > > + return dev_err_probe(dev, err, "reset bulk assert core reset\n"); > > > > > > udelay(10); > > > > > > - err = reset_control_deassert(rockchip->pm_rst); > > > - if (err) { > > > - dev_err(dev, "deassert pm_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > - > > > - err = reset_control_deassert(rockchip->aclk_rst); > > > - if (err) { > > > - dev_err(dev, "deassert aclk_rst err %d\n", err); > > > - goto err_exit_phy; > > > - } > > > - > > > - err = reset_control_deassert(rockchip->pclk_rst); > > > + err = reset_control_bulk_deassert(ROCKCHIP_NUM_PM_RSTS, > > > + rockchip->pm_rsts); > > > if (err) { > > > - dev_err(dev, "deassert pclk_rst err %d\n", err); > > > + dev_err(dev, "reset bulk deassert pm err %d\n", err); > > > goto err_exit_phy; > > > } > > > > > > @@ -259,31 +184,15 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) > > > * Please don't reorder the deassert sequence of the following > > > * four reset pins. > > > */ > > > > The comment above says that the resets should not be reordered. But you have > > reordered the resets. > > > As per the TRM [1], CRU_SOFTRST_CON8 clock reset unit has two groups > one with reset value 0x1 and the other 0x0, so this patch groups them > accordingly. > > [1] https://opensource.rock-chips.com/images/e/ee/Rockchip_RK3399TRM_V1.4_Part1-20170408.pdf > > If I only use reset_control_bulk_assert and > reset_control_bulk_deassert for all the reset > I get the below reset warning. > I think you misunderstood what I asked for. The comment says that the 4 resets (mgmt-sticky, core, mgmt, pipe) should not be reordered. In your new group rockchip_pci_core_rsts(), you have reordered them. I was just asking you to keep the 4 resets sorted as per the comment. - Mani -- மணிவண்ணன் சதாசிவம்