[PATCH v6 04/33] staging: mt7621-pci: factor out 'mt7621_pcie_enable_port' function

NeilBrown neil at brown.name
Fri Nov 23 22:44:53 UTC 2018


On Sun, Nov 04 2018, Sergio Paracuellos wrote:

> Driver probe function is a mess and shall be refactored a lot. At first
> make use of assert and deassert control factoring out a new function
> called 'mt7621_pcie_enable_port'.

Testing continues....

there are 2.5 problems with this patch.

Firstly you changed the asserting of reset from

> -	ASSERT_SYSRST_PCIE(RALINK_PCIE0_RST | RALINK_PCIE1_RST | RALINK_PCIE2_RST);

to
> +	reset_control_assert(port->pcie_rst);
(for each port).
This looks reasonable, but doesn't work.

#define ASSERT_SYSRST_PCIE(val)		\
	do {								\
		if (rt_sysc_r32(SYSC_REG_CHIP_REV) == 0x00030101)	\
			rt_sysc_m32(0, val, RALINK_RSTCTRL);		\
		else							\
			rt_sysc_m32(val, 0, RALINK_RSTCTRL);		\
	} while (0)

If the CHIP_REV is 0x30101, then we set the bit to assert (and clear to
deassert).
This is what reset_control_assert() does - it maps through to
ralink_assert_device().
My CHIP_REV is 0x30103 - so this does the wrong thing.

Secondly you have moved the updating of RALINK_PCI_PCIMSK_ADDR (I'm
guess that is the import piece) to before the
 read_config(pcie, slot, 0x70c);
This seems to break things.
If I move the read_config/dev_info() inside the new
mt7621_pcie_enable_port(), just after the reset(), it starts working
again.

Finally, the 1/2 problem is that there was previously a 300 msec delay
between asserting reset and deasserting it - you've removed that.
It still seems to work, so maybe it is OK.  But often hardware prefers
the reset to be held down for some minimum time.  So I'd feel more
comfortable having a msleep(100) while the port is in reset.

Below is my current fix-up patch which make the board work again after
this patch.  Swapping 'assert' and 'deassert' is obviously just a hack -
some more proper solution is required.

Thanks,
NeilBrown


diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 9be5ca109a1b..6e32fbef9441 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -494,12 +494,16 @@ static int mt7621_pcie_enable_port(struct mt7621_pcie_port *port)
 		return err;
 	}
 
-	reset_control_assert(port->pcie_rst);
 	reset_control_deassert(port->pcie_rst);
+	msleep(100);
+	reset_control_assert(port->pcie_rst);
+
+	val = read_config(pcie, slot, 0x70c);
+	dev_info(dev, "Port %d N_FTS = %x\n", (unsigned int)val, slot);
 
 	if ((pcie_port_read(port, RALINK_PCI_STATUS) & 0x1) == 0) {
 		dev_err(dev, "pcie%d no card, disable it (RST & CLK)\n", slot);
-		reset_control_assert(port->pcie_rst);
+		reset_control_deassert(port->pcie_rst);
 		rt_sysc_m32(BIT(24 + slot), 0, RALINK_CLKCFG1);
 		pcie_link_status &= ~(1 << slot);
 	} else {
@@ -601,12 +605,6 @@ static int mt7621_pci_probe(struct platform_device *pdev)
 		bypass_pipe_rst(pcie);
 	set_phy_for_ssc(pcie);
 
-	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
-		u32 slot = port->slot;
-		val = read_config(pcie, slot, 0x70c);
-		dev_info(dev, "Port %d N_FTS = %x\n", (unsigned int)val, slot);
-	}
-
 	rt_sysc_m32(0, RALINK_PCIE_RST, RALINK_RSTCTRL);
 	rt_sysc_m32(0x30, 2 << 4, SYSC_REG_SYSTEM_CONFIG1);
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20181124/0e51783f/attachment.asc>


More information about the devel mailing list