On Mon, Oct 07, 2024 at 01:12:16PM +0900, Damien Le Moal wrote:
> The Rockchip rk339 technical reference manual describe the endpoint mode
RK3399
Please include the full reference: TRM name followed by the section.
> link training process clearly and states that:
> Insure link training completion and success by observing link_st field
> in PCIe Client BASIC_STATUS1 register change to 2'b11. If both side
> support PCIe Gen2 speed, re-train can be Initiated by asserting the
> Retrain Link field in Link Control and Status Register. The software
> should insure the BASIC_STATUS0[negotiated_speed] changes to "1", that
> indicates re-train to Gen2 successfully.
> This procedure is very similar to what is done for the root-port mode in
> rockchip_pcie_host_init_port().
>
> Implement this link training procedure for the endpoint mode as well.
> Given that the rk3399 SoC does not have an interrupt signaling link
> status changes, training is implemented as a delayed work which is
> rescheduled until the link training completes or the endpoint controller
> is stopped. The link training work is first scheduled in
> rockchip_pcie_ep_start() when the endpoint function is started. Link
> training completion is signaled to the function using pci_epc_linkup().
> Accordingly, the linkup_notifier field of the rockchip pci_epc_features
> structure is changed to true.
>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/pci/controller/pcie-rockchip-ep.c | 79 ++++++++++++++++++++++-
> drivers/pci/controller/pcie-rockchip.h | 11 ++++
> 2 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index a801e040bcad..af50432525b4 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -16,6 +16,8 @@
> #include <linux/platform_device.h>
> #include <linux/pci-epf.h>
> #include <linux/sizes.h>
> +#include <linux/workqueue.h>
> +#include <linux/iopoll.h>
Please keep the includes sorted.
>
> #include "pcie-rockchip.h"
>
> @@ -48,6 +50,7 @@ struct rockchip_pcie_ep {
> u64 irq_pci_addr;
> u8 irq_pci_fn;
> u8 irq_pending;
> + struct delayed_work link_training;
> };
>
> static void rockchip_pcie_clear_ep_ob_atu(struct rockchip_pcie *rockchip,
> @@ -465,6 +468,8 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
> PCIE_CLIENT_CONF_ENABLE,
> PCIE_CLIENT_CONFIG);
>
> + schedule_delayed_work(&ep->link_training, 0);
> +
> return 0;
> }
>
> @@ -473,6 +478,8 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
> struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
> struct rockchip_pcie *rockchip = &ep->rockchip;
>
> + cancel_delayed_work_sync(&ep->link_training);
> +
> /* Stop link training and disable configuration */
> rockchip_pcie_write(rockchip,
> PCIE_CLIENT_CONF_DISABLE |
> @@ -480,8 +487,77 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
> PCIE_CLIENT_CONFIG);
> }
>
> +static void rockchip_pcie_ep_retrain_link(struct rockchip_pcie *rockchip)
> +{
> + u32 status;
> +
> + status = rockchip_pcie_read(rockchip, PCIE_EP_CONFIG_LCS);
> + status |= PCI_EXP_LNKCTL_RL;
> + rockchip_pcie_write(rockchip, status, PCIE_EP_CONFIG_LCS);
> +}
> +
> +static bool rockchip_pcie_ep_link_up(struct rockchip_pcie *rockchip)
> +{
> + u32 val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS1);
> +
> + return PCIE_LINK_UP(val);
> +}
> +
> +static void rockchip_pcie_ep_link_training(struct work_struct *work)
> +{
> + struct rockchip_pcie_ep *ep =
> + container_of(work, struct rockchip_pcie_ep, link_training.work);
> + struct rockchip_pcie *rockchip = &ep->rockchip;
> + struct device *dev = rockchip->dev;
> + u32 val;
> + int ret;
> +
> + /* Enable Gen1 training and wait for its completion */
> + ret = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
> + val, PCIE_LINK_TRAINING_DONE(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + if (ret)
> + goto again;
> +
> + /* Make sure that the link is up */
> + ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
> + val, PCIE_LINK_UP(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + if (ret)
> + goto again;
> +
> + /* Check the current speed */
> + val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
> + if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
PCIE_LINK_IS_GEN2()?
> + /* Enable retrain for gen2 */
> + rockchip_pcie_ep_retrain_link(rockchip);
> + readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
> + val, PCIE_LINK_IS_GEN2(val), 50,
> + LINK_TRAIN_TIMEOUT);
> + }
> +
> + /* Check again that the link is up */
> + if (!rockchip_pcie_ep_link_up(rockchip))
> + goto again;
TRM doesn't mention this check. Is this really necessary?
> +
> + val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0);
> + dev_info(dev,
> + "Link UP (Negociated speed: %sGT/s, width: x%lu)\n",
Negotiated
> + (val & PCIE_CLIENT_NEG_LINK_SPEED) ? "5" : "2.5",
> + ((val & PCIE_CLIENT_NEG_LINK_WIDTH_MASK) >>
> + PCIE_CLIENT_NEG_LINK_WIDTH_SHIFT) << 1);
> +
> + /* Notify the function */
> + pci_epc_linkup(ep->epc);
> +
> + return;
> +
> +again:
> + schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5));
> +}
> +
> static const struct pci_epc_features rockchip_pcie_epc_features = {
> - .linkup_notifier = false,
> + .linkup_notifier = true,
> .msi_capable = true,
> .msix_capable = false,
> .align = ROCKCHIP_PCIE_AT_SIZE_ALIGN,
> @@ -642,6 +718,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
> rockchip = &ep->rockchip;
> rockchip->is_rc = false;
> rockchip->dev = dev;
> + INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training);
>
> epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops);
> if (IS_ERR(epc)) {
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 0263f158ee8d..3963b7097a91 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -26,6 +26,7 @@
> #define MAX_LANE_NUM 4
> #define MAX_REGION_LIMIT 32
> #define MIN_EP_APERTURE 28
> +#define LINK_TRAIN_TIMEOUT (5000 * USEC_PER_MSEC)
pcie-rockchip-host has only 500ms timeout.
- Mani
--
மணிவண்ணன் சதாசிவம்
