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 X-Spam-Level: X-Spam-Status: No, score=-17.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68DBDC433ED for ; Sat, 17 Apr 2021 10:51:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 43BCD60FEA for ; Sat, 17 Apr 2021 10:51:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236123AbhDQKv6 (ORCPT ); Sat, 17 Apr 2021 06:51:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:55712 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229972AbhDQKvz (ORCPT ); Sat, 17 Apr 2021 06:51:55 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id B5370606A5; Sat, 17 Apr 2021 10:51:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618656689; bh=9/tc/wXYRdUG9mbYlOPIkpy2FgsFbviR+V3KchkASus=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=erFgjgVe401+n/oW1Hd169bCXH1l6m0QQpzE0y0a283DsaleeoJf5VAivuyQsPFCf mK2KoN6wD9yj0MOB2PmxytRqp3MibeN3+6wtTC90/Z/dV1tScMqSsQxqfK/6i0k6BY cEy1E1JZ8V2To+zy1y/DeNoFaNZZ8fCxo1SAxxZomKqJE7cYKBSGoE4qbHkRjvmVWO RnCYS+XtYzoAN4z0SdmbtgU9dpFa3WVyLUFf3fBZUYeddp+TYWAt4AqX9mBD7ULiR+ F287YJ8TmILBxIHOlmvdH/Y5HSA2W5i8JWCYKvc12Be7pEFYEBBzPVWaG0tLqbI+91 RAfL6XklHE2uA== Received: by pali.im (Postfix) id 453B09F7; Sat, 17 Apr 2021 12:51:26 +0200 (CEST) Date: Sat, 17 Apr 2021 12:51:26 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: bpeled@marvell.com Cc: thomas.petazzoni@bootlin.com, lorenzo.pieralisi@arm.com, bhelgaas@google.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, sebastian.hesselbarth@gmail.com, gregory.clement@bootlin.com, andrew@lunn.ch, robh+dt@kernel.org, mw@semihalf.com, jaz@semihalf.com, kostap@marvell.com, nadavh@marvell.com, stefanc@marvell.com, oferh@marvell.com Subject: Re: =?utf-8?B?W+KAnVBBVENI?= =?utf-8?B?4oCd?= v2 2/5] PCI: armada8k: Add link-down handle Message-ID: <20210417105126.qrqmviqnbppktzw7@pali> References: <1618406454-7953-1-git-send-email-bpeled@marvell.com> <1618406454-7953-3-git-send-email-bpeled@marvell.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1618406454-7953-3-git-send-email-bpeled@marvell.com> User-Agent: NeoMutt/20180716 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 14 April 2021 16:20:51 bpeled@marvell.com wrote: > From: Ben Peled > > In PCIE ISR routine caused by RST_LINK_DOWN > we schedule work to handle the link-down procedure. > Link-down procedure will: > 1. Remove PCIe bus > 2. Reset the MAC > 3. Reconfigure link back up > 4. Rescan PCIe bus Hello! This looks like a part of PCIe Hot Plug procedure, which is already handled by kernel pci hotplug code, it can detect link down interrupt and remove PCI device from the bus. I'm not sure if it would not be better to use existing "infrastructure" for hotplug rather then implementing new in controller driver. But I do not know what is "correct" way, so I hope that other people (maybe Bjorn?) would say something about this topic. > Signed-off-by: Ben Peled > --- > drivers/pci/controller/dwc/pcie-armada8k.c | 69 ++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c > index b2278b1..34b253c 100644 > --- a/drivers/pci/controller/dwc/pcie-armada8k.c > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c > @@ -22,6 +22,8 @@ > #include > #include > #include > +#include > +#include > > #include "pcie-designware.h" > > @@ -33,6 +35,9 @@ struct armada8k_pcie { > struct clk *clk_reg; > struct phy *phy[ARMADA8K_PCIE_MAX_LANES]; > unsigned int phy_count; > + struct regmap *sysctrl_base; > + u32 mac_rest_bitmask; > + struct work_struct recover_link_work; > }; > > #define PCIE_VENDOR_REGS_OFFSET 0x8000 > @@ -73,6 +78,8 @@ struct armada8k_pcie { > #define AX_USER_DOMAIN_MASK 0x3 > #define AX_USER_DOMAIN_SHIFT 4 > > +#define UNIT_SOFT_RESET_CONFIG_REG 0x268 > + > #define to_armada8k_pcie(x) dev_get_drvdata((x)->dev) > > static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie) > @@ -225,6 +232,50 @@ static int armada8k_pcie_host_init(struct pcie_port *pp) > return 0; > } > > +static void armada8k_pcie_recover_link(struct work_struct *ws) > +{ > + struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work); > + struct pcie_port *pp = &pcie->pci->pp; > + struct pci_bus *bus = pp->bridge->bus; > + struct pci_dev *root_port; > + int ret; > + > + root_port = pci_get_slot(bus, 0); > + if (!root_port) { > + dev_err(pcie->pci->dev, "failed to get root port\n"); > + return; > + } > + pci_lock_rescan_remove(); > + pci_stop_and_remove_bus_device(root_port); > + /* > + * Sleep needed to make sure all pcie transactions and access > + * are flushed before resetting the mac > + */ > + msleep(100); > + > + /* Reset mac */ > + regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > + pcie->mac_rest_bitmask, 0, NULL, false, true); > + udelay(1); > + regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > + pcie->mac_rest_bitmask, pcie->mac_rest_bitmask, > + NULL, false, true); > + udelay(1); > + ret = armada8k_pcie_host_init(pp); > + if (ret) { > + dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret); > + pci_unlock_rescan_remove(); > + pci_dev_put(root_port); > + return; > + } > + > + bus = NULL; > + while ((bus = pci_find_next_bus(bus)) != NULL) > + pci_rescan_bus(bus); > + pci_unlock_rescan_remove(); > + pci_dev_put(root_port); > +} > + > static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) > { > struct armada8k_pcie *pcie = arg; > @@ -262,6 +313,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) > * initiate a link retrain. If link retrains were > * possible, that is. > */ > + if (pcie->sysctrl_base && pcie->mac_rest_bitmask) > + schedule_work(&pcie->recover_link_work); > + > dev_dbg(pci->dev, "%s: link went down\n", __func__); > } > > @@ -330,6 +384,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > > pcie->pci = pci; > > + INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link); > + > pcie->clk = devm_clk_get(dev, NULL); > if (IS_ERR(pcie->clk)) > return PTR_ERR(pcie->clk); > @@ -357,6 +413,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > goto fail_clkreg; > } > > + pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > + "marvell,system-controller"); > + if (IS_ERR(pcie->sysctrl_base)) { > + dev_warn(dev, "failed to find marvell,system-controller\n"); > + pcie->sysctrl_base = 0x0; > + } > + > + ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask", > + &pcie->mac_rest_bitmask); > + if (ret < 0) { > + dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret); > + pcie->mac_rest_bitmask = 0x0; > + } > ret = armada8k_pcie_setup_phys(pcie); > if (ret) > goto fail_clkreg; > -- > 2.7.4 > 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 X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AB73C433ED for ; Sat, 17 Apr 2021 10:53:35 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0020B610E8 for ; Sat, 17 Apr 2021 10:53:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0020B610E8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To: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=I1ptTmc1Zfi389CLHlrCu1B2xetmsXeUsFvDw9nNCoQ=; b=fgJVhtpjaV5G0UA/DAJFx7Z1X s5HvD5VSOgVUvmEND8p+YQbFrKE5k4eyAgyn5Gzrg/v5YGzWT334eo2QthS/BiE/ZLBQvhzRPgKbK KsNzXh2ftjBx+1MXpgBgXfXMUYTeUU3/yKDd5m2vWcCkukzax74xxMR0ZSAhub66vaw4pWk+8WX2z ZstUT7JRS0Z8Z2dme/E0hOYejusSd2u7QJSOS/4u8jPlyBfWHIcVw76EHmK5yWQFYqAbz7I7wzuuI QPwYegbKcEFyh7z6mjUFym3/YzcatH4wG6eAEaIJ1IFAqjucp+eXvN5ZFkDI7jAso4D8Z21RZOLrb PjM7louXA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lXiY5-0053fh-S9; Sat, 17 Apr 2021 10:51:38 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lXiY1-0053fY-0J for linux-arm-kernel@desiato.infradead.org; Sat, 17 Apr 2021 10:51:33 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=oNmA8OSmew+AjM6ynz1BrHR2P5oxxJJKKYiuiW1Zffw=; b=2iRlGNuHkLd0S8ReSJNgfluk44 bXf2Fjde7EYtfk6SQTlOF7wgFkM3D+JkQhl89oFy0ABVK7Ms6RizGAbYZGkrp7Prjvvbup2aTwf57 Qu6+5ln46Q+I2Bg6EnRNC3r1FD2macqEmY3EqiRivVYFpCOWSlgqrZORGEnLH78c3JVPfifKf6nd9 uXu5N6MZEK5xSzylnhEQtexu76QgRDNsjutyDqEqsxI0HBiAX3VXxzdigm1J/d5F0zFcHaga0hap3 pSmsefrO8olUsaHH9chRB9SB6cr199pHJXW9rY9VpmrPD/+6M4RmAXsjKiWbWT9yzaWqtpFKfQqBo hTqg7bfw==; Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lXiXy-009zMc-2w for linux-arm-kernel@lists.infradead.org; Sat, 17 Apr 2021 10:51:31 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id B5370606A5; Sat, 17 Apr 2021 10:51:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618656689; bh=9/tc/wXYRdUG9mbYlOPIkpy2FgsFbviR+V3KchkASus=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=erFgjgVe401+n/oW1Hd169bCXH1l6m0QQpzE0y0a283DsaleeoJf5VAivuyQsPFCf mK2KoN6wD9yj0MOB2PmxytRqp3MibeN3+6wtTC90/Z/dV1tScMqSsQxqfK/6i0k6BY cEy1E1JZ8V2To+zy1y/DeNoFaNZZ8fCxo1SAxxZomKqJE7cYKBSGoE4qbHkRjvmVWO RnCYS+XtYzoAN4z0SdmbtgU9dpFa3WVyLUFf3fBZUYeddp+TYWAt4AqX9mBD7ULiR+ F287YJ8TmILBxIHOlmvdH/Y5HSA2W5i8JWCYKvc12Be7pEFYEBBzPVWaG0tLqbI+91 RAfL6XklHE2uA== Received: by pali.im (Postfix) id 453B09F7; Sat, 17 Apr 2021 12:51:26 +0200 (CEST) Date: Sat, 17 Apr 2021 12:51:26 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: bpeled@marvell.com Cc: thomas.petazzoni@bootlin.com, lorenzo.pieralisi@arm.com, bhelgaas@google.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, sebastian.hesselbarth@gmail.com, gregory.clement@bootlin.com, andrew@lunn.ch, robh+dt@kernel.org, mw@semihalf.com, jaz@semihalf.com, kostap@marvell.com, nadavh@marvell.com, stefanc@marvell.com, oferh@marvell.com Subject: Re: =?utf-8?B?W+KAnVBBVENI?= =?utf-8?B?4oCd?= v2 2/5] PCI: armada8k: Add link-down handle Message-ID: <20210417105126.qrqmviqnbppktzw7@pali> References: <1618406454-7953-1-git-send-email-bpeled@marvell.com> <1618406454-7953-3-git-send-email-bpeled@marvell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1618406454-7953-3-git-send-email-bpeled@marvell.com> User-Agent: NeoMutt/20180716 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210417_035130_216387_497EFE89 X-CRM114-Status: GOOD ( 27.57 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wednesday 14 April 2021 16:20:51 bpeled@marvell.com wrote: > From: Ben Peled > > In PCIE ISR routine caused by RST_LINK_DOWN > we schedule work to handle the link-down procedure. > Link-down procedure will: > 1. Remove PCIe bus > 2. Reset the MAC > 3. Reconfigure link back up > 4. Rescan PCIe bus Hello! This looks like a part of PCIe Hot Plug procedure, which is already handled by kernel pci hotplug code, it can detect link down interrupt and remove PCI device from the bus. I'm not sure if it would not be better to use existing "infrastructure" for hotplug rather then implementing new in controller driver. But I do not know what is "correct" way, so I hope that other people (maybe Bjorn?) would say something about this topic. > Signed-off-by: Ben Peled > --- > drivers/pci/controller/dwc/pcie-armada8k.c | 69 ++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c > index b2278b1..34b253c 100644 > --- a/drivers/pci/controller/dwc/pcie-armada8k.c > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c > @@ -22,6 +22,8 @@ > #include > #include > #include > +#include > +#include > > #include "pcie-designware.h" > > @@ -33,6 +35,9 @@ struct armada8k_pcie { > struct clk *clk_reg; > struct phy *phy[ARMADA8K_PCIE_MAX_LANES]; > unsigned int phy_count; > + struct regmap *sysctrl_base; > + u32 mac_rest_bitmask; > + struct work_struct recover_link_work; > }; > > #define PCIE_VENDOR_REGS_OFFSET 0x8000 > @@ -73,6 +78,8 @@ struct armada8k_pcie { > #define AX_USER_DOMAIN_MASK 0x3 > #define AX_USER_DOMAIN_SHIFT 4 > > +#define UNIT_SOFT_RESET_CONFIG_REG 0x268 > + > #define to_armada8k_pcie(x) dev_get_drvdata((x)->dev) > > static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie) > @@ -225,6 +232,50 @@ static int armada8k_pcie_host_init(struct pcie_port *pp) > return 0; > } > > +static void armada8k_pcie_recover_link(struct work_struct *ws) > +{ > + struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work); > + struct pcie_port *pp = &pcie->pci->pp; > + struct pci_bus *bus = pp->bridge->bus; > + struct pci_dev *root_port; > + int ret; > + > + root_port = pci_get_slot(bus, 0); > + if (!root_port) { > + dev_err(pcie->pci->dev, "failed to get root port\n"); > + return; > + } > + pci_lock_rescan_remove(); > + pci_stop_and_remove_bus_device(root_port); > + /* > + * Sleep needed to make sure all pcie transactions and access > + * are flushed before resetting the mac > + */ > + msleep(100); > + > + /* Reset mac */ > + regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > + pcie->mac_rest_bitmask, 0, NULL, false, true); > + udelay(1); > + regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > + pcie->mac_rest_bitmask, pcie->mac_rest_bitmask, > + NULL, false, true); > + udelay(1); > + ret = armada8k_pcie_host_init(pp); > + if (ret) { > + dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret); > + pci_unlock_rescan_remove(); > + pci_dev_put(root_port); > + return; > + } > + > + bus = NULL; > + while ((bus = pci_find_next_bus(bus)) != NULL) > + pci_rescan_bus(bus); > + pci_unlock_rescan_remove(); > + pci_dev_put(root_port); > +} > + > static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) > { > struct armada8k_pcie *pcie = arg; > @@ -262,6 +313,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) > * initiate a link retrain. If link retrains were > * possible, that is. > */ > + if (pcie->sysctrl_base && pcie->mac_rest_bitmask) > + schedule_work(&pcie->recover_link_work); > + > dev_dbg(pci->dev, "%s: link went down\n", __func__); > } > > @@ -330,6 +384,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > > pcie->pci = pci; > > + INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link); > + > pcie->clk = devm_clk_get(dev, NULL); > if (IS_ERR(pcie->clk)) > return PTR_ERR(pcie->clk); > @@ -357,6 +413,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > goto fail_clkreg; > } > > + pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > + "marvell,system-controller"); > + if (IS_ERR(pcie->sysctrl_base)) { > + dev_warn(dev, "failed to find marvell,system-controller\n"); > + pcie->sysctrl_base = 0x0; > + } > + > + ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask", > + &pcie->mac_rest_bitmask); > + if (ret < 0) { > + dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret); > + pcie->mac_rest_bitmask = 0x0; > + } > ret = armada8k_pcie_setup_phys(pcie); > if (ret) > goto fail_clkreg; > -- > 2.7.4 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel