Hi,
On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:
> From: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx>
>
[...]
> +
> + /* 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);
> +
> + 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.
> +
> + writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
> +
> + return 0;
> +}
> +
[...]
Looks good to me otherwise,
Reviewed-by: Sven Peter <sven@xxxxxxxxxxxxx>
Thanks,
Sven
