On Wed, 22 Sep 2021 22:08:33 +0100,
"Sven Peter" [off-list ref] wrote:

Hi,


On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:

quoted

From: Alyssa Rosenzweig <redacted>

[...]

quoted

+
+	/* Use the first reg entry to work out the port index */
+	port->idx = idx >> 11;
+	port->pcie = pcie;
+	port->np = np;
+
+	port->base = devm_platform_ioremap_resource(platform, port->idx + 2);
+	if (IS_ERR(port->base))
+		return -ENODEV;
+
+	rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);

I think this should be

    rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK);

Ouch. Well caught. I wonder how many of these I introduced...:-/

quoted

+
+	rmw_set(PORT_PERST_OFF, port->base + PORT_PERST);
+	gpiod_set_value(reset, 1);
+
+	ret = readl_relaxed_poll_timeout(port->base + PORT_STATUS, stat,
+					 stat & PORT_STATUS_READY, 100, 250000);
+	if (ret < 0) {
+		dev_err(pcie->dev, "port %pOF ready wait timeout\n", np);
+		return ret;
+	}
+
+	/* Flush writes and enable the link */
+	dma_wmb();

I don't think this barrier is required.

It really isn't, and I though I had removed it. I now wonder whether I
have pushed out the right branch, or messed up by moving from one
machine to another...

quoted

+
+	writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
+
+	return 0;
+}
+

[...]


Looks good to me otherwise,

Reviewed-by: Sven Peter <redacted>

Thanks. Hopefully I'll manage to post a non broken series next time...

	M.

-- 
Without deviation from the norm, progress is not possible.