Re: [PATCH v7 5/7] PCI: rcar-gen4: Add .ltssm_enable() for other SoC support

[Manivannan Sadhasivam <mani@kernel.org>]

PCI: rcar-gen4: Move LTSSM handling to ltssm_control() callback

On Mon, Apr 15, 2024 at 05:11:33PM +0900, Yoshihiro Shimoda wrote:
> This driver can reuse other R-Car Gen4 SoCs support like r8a779g0 and
> r8a779h0. However, r8a779g0 and r8a779h0 require other initializing
> settings that differ than r8a779f0. So, add a new function pointer
> .ltssm_enable() for it.
> 
> After applied this patch, probing SoCs by rcar_gen4_pcie_of_match[]
> will be changed like below:
> 
> - r8a779f0 as "renesas,r8a779f0-pcie" and "renesas,r8a779f0-pcie-ep"
> 
> Existing dts files have the compatible above. So, no behavior changes.
> 

How about:

Sequence for controlling the LTSSM state machine is going to change for SoCs
like r8a779f0. So let's move the LTSSM code to a new callback ltssm_control()
and populate it for each SoCs.

This also warrants the addition of new compatibles for r8a779g0 and r8a779h0.
But since they are already part of the DT binding, it won't make any difference.

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 41 ++++++++++++++++++---
>  1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 3da0a844e1b6..980a916933d6 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -48,7 +48,9 @@
>  #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET	0x1000
>  #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET	0x800
>  
> +struct rcar_gen4_pcie;
>  struct rcar_gen4_pcie_drvdata {
> +	int (*ltssm_enable)(struct rcar_gen4_pcie *rcar);

int (*ltssm_control)(struct rcar_gen4_pcie *rcar, bool enable);

>  	enum dw_pcie_device_mode mode;
>  };
>  
> @@ -61,8 +63,8 @@ struct rcar_gen4_pcie {
>  #define to_rcar_gen4_pcie(_dw)	container_of(_dw, struct rcar_gen4_pcie, dw)
>  
>  /* Common */
> -static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> -					bool enable)
> +static void rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar,
> +					 bool enable)
>  {
>  	u32 val;
>  
> @@ -127,9 +129,13 @@ static int rcar_gen4_pcie_speed_change(struct dw_pcie *dw)
>  static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
>  {
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> -	int i, changes;
> +	int i, changes, ret;
>  
> -	rcar_gen4_pcie_ltssm_enable(rcar, true);
> +	if (rcar->drvdata->ltssm_enable) {
> +		ret = rcar->drvdata->ltssm_enable(rcar);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/*
>  	 * Require direct speed change with retrying here if the link_gen is
> @@ -157,7 +163,7 @@ static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
>  {
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>  
> -	rcar_gen4_pcie_ltssm_enable(rcar, false);
> +	rcar_gen4_pcie_ltssm_control(rcar, false);

You should use the callback here as like above.

>  }
>  
>  static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
> @@ -504,6 +510,23 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
>  	rcar_gen4_pcie_unprepare(rcar);
>  }
>  
> +static int r8a779f0_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar)
> +{
> +	rcar_gen4_pcie_ltssm_control(rcar, true);
> +
> +	return 0;
> +}
> +
> +static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {
> +	.ltssm_enable = r8a779f0_pcie_ltssm_enable,

Just pass rcar_gen4_pcie_ltssm_control() directly.

- Mani

-- 
மணிவண்ணன் சதாசிவம்