All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergio Paracuellos <sergio.paracuellos@gmail.com>
To: gregkh@linuxfoundation.org
Cc: neil@brown.name, driverdev-devel@linuxdriverproject.org, gerg@kernel.org
Subject: [PATCH 1/5] staging: mt7621-pci-phy: avoid to create to different phys for a dual port one
Date: Fri, 20 Mar 2020 12:01:19 +0100	[thread overview]
Message-ID: <20200320110123.9907-2-sergio.paracuellos@gmail.com> (raw)
In-Reply-To: <20200320110123.9907-1-sergio.paracuellos@gmail.com>

This soc has two phy's for the pcie one of them using just a different
register for settig it up but sharing all the rest of the config. Until
now we was presenting this schema as three different phy's in the device
tree using the 'phy-cells' node property to discriminate an index and
setting up a complete phy for the dual port index. This sometimes worked
properly but reconfiguring the same registers twice presents sometimes
some unstable pcie links and the ports was not properly being detected.
The problems only appears on hard resets and soft resets was properly
working. Instead of having this schema just set two phy's in the device
ree and use the 'phy-cells' property to say if the port has or not a dual
port. Doing this configuration and set up becomes easier, LOC is decreased 
and the behaviour also gets deterministic with properly and stable pcie
links in both hard and soft resets.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 144 +++++++-----------
 1 file changed, 59 insertions(+), 85 deletions(-)

diff --git a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
index 648f0c8df003..63dfbb85af11 100644
--- a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
+++ b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
@@ -77,32 +77,22 @@
 
 #define MAX_PHYS	2
 
-/**
- * struct mt7621_pci_phy_instance - Mt7621 Pcie PHY device
- * @phy: pointer to the kernel PHY device
- * @port_base: base register
- * @index: internal ID to identify the Mt7621 PCIe PHY
- */
-struct mt7621_pci_phy_instance {
-	struct phy *phy;
-	void __iomem *port_base;
-	u32 index;
-};
-
 /**
  * struct mt7621_pci_phy - Mt7621 Pcie PHY core
  * @dev: pointer to device
  * @regmap: kernel regmap pointer
- * @phys: pointer to Mt7621 PHY device
- * @nphys: number of PHY devices for this core
+ * @phy: pointer to the kernel PHY device
+ * @port_base: base register
+ * @has_dual_port: if the phy has dual ports.
  * @bypass_pipe_rst: mark if 'mt7621_bypass_pipe_rst'
  * needs to be executed. Depends on chip revision.
  */
 struct mt7621_pci_phy {
 	struct device *dev;
 	struct regmap *regmap;
-	struct mt7621_pci_phy_instance **phys;
-	int nphys;
+	struct phy *phy;
+	void __iomem *port_base;
+	bool has_dual_port;
 	bool bypass_pipe_rst;
 };
 
@@ -130,23 +120,23 @@ static inline void mt7621_phy_rmw(struct mt7621_pci_phy *phy,
 	phy_write(phy, val, reg);
 }
 
-static void mt7621_bypass_pipe_rst(struct mt7621_pci_phy *phy,
-				   struct mt7621_pci_phy_instance *instance)
+static void mt7621_bypass_pipe_rst(struct mt7621_pci_phy *phy)
 {
-	u32 offset = (instance->index != 1) ?
-		RG_PE1_PIPE_REG : RG_PE1_PIPE_REG + RG_P0_TO_P1_WIDTH;
-
-	mt7621_phy_rmw(phy, offset,
-		       RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC,
-		       RG_PE1_PIPE_RST | RG_PE1_PIPE_CMD_FRC);
+	mt7621_phy_rmw(phy, RG_PE1_PIPE_REG, 0, RG_PE1_PIPE_RST);
+	mt7621_phy_rmw(phy, RG_PE1_PIPE_REG, 0, RG_PE1_PIPE_CMD_FRC);
+
+	if (phy->has_dual_port) {
+		mt7621_phy_rmw(phy, RG_PE1_PIPE_REG + RG_P0_TO_P1_WIDTH,
+			       0, RG_PE1_PIPE_RST);
+		mt7621_phy_rmw(phy, RG_PE1_PIPE_REG + RG_P0_TO_P1_WIDTH,
+			       0, RG_PE1_PIPE_CMD_FRC);
+	}
 }
 
-static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy,
-				   struct mt7621_pci_phy_instance *instance)
+static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
 {
 	struct device *dev = phy->dev;
 	u32 reg = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0);
-	u32 offset;
 
 	reg = (reg >> 6) & 0x7;
 	/* Set PCIe Port PHY to disable SSC */
@@ -156,10 +146,13 @@ static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy,
 		       RG_PE1_FRC_H_XTAL_TYPE | RG_PE1_H_XTAL_TYPE_VAL(0x00));
 
 	/* disable port */
-	offset = (instance->index != 1) ?
-		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
-	mt7621_phy_rmw(phy, offset,
-		       RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
+	mt7621_phy_rmw(phy, RG_PE1_FRC_PHY_REG,
+		       RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
+
+	if (phy->has_dual_port) {
+		mt7621_phy_rmw(phy, RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH,
+			       RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
+	}
 
 	if (reg <= 5 && reg >= 3) { /* 40MHz Xtal */
 		/* Set Pre-divider ratio (for host mode) */
@@ -223,43 +216,44 @@ static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy,
 
 static int mt7621_pci_phy_init(struct phy *phy)
 {
-	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
-	struct mt7621_pci_phy *mphy = dev_get_drvdata(phy->dev.parent);
+	struct mt7621_pci_phy *mphy = phy_get_drvdata(phy);
 
 	if (mphy->bypass_pipe_rst)
-		mt7621_bypass_pipe_rst(mphy, instance);
+		mt7621_bypass_pipe_rst(mphy);
 
-	mt7621_set_phy_for_ssc(mphy, instance);
+	mt7621_set_phy_for_ssc(mphy);
 
 	return 0;
 }
 
 static int mt7621_pci_phy_power_on(struct phy *phy)
 {
-	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
-	struct mt7621_pci_phy *mphy = dev_get_drvdata(phy->dev.parent);
-	u32 offset = (instance->index != 1) ?
-		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
+	struct mt7621_pci_phy *mphy = phy_get_drvdata(phy);
 
 	/* Enable PHY and disable force mode */
-	mt7621_phy_rmw(mphy, offset,
-		       RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN,
-		       RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN);
+	mt7621_phy_rmw(mphy, RG_PE1_FRC_PHY_REG,
+		       RG_PE1_FRC_PHY_EN, RG_PE1_PHY_EN);
+
+	if (mphy->has_dual_port) {
+		mt7621_phy_rmw(mphy, RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH,
+			       RG_PE1_FRC_PHY_EN, RG_PE1_PHY_EN);
+	}
 
 	return 0;
 }
 
 static int mt7621_pci_phy_power_off(struct phy *phy)
 {
-	struct mt7621_pci_phy_instance *instance = phy_get_drvdata(phy);
-	struct mt7621_pci_phy *mphy = dev_get_drvdata(phy->dev.parent);
-	u32 offset = (instance->index != 1) ?
-		RG_PE1_FRC_PHY_REG : RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH;
+	struct mt7621_pci_phy *mphy = phy_get_drvdata(phy);
 
 	/* Disable PHY */
-	mt7621_phy_rmw(mphy, offset,
-		       RG_PE1_FRC_PHY_EN | RG_PE1_PHY_EN,
-		       RG_PE1_FRC_PHY_EN);
+	mt7621_phy_rmw(mphy, RG_PE1_FRC_PHY_REG,
+		       RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
+
+	if (mphy->has_dual_port) {
+		mt7621_phy_rmw(mphy, RG_PE1_FRC_PHY_REG + RG_P0_TO_P1_WIDTH,
+			       RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
+	}
 
 	return 0;
 }
@@ -282,13 +276,15 @@ static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
 {
 	struct mt7621_pci_phy *mt7621_phy = dev_get_drvdata(dev);
 
-	if (args->args_count == 0)
-		return mt7621_phy->phys[0]->phy;
-
 	if (WARN_ON(args->args[0] >= MAX_PHYS))
 		return ERR_PTR(-ENODEV);
 
-	return mt7621_phy->phys[args->args[0]]->phy;
+	mt7621_phy->has_dual_port = args->args[0];
+
+	dev_info(dev, "PHY for 0x%08x (dual port = %d)\n",
+		 (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
+
+	return mt7621_phy->phy;
 }
 
 static const struct soc_device_attribute mt7621_pci_quirks_match[] = {
@@ -309,19 +305,11 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
 	struct phy_provider *provider;
 	struct mt7621_pci_phy *phy;
 	struct resource *res;
-	int port;
-	void __iomem *port_base;
 
 	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy)
 		return -ENOMEM;
 
-	phy->nphys = MAX_PHYS;
-	phy->phys = devm_kcalloc(dev, phy->nphys,
-				 sizeof(*phy->phys), GFP_KERNEL);
-	if (!phy->phys)
-		return -ENOMEM;
-
 	attr = soc_device_match(mt7621_pci_quirks_match);
 	if (attr)
 		phy->bypass_pipe_rst = true;
@@ -335,39 +323,25 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	port_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(port_base)) {
+	phy->port_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(phy->port_base)) {
 		dev_err(dev, "failed to remap phy regs\n");
-		return PTR_ERR(port_base);
+		return PTR_ERR(phy->port_base);
 	}
 
-	phy->regmap = devm_regmap_init_mmio(phy->dev, port_base,
+	phy->regmap = devm_regmap_init_mmio(phy->dev, phy->port_base,
 					    &mt7621_pci_phy_regmap_config);
 	if (IS_ERR(phy->regmap))
 		return PTR_ERR(phy->regmap);
 
-	for (port = 0; port < MAX_PHYS; port++) {
-		struct mt7621_pci_phy_instance *instance;
-		struct phy *pphy;
-
-		instance = devm_kzalloc(dev, sizeof(*instance), GFP_KERNEL);
-		if (!instance)
-			return -ENOMEM;
-
-		phy->phys[port] = instance;
-
-		pphy = devm_phy_create(dev, dev->of_node, &mt7621_pci_phy_ops);
-		if (IS_ERR(phy)) {
-			dev_err(dev, "failed to create phy\n");
-			return PTR_ERR(phy);
-		}
-
-		instance->port_base = port_base;
-		instance->phy = pphy;
-		instance->index = port;
-		phy_set_drvdata(pphy, instance);
+	phy->phy = devm_phy_create(dev, dev->of_node, &mt7621_pci_phy_ops);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create phy\n");
+		return PTR_ERR(phy);
 	}
 
+	phy_set_drvdata(phy->phy, phy);
+
 	provider = devm_of_phy_provider_register(dev, mt7621_pcie_phy_of_xlate);
 
 	return PTR_ERR_OR_ZERO(provider);
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

  reply	other threads:[~2020-03-20 11:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-20 11:01 [PATCH 0/5] staging: mt7621-pci-phy: Use only two phys to avoid unstable pcie links Sergio Paracuellos
2020-03-20 11:01 ` Sergio Paracuellos [this message]
2020-03-20 11:01 ` [PATCH 2/5] staging: mt7621-dts: set up only two pcie phys Sergio Paracuellos
2020-03-20 11:01 ` [PATCH 3/5] staging: mt7621-pci: use only two phys from device tree Sergio Paracuellos
2020-03-20 11:01 ` [PATCH 4/5] staging: mt7621-pci: change variable to print for slot Sergio Paracuellos
2020-03-20 11:01 ` [PATCH 5/5] staging: mt7621-pci: be sure gpio descriptor is null on fails Sergio Paracuellos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200320110123.9907-2-sergio.paracuellos@gmail.com \
    --to=sergio.paracuellos@gmail.com \
    --cc=driverdev-devel@linuxdriverproject.org \
    --cc=gerg@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=neil@brown.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.