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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);
 

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux